Uh oh!
There was an error while loading. Please reload this page.
[opt](rowset) Aggregate non-MOW segment key bounds to reduce rowset meta size - #62604
Conversation
…eta size For non-MOW (duplicate / aggregate key) tables, per-segment key bounds are not consumed on the read path — only the rowset-level [min, max] is used by the reader and ordered compaction. In cloud mode, persisting bounds for every segment can blow past FDB's value size limit on commit_rowset. Introduce an `enable_aggregate_non_mow_key_bounds` BE config (default off). When enabled, non-MOW rowsets collapse per-segment bounds into a single [overall_min, overall_max] entry at write time, and compaction preserves this behavior. MOW rowsets always retain per-segment bounds — their `lookup_row_key` path relies on them for delete bitmap computation, and is guarded by a new DCHECK against aggregated input. A new optional `segments_key_bounds_aggregated` flag is added to both RowsetMetaPB and RowsetMetaCloudPB so consumers can distinguish aggregated from per-segment layouts. Proto round-trip, pb_convert, snapshot restore, and index builder all preserve both this flag and the existing `segments_key_bounds_truncated` flag. Correctness notes: - `first_key/last_key` callers (`block_reader`, ordered compaction) already bail out on overlapping rowsets, so for non-overlapping rowsets the aggregated [min, max] equals seg[0].min / seg[last].max exactly. - `merge_rowset_meta` (MOW partial-update publish) DCHECKs both sides are non-aggregated.
hello-stephen
commented
Apr 19, 2026
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
liaoxin01
commented
Apr 19, 2026
/review |
There was a problem hiding this comment.
Pull request overview
This PR introduces an optional optimization to reduce rowset metadata size for non-MOW (duplicate/aggregate-key) tables by aggregating per-segment key bounds into a single rowset-level [min, max] entry, primarily targeting cloud mode FDB value size limits.
Changes:
- Add
enable_aggregate_non_mow_key_boundsBE config (default off) and use it in rowset write + ordered compaction to optionally aggregate non-MOWsegments_key_bounds. - Add
segments_key_bounds_aggregatedto RowsetMeta protobufs and propagate it through pb_convert, snapshot restore, index builder, and rowset meta helpers. - Add BE unit tests and a regression suite validating aggregated/non-aggregated behavior for non-MOW vs MOW tables.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| regression-test/suites/data_model_p0/duplicate/test_non_mow_key_bounds_aggregation.groovy | Regression coverage for config on/off and MOW vs non-MOW layout expectations. |
| gensrc/proto/olap_file.proto | Adds segments_key_bounds_aggregated flag to RowsetMetaPB/RowsetMetaCloudPB. |
| be/test/storage/rowset/rowset_meta_test.cpp | Unit tests for aggregation behavior and truncation interaction. |
| be/src/storage/task/index_builder.cpp | Preserves aggregated/truncated flags when rebuilding rowset meta for index tasks. |
| be/src/storage/tablet/base_tablet.cpp | Adds DCHECK to ensure MOW lookup path never sees aggregated bounds. |
| be/src/storage/rowset/rowset_meta.h | Adds aggregated flag accessors and extends set_segments_key_bounds API. |
| be/src/storage/rowset/rowset_meta.cpp | Implements aggregation in set_segments_key_bounds; adds DCHECK in merge_rowset_meta. |
| be/src/storage/rowset/rowset.h | Exposes is_segments_key_bounds_aggregated() on Rowset. |
| be/src/storage/rowset/beta_rowset_writer.cpp | Aggregates non-MOW key bounds at write time based on new config. |
| be/src/storage/compaction/compaction.cpp | Aggregates non-MOW key bounds for ordered compaction output based on new config. |
| be/src/common/config.h / be/src/common/config.cpp | Declares/defines enable_aggregate_non_mow_key_bounds. |
| be/src/cloud/pb_convert.cpp | Propagates aggregated flag between Doris and cloud rowset meta PBs. |
| be/src/cloud/cloud_snapshot_mgr.cpp | Preserves aggregated/truncated flags during snapshot rowset meta creation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
liaoxin01
commented
Apr 19, 2026
run buildall |
1b267ba to
89e4675Compareliaoxin01
commented
Apr 19, 2026
run buildall |
Uh oh!
There was an error while loading. Please reload this page.
OpenCode automated review failed and did not complete. Error: Review step was failure (possibly timeout or cancelled) Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
hello-stephen
commented
Apr 19, 2026
FE UT Coverage ReportIncrement line coverage `` 🎉 |
liaoxin01
commented
Apr 20, 2026
run buildall |
hello-stephen
commented
Apr 20, 2026
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
hello-stephen
commented
Apr 20, 2026
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
hello-stephen
commented
Apr 20, 2026
FE Regression Coverage ReportIncrement line coverage |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
PR approved by at least one committer and no changes requested. |
PR approved by anyone and no changes requested. |
Correction to my earlier review text: the blocking issue is in be/src/storage/task/index_builder.cpp. rowset_meta->set_segments_key_bounds_aggregated(input_rowset_meta->is_segments_key_bounds_aggregated()) is immediately overwritten by the next rowset_meta->set_segments_key_bounds(key_bounds) call. When the input rowset is already aggregated, key_bounds contains the single aggregated entry, but set_segments_key_bounds(key_bounds) runs with aggregate_into_single=false, so the rebuilt rowset loses the aggregated flag while still keeping num_segments > 1 and segments_key_bounds.size() == 1. Ordered compaction relies on the aggregated flag to preserve that layout when the config is off, so ADD/DROP INDEX can emit inconsistent rowset metadata here. Please pass the source flag into set_segments_key_bounds(key_bounds, input_rowset_meta->is_segments_key_bounds_aggregated()) or remove the standalone setter, and add a test that rebuilds an already-aggregated non-MOW rowset through the index path. |
liaoxin01
commented
Apr 22, 2026
run buildall |
hello-stephen
commented
Apr 22, 2026
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
hello-stephen
commented
Apr 22, 2026
FE UT Coverage ReportIncrement line coverage `` 🎉 |
hello-stephen
commented
Apr 22, 2026
FE Regression Coverage ReportIncrement line coverage |
hello-stephen
commented
Apr 22, 2026
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
PR approved by at least one committer and no changes requested. |
Uh oh!
There was an error while loading. Please reload this page.
…eta size (#62604) For non-MOW (duplicate / aggregate key) tables, per-segment key bounds are not consumed on the read path — only the rowset-level [min, max] is used by the reader and ordered compaction. In cloud mode, persisting bounds for every segment can blow past FDB's value size limit on commit_rowset. Introduce an `enable_aggregate_non_mow_key_bounds` BE config (default off). When enabled, non-MOW rowsets collapse per-segment bounds into a single [overall_min, overall_max] entry at write time, and compaction preserves this behavior. MOW rowsets always retain per-segment bounds — their `lookup_row_key` path relies on them for delete bitmap computation, and is guarded by a new DCHECK against aggregated input. A new optional `segments_key_bounds_aggregated` flag is added to both RowsetMetaPB and RowsetMetaCloudPB so consumers can distinguish aggregated from per-segment layouts. Proto round-trip, pb_convert, snapshot restore, and index builder all preserve both this flag and the existing `segments_key_bounds_truncated` flag. Correctness notes: - `first_key/last_key` callers (`block_reader`, ordered compaction) already bail out on overlapping rowsets, so for non-overlapping rowsets the aggregated [min, max] equals seg[0].min / seg[last].max exactly. - `merge_rowset_meta` (MOW partial-update publish) DCHECKs both sides are non-aggregated.
…eta size (#62604) For non-MOW (duplicate / aggregate key) tables, per-segment key bounds are not consumed on the read path — only the rowset-level [min, max] is used by the reader and ordered compaction. In cloud mode, persisting bounds for every segment can blow past FDB's value size limit on commit_rowset. Introduce an `enable_aggregate_non_mow_key_bounds` BE config (default off). When enabled, non-MOW rowsets collapse per-segment bounds into a single [overall_min, overall_max] entry at write time, and compaction preserves this behavior. MOW rowsets always retain per-segment bounds — their `lookup_row_key` path relies on them for delete bitmap computation, and is guarded by a new DCHECK against aggregated input. A new optional `segments_key_bounds_aggregated` flag is added to both RowsetMetaPB and RowsetMetaCloudPB so consumers can distinguish aggregated from per-segment layouts. Proto round-trip, pb_convert, snapshot restore, and index builder all preserve both this flag and the existing `segments_key_bounds_truncated` flag. Correctness notes: - `first_key/last_key` callers (`block_reader`, ordered compaction) already bail out on overlapping rowsets, so for non-overlapping rowsets the aggregated [min, max] equals seg[0].min / seg[last].max exactly. - `merge_rowset_meta` (MOW partial-update publish) DCHECKs both sides are non-aggregated.
…eta size (#62604) For non-MOW (duplicate / aggregate key) tables, per-segment key bounds are not consumed on the read path — only the rowset-level [min, max] is used by the reader and ordered compaction. In cloud mode, persisting bounds for every segment can blow past FDB's value size limit on commit_rowset. Introduce an `enable_aggregate_non_mow_key_bounds` BE config (default off). When enabled, non-MOW rowsets collapse per-segment bounds into a single [overall_min, overall_max] entry at write time, and compaction preserves this behavior. MOW rowsets always retain per-segment bounds — their `lookup_row_key` path relies on them for delete bitmap computation, and is guarded by a new DCHECK against aggregated input. A new optional `segments_key_bounds_aggregated` flag is added to both RowsetMetaPB and RowsetMetaCloudPB so consumers can distinguish aggregated from per-segment layouts. Proto round-trip, pb_convert, snapshot restore, and index builder all preserve both this flag and the existing `segments_key_bounds_truncated` flag. Correctness notes: - `first_key/last_key` callers (`block_reader`, ordered compaction) already bail out on overlapping rowsets, so for non-overlapping rowsets the aggregated [min, max] equals seg[0].min / seg[last].max exactly. - `merge_rowset_meta` (MOW partial-update publish) DCHECKs both sides are non-aggregated.
…eta size (apache#62604) For non-MOW (duplicate / aggregate key) tables, per-segment key bounds are not consumed on the read path — only the rowset-level [min, max] is used by the reader and ordered compaction. In cloud mode, persisting bounds for every segment can blow past FDB's value size limit on commit_rowset. Introduce an `enable_aggregate_non_mow_key_bounds` BE config (default off). When enabled, non-MOW rowsets collapse per-segment bounds into a single [overall_min, overall_max] entry at write time, and compaction preserves this behavior. MOW rowsets always retain per-segment bounds — their `lookup_row_key` path relies on them for delete bitmap computation, and is guarded by a new DCHECK against aggregated input. A new optional `segments_key_bounds_aggregated` flag is added to both RowsetMetaPB and RowsetMetaCloudPB so consumers can distinguish aggregated from per-segment layouts. Proto round-trip, pb_convert, snapshot restore, and index builder all preserve both this flag and the existing `segments_key_bounds_truncated` flag. Correctness notes: - `first_key/last_key` callers (`block_reader`, ordered compaction) already bail out on overlapping rowsets, so for non-overlapping rowsets the aggregated [min, max] equals seg[0].min / seg[last].max exactly. - `merge_rowset_meta` (MOW partial-update publish) DCHECKs both sides are non-aggregated.
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
For non-MOW (duplicate / aggregate key) tables, per-segment key bounds are not consumed on the read path — only the rowset-level [min, max] is used by the reader and ordered compaction. In cloud mode, persisting bounds for every segment can blow past FDB's value size limit on commit_rowset.
Introduce an
enable_aggregate_non_mow_key_boundsBE config (default off). When enabled, non-MOW rowsets collapse per-segment bounds into a single [overall_min, overall_max] entry at write time, and compaction preserves this behavior. MOW rowsets always retain per-segment bounds — theirlookup_row_keypath relies on them for delete bitmap computation, and is guarded by a new DCHECK against aggregated input.A new optional
segments_key_bounds_aggregatedflag is added to both RowsetMetaPB and RowsetMetaCloudPB so consumers can distinguish aggregated from per-segment layouts. Proto round-trip, pb_convert, snapshot restore, and index builder all preserve both this flag and the existingsegments_key_bounds_truncatedflag.Correctness notes:
first_key/last_keycallers (block_reader, ordered compaction) already bail out on overlapping rowsets, so for non-overlapping rowsets the aggregated [min, max] equals seg[0].min / seg[last].max exactly.merge_rowset_meta(MOW partial-update publish) DCHECKs both sides are non-aggregated.Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)