[feat](cloud) Support Cloud partition inverted format rollout - #66475
Open
hoshinojyunn wants to merge 1 commit into
Open
[feat](cloud) Support Cloud partition inverted format rollout#66475hoshinojyunn wants to merge 1 commit into
hoshinojyunn wants to merge 1 commit into
Conversation
hoshinojyunn
requested review from
924060929,
CalvinKirs,
dataroaring,
englefly,
gavinchou,
liaoxin01,
luwei16,
morningman,
morrySnow,
mymeiyi,
starocean999 and
yiguolei
as code owners
August 5, 2026 07:25
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
hoshinojyunn
force-pushed
the
fix/rowset
branch
from
August 5, 2026 10:28
a37f49b to
cc40ae3
Compare
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 29103 ms |
Contributor
TPC-DS: Total hot run time: 165848 ms |
Contributor
ClickBench: Total hot run time: 25.71 s |
Contributor
FE Regression Coverage ReportIncrement line coverage |
hoshinojyunn
force-pushed
the
fix/rowset
branch
from
August 7, 2026 07:08
cc40ae3 to
d9daee3
Compare
gavinchou
previously approved these changes
Aug 14, 2026
16 tasks
hoshinojyunn
added a commit
to hoshinojyunn/doris
that referenced
this pull request
Aug 15, 2026
### What problem does this PR solve? Issue Number: None Related PR: apache#66475 Problem Summary: This change separately submits the Meta Service portion of apache#66475. During a partition-level inverted-index format rollout, a rowset can use a format that differs from its shared schema metadata. The recycler and checker previously selected index paths from shared schema metadata or its cache, which could target V1 paths for a rowset whose files use V3. Persist the actual inverted-index storage format in rowset and tablet metadata. Make the recycler and checker prefer the rowset format, while retaining schema and schema-KV fallback for metadata written before the field existed. Add a Cloud unit test where the shared schema is V1 and the rowset is V3, verifying that recycler deletes the V3 index file. ### Release note None ### Check List (For Author) - Test: Unit Test - `./run-cloud-ut.sh --run --filter=recycler_test:RecyclerTest.delete_tmp_rowset_data_with_rowset_idx_v3` - Behavior changed: Yes. Meta Service recycler and checker use each rowset\047s persisted inverted-index format when selecting index files. - Does this need documentation: No
gavinchou
pushed a commit
that referenced
this pull request
Aug 18, 2026
Related PR: #66475 Problem Summary: This PR separately submits the Meta Service changes from #66475. When the shared schema format differs from the physical format used by an existing rowset, recycler and checker must use the rowset format to select the correct inverted-index files. The change persists the physical inverted-index format in metadata and makes recycler/checker prefer the rowset value, with the existing schema and schema-KV fallback retained for old metadata. ### Release note None ### Check List (For Author) - Test <!-- At least one of them must be included. --> - [ ] Regression test - [x] Unit Test - `./run-cloud-ut.sh --run --filter=recycler_test:RecyclerTest.delete_tmp_rowset_data_with_rowset_idx_v3` - [ ] Manual test (add detailed scripts or steps below) - [ ] No need to test or manual test. Explain why: - [ ] This is a refactor/code format and no logic has been changed. - [ ] Previous test can cover this change. - [ ] No code files have been changed. - [ ] Other reason <!-- Add your reason? --> - Behavior changed: - [ ] No. - [x] Yes. Recycler and checker select inverted-index files from the persisted rowset format. - Does this need documentation? - [x] No. - [ ] Yes. <!-- Add document PR link here. eg: apache/doris-website#1214 -->
Issue Number: None
Related PR: None
Problem Summary: Cloud tables previously had a single table-level inverted-index storage format. That made V2-to-V3 upgrades all-or-nothing: updating the default could not preserve the format selected when existing partitions and their tablet metadata were created.
This change adds the Cloud-only partition.inverted_index_storage_format table property. It controls the format for subsequently created partitions, while each existing partition retains its resolved V2 or V3 format. The resolved value is durable in partition metadata and is carried through journal replay, recycle/recover, truncate, insert overwrite, and schema-change paths. Tablet and rowset metadata record the physical format separately so mixed-format partitions remain distinguishable during a rolling upgrade. Partition inspection commands read the value from FE partition metadata.
Cloud OLAP tables can roll out inverted-index storage formats by partition. New partitions can use V3 while existing V2 partitions remain online and queryable.
- Test: Regression test / Unit Test
- FE unit tests covering partition properties, persistence, recycle/recover, and Cloud schema-change handling
- BE unit tests covering rowset and tablet metadata serialization
- Cloud regression test: test_partition_cloud_inverted_index_storage_format_meta_write_switch
- Behavior changed: Yes. Cloud tables accept partition.inverted_index_storage_format for new partitions and expose the resolved format in partition metadata commands.
- Does this need documentation: No
hoshinojyunn
force-pushed
the
fix/rowset
branch
from
August 19, 2026 17:10
d9daee3 to
513370a
Compare
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 17342 ms |
Contributor
TPC-DS: Total hot run time: 83255 ms |
Contributor
ClickBench: Total hot run time: 14.54 s |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Member
|
/review |
Contributor
|
Codex 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. |
Member
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Request changes: I found eight independently verified issues (seven P1, one P2) in the per-partition inverted-index format rollout.
Review coverage and checkpoint conclusions:
- Goal and test: the PR adds durable per-partition V2/V3/SNII selection across FE and BE metadata plus SHOW/TVF observability. The happy-path unit and regression coverage is broad, but it does not exercise the restart/failover, mixed-version activation, selected restore, snapshot-rowset restore, truncate-old-partition recovery, or value-level TVF cases that expose these issues.
- Scope and focus: reviewed all 51 changed files and all 3,817 lines of the authoritative diff; no additional user-provided focus was present.
- Concurrency, lifecycle, and initialization: no separate lock-order or static-initialization defect was found. The material lifecycle failures are process-local config drift after FE restart/failover, truncate/recover metadata loss, and selected/snapshot restore omissions.
- Configuration dynamics: rollout enablement changes interpretation of already-persisted metadata but is mutable, master-only, default-false, and non-journaled. Durable raw values must remain authoritative after activation.
- Compatibility and rolling upgrade: the new top-level BE discriminators work only after every serving BE understands them; activation currently has neither an enforced capability fence nor a documented and tested operator precondition.
- Parallel paths and special conditions: ordinary create/add/auto/dynamic/temp/insert-overwrite, load, compaction, partial update, schema change, protobuf conversion, cache resolution, and rollup paths were traced. Snapshot restore remains a direct writer bypass, selected-partition restore omits the FE map, and ALTER/ADD/BUILD validation is still table-scoped despite per-partition V2/V3/SNII execution.
- Tests and results: no local builds or tests were run, as required by the review-only bundle. On the reviewed head, every non-skipped GitHub check is successful, including CheckStyle, Clang Formatter, License Check, secret/dependency checks, and BE UT on macOS; those checks do not cover the failure paths above.
- Observability: SHOW/PROC and the BE debug synchronization path were checked without another defect, but the
$partitionsTVF maps the new field toBinlogSizerather than the format column. - Persistence, failover, and write safety: image/add-partition/recycle persistence and normal metadata conversions were checked. The accepted findings cover non-deterministic FE interpretation, truncate recycle loss, selected restore publication, and rowset-schema detachment during snapshot restore; no separate atomicity or crash-window issue was substantiated.
- FE/BE variable paths: FE creation and schema-change consumers were traced to the per-partition getters, and BE tablet/rowset override propagation was traced through shared schema-KV resolution. The identified gaps are the old-BE path and the direct restore writer.
- Performance: the new test mocks away the production
Env.modifyTablePropertiespath, which still scans all partitions under the table write lock for this future-only property. No other material regression was found. - Other issues: all candidates were independently validated and deduplicated against the live PR. Round 2 complete-review and risk-focused passes all reported
NO_NEW_VALUABLE_FINDINGS, so review convergence was reached without a third round.
| // init _schema | ||
| TabletSchemaSPtr schema = std::make_shared<TabletSchema>(); | ||
| schema->init_from_pb(tablet_meta_pb.schema()); | ||
| TabletSchemaPB schema_pb = tablet_meta_pb.schema(); |
Contributor
There was a problem hiding this comment.
[P1] Enforce or document the old-BE activation precondition. These top-level overrides are the only place a BE distinguishes two physical formats sharing one schema-KV key; a pre-change BE ignores the unknown fields and uses whichever format the shared schema contains. The default-false switch can serve as a manual fence only if operators wait until every serving BE supports these fields, but the dynamic setter performs no capability/version check and the PR adds no documented/tested activation order. Enabling it while an old BE still serves lets that BE read or write a new V3/SNII partition as V2. Please enforce the all-BEs-supported precondition, or make the activation contract explicit and test it.
| new Column("UnsyncTables", ScalarType.createStringType()), | ||
| new Column("CommittedVersion", ScalarType.createType(PrimitiveType.BIGINT)), | ||
| new Column("RowCount", ScalarType.createType(PrimitiveType.BIGINT)), | ||
| new Column("InvertedIndexStorageFormat", ScalarType.createStringType())); |
Contributor
There was a problem hiding this comment.
[P1] Keep the TVF schema aligned with the proc row. PartitionsProcDir emits CommittedVersion, RowCount, BinlogSize, then InvertedIndexStorageFormat, but this schema omits BinlogSize and derives source indices from its own ordinal. MetadataGenerator therefore fetches proc cell 22 for the new field, returning a human-readable binlog size instead of V2/V3/SNII. Add BinlogSize before this column (or use an explicit source map) and add a value-level TVF assertion.
| new CloudSchemaChangeHandler().updateTableProperties(database, "tbl", properties); | ||
|
|
||
| Mockito.verify(env).modifyTableProperties(database, table, properties); | ||
| Mockito.verify(table, Mockito.never()).getAllPartitions(); |
Contributor
There was a problem hiding this comment.
[P2] This test does not exercise the scan it claims to exclude. Env is mocked, so modifyTableProperties never reaches its real unconditional table.getPartitions loop, which runs under the table write lock and rewrites in-memory/storage-policy entries even when this future-only property is the sole change. On a large auto-partitioned table the new ALTER remains O(partitions). Please skip that generic propagation when no partition-wide property changed and make the test execute or directly cover the real Env path.
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
FE Regression Coverage ReportIncrement line coverage |
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
4 participants
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.
What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary:
Cloud tables previously had one table-level inverted_index_storage_format, so an online V2-to-V3 migration required all partitions to move together. This prevents a gradual rollout and cannot retain the format selected when an existing partition was created while new partitions use a new default.
This PR adds the Cloud-only table property partition.inverted_index_storage_format. It is the default for partitions created after the property is set. The property supports V2 and V3; V1 is rejected. Existing partitions and their tablet metadata are not rewritten by a property update.
Basic usage, with
enable_partition_inverted_index_storage_format_rolloutenabled:Expected result:
Release note
Cloud OLAP tables can roll out inverted-index file storage formats by partition. New partitions can use V3 while existing V2 partitions remain online and queryable.
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)