Fix realtime segment size metadata#18992
Draft
xiangfu0 wants to merge 1 commit into
Draft
Conversation
b689cc5 to
3490f3c
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #18992 +/- ##
============================================
+ Coverage 65.22% 65.24% +0.01%
Complexity 1405 1405
============================================
Files 3418 3418
Lines 214651 214674 +23
Branches 33922 33924 +2
============================================
+ Hits 140006 140062 +56
+ Misses 63356 63313 -43
- Partials 11289 11299 +10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
segmentSizeBytesand creating the commit archive.getSegmentSizeBytes()value.segment.size.in.bytesmetadata from the loaded immutable segment during deep-store recovery of an already-DONE segment.This change does not add compressed-size metadata or change the segment completion protocol.
Why the old value was wrong
RealtimeSegmentDataManagerretains the table configuration captured when consumption starts. At seal time, the realtime converter used that construction-time snapshot and immediately measured the generated directory. That value was sent to the controller and persisted assegment.size.in.bytes.The segment was not necessarily in its final on-disk form yet. When the consuming segment was replaced,
RealtimeTableDataManagerfetched the latest table configuration andImmutableSegmentLoaderpreprocessed the files. Preprocessing can remove indexes that were deleted from the table config or rebuild indexes whose definitions changed. For a long-running consuming segment, the preprocessed directory could therefore be substantially smaller than the directory measured before finalization.The result was a stale pre-finalization byte count in ZK metadata, even though every loaded replica contained the smaller finalized immutable segment. In the regression fixture, the old path published
37,452bytes while the finalized loaded segment occupied35,174bytes.The deep-store recovery path had a second stale-value case. The selected server already returned
ImmutableSegment#getSegmentSizeBytes(), but the controller only copied the full response for a COMMITTING segment. For an already-DONE segment it updated CRC fields and the download URL while preserving the old size, so recovery could not repair the confusing value.Fix
Before publishing a locally generated segment, the server now:
IndexLoadingConfig.The local replacement path checks whether another preprocessing pass is actually needed. It skips the pass when the just-finalized files still match the current configuration, but preprocesses again if the configuration changed between build and replacement.
For deep-store recovery, the controller accepts a non-negative size returned by the selected server for DONE segments, just as it already accepts refreshed CRC metadata.
segment.size.in.bytesremains the logical uncompressed footprint of the finalized immutable segment. It is not the compressed archive length and is not filesystem-allocated block usage fromdu. It is one shared metadata snapshot; later reloads can still make individual replica footprints differ.User guide and sample queries
pinot-tools/src/main/resources/examples/compression-stats/README.mdnow explains the field and includescurl/jqexamples comparing:segment.size.in.bytesfrom the segment metadata API; anddiskSizeInBytesfrom/tables/{table}/size?verbose=true.There is no sample table-config change because this fix introduces no configuration option.
Testing
RealtimeSegmentDataManagerTest: 28 tests passed, including a regression that changes the inverted-index configuration while a segment is consuming and verifies the published size equals the finalized loaded size.LoaderTest#testIfNeedConvertSegmentFormat: verifies file-based preprocessing decisions before and after format conversion.PinotLLCRealtimeSegmentManagerTest#testUploadCommittedSegment: verifies DONE-segment recovery replaces stale size metadata with the loaded server value.spotless:apply,checkstyle:check,license:format, andlicense:checkpassed forpinot-segment-local,pinot-core,pinot-controller, andpinot-tools.