-
Notifications
You must be signed in to change notification settings - Fork 2.2k
[Feature] Add tablet range pruning for range-distribution Lake tables for shared segments and sstables. #66743
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@cursor review |
alvin-celerdata
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to know how to reuse the same rowset with different seek ranges.
be/src/storage/lake/rowset.h
Outdated
| private: | ||
| Status _init_seek_range(); | ||
|
|
||
| TabletManager* _tablet_mgr; | ||
| int64_t _tablet_id; | ||
| const RowsetMetadataPB* _metadata; | ||
| int _index; | ||
| TabletSchemaPtr _tablet_schema; | ||
| TabletMetadataPtr _tablet_metadata; | ||
| std::vector<SegmentSharedPtr> _segments; | ||
| bool _parallel_load; | ||
| // only takes effect when rowset is overlapped, tells how many segments will be used in compaction, | ||
| // default is 0 means every segment will be used. | ||
| // only used for compaction | ||
| size_t _compaction_segment_limit; | ||
| // represents the seek range of the rowset, this member is only used for range distribution table | ||
| std::unique_ptr<SeekRange> _seek_range; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be a significant change for Rowset.
Before this change, the rowset could be reused for all requests because every reader would have the same range.
But when a rowset has a range, it becomes hard to reuse, especially when you are trying to record the seek range in this class.
If a rowset will be reused for different ranges, how do you differentiate the various ranges for the same rowset?
I will set a request change before we have an alignment on how you are going to handle it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have remove the _seek_range from rowset and rename it to avoid ambiguity. _seek_range means tablet_range which is a immutable property for a range distribution tablet. tablet_range is totally different from the query range. It means that even a rowset is reused by different reader with different query ranges, this rowset must belong to the same tablet so all reader share this rowset must satisfy the same tablet_range to meet the tablet definition
2a0eb99 to
9e26d29
Compare
|
@cursor review |
|
@cursor review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Bugbot reviewed your changes and found no bugs!
4ba44c7 to
ee64c88
Compare
|
@cursor review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Bugbot reviewed your changes and found no bugs!
ee64c88 to
f222e99
Compare
[BE Incremental Coverage Report]❌ fail : 11 / 27 (40.74%) file detail
|
|
@cursor review |
0a583c1 to
b40a582
Compare
|
@cursor review |
|
@cursor review |
| return Status::InternalError( | ||
| fmt::format("Sort key index {} must be the same as pk key index {}", i, sort_key_idxes[i])); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing runtime check for sort_key_idxes bounds access
The code uses DCHECK_EQ(sort_key_idxes.size(), tablet_schema->num_key_columns()) at line 169, but DCHECK only runs in debug builds. The loop at line 172 then iterates up to num_key_columns() and accesses sort_key_idxes[i] at line 173. In release builds, if sort_key_idxes.size() < num_key_columns() (due to data corruption or older data), this would cause an out-of-bounds vector access with undefined behavior. While the FE analyzer validates this constraint for new tables, the runtime check relies only on a debug assertion.
22a39aa to
ba85e7b
Compare
|
@cursor review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Bugbot reviewed your changes and found no bugs!
[Java-Extensions Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[FE Incremental Coverage Report]❌ fail : 4 / 6 (66.67%) file detail
|
… for shared segments and sstables. Signed-off-by: srlch <[email protected]>
Signed-off-by: srlch <[email protected]>
Signed-off-by: srlch <[email protected]>
ba85e7b to
bc01377
Compare
|



Summary
Adds tablet-range based pruning for Lake range-distribution tablets across:
Query path (Rowset → SegmentReadOptions.tablet_range → SegmentIterator::_apply_tablet_range), only for shared segments.
PK index compaction path (both serial LakePersistentIndex::major_compact and parallel LakePersistentIndexParallelCompactMgr) via TabletRangeHelper::create_sst_seek_range_from.
Centralizes TabletRangePB parsing in TabletRangeHelper and introduces a standalone SstSeekRange type under lake/, cleaning up includes and ownership between helpers and the compaction manager.
UTs cover single/multi-column PK, inclusive/exclusive bounds, invalid metadata, CHAR/VARCHAR, and both serial/parallel compaction with tablet range filtering.
Fixes #64986
What type of PR is this:
Does this PR entail a change in behavior?
If yes, please specify the type of change:
Checklist:
Bugfix cherry-pick branch check:
Note
Introduces tablet-range aware pruning for Lake range-distribution tablets and consolidates range parsing.
TabletRangeHelperandSstSeekRangeto parseTabletRangePBand derive seek/stop keys; wires into build (CMake) and addsTabletMetadataPB.rangein protoSegmentReadOptions.tablet_rangeandSegmentIterator::_apply_tablet_range;Rowsetsupplies range only for shared segmentsLakePersistentIndex::{prepare_merging_iterator,merge_sstables,major_compact}andLakePersistentIndexParallelCompactMgrrestrict input/merge by tablet range when sstables are sharedRangeRouternow builds boundaries viaTabletRangeHelperand strengthens range validationCreateTableAnalyzerenforces sort keys equal primary key order when range distribution is enabled for PK tablesWritten by Cursor Bugbot for commit ba85e7b. This will update automatically on new commits. Configure here.