Repository navigation
[Opt](cloud) Notify BE to make committed rowset visible directly after txn commit without fetching from meta service - #59754
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
ed17b16 to
2394c9c
Compare
0a3020d to
59a710c
Compare
|
run buildall |
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
TPC-H: Total hot run time: 31594 ms |
TPC-DS: Total hot run time: 173905 ms |
ClickBench: Total hot run time: 27.82 s |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
|
run cloud_p0 |
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
FE Regression Coverage ReportIncrement line coverage |
7bd047b to
728053d
Compare
|
run buildall |
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
e1f8a1b to
2f31af9
Compare
2f31af9 to
1586ff7
Compare
|
run buildall |
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
TPC-H: Total hot run time: 27029 ms |
TPC-DS: Total hot run time: 169021 ms |
|
run p0 |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
…r txn commit without fetching from meta service (apache#59754) 1FE+3BE, 400bucket mow table, 10 concurrency stream load before: <img width="3218" height="1300" alt="image" src="https://github.com/user-attachments/assets/3d4fe51d-4fae-484b-a257-a6b5ec010970" /> after: <img width="3206" height="1304" alt="image" src="https://github.com/user-attachments/assets/e7230b3c-999d-407b-aa57-783492995e5b" /> --- Problem Summary In cloud storage mode, after a load transaction commits rowsets to Meta Service (MS), BE nodes need to fetch the committed rowset metadata from MS during subsequent sync_rowsets() operations. This introduces additional latency and MS query overhead, especially for high-frequency import scenarios. This PR implements a notification mechanism where FE/BE directly notifies BE nodes about committed rowsets after transaction commit, allowing BE to update tablet metadata immediately without fetching from MS. Solution 1. CloudCommittedRSMgr (BE): A new in-memory manager that caches committed temporary rowset metadata after they are committed to MS - Stores rowset meta with expiration time - Supports efficient lookup by (txn_id, tablet_id) - Handles empty rowset markers - Background thread for cleaning expired entries 2. Notification Flow: - When FE commits a load transaction to MS, it sends MakeCloudTmpRsVisibleTask to involved BE nodes - BE receives the task with final version/visible_ts information - BE promotes cached rowset metadata to tablet meta directly - BE can forward notification to FE if needed (for tablets not on the original BE) 3. Configuration: - FE: enable_cloud_notify_be_after_load_txn_commit (default: false) - BE: enable_cloud_make_rs_visible_on_be (default: false) - BE: cloud_make_committed_rs_visible_worker_count (default: 4) - BE: cloud_mow_sync_rowsets_when_load_txn_begin (default: true) Benefits - Reduces Meta Service query pressure by avoiding redundant rowset metadata fetches - Improves import latency by making rowsets visible faster - Optimizes cloud storage mode performance for high-frequency imports
…r txn commit without fetching from meta service (apache#59754) 1FE+3BE, 400bucket mow table, 10 concurrency stream load before: <img width="3218" height="1300" alt="image" src="https://github.com/user-attachments/assets/3d4fe51d-4fae-484b-a257-a6b5ec010970" /> after: <img width="3206" height="1304" alt="image" src="https://github.com/user-attachments/assets/e7230b3c-999d-407b-aa57-783492995e5b" /> --- Problem Summary In cloud storage mode, after a load transaction commits rowsets to Meta Service (MS), BE nodes need to fetch the committed rowset metadata from MS during subsequent sync_rowsets() operations. This introduces additional latency and MS query overhead, especially for high-frequency import scenarios. This PR implements a notification mechanism where FE/BE directly notifies BE nodes about committed rowsets after transaction commit, allowing BE to update tablet metadata immediately without fetching from MS. Solution 1. CloudCommittedRSMgr (BE): A new in-memory manager that caches committed temporary rowset metadata after they are committed to MS - Stores rowset meta with expiration time - Supports efficient lookup by (txn_id, tablet_id) - Handles empty rowset markers - Background thread for cleaning expired entries 2. Notification Flow: - When FE commits a load transaction to MS, it sends MakeCloudTmpRsVisibleTask to involved BE nodes - BE receives the task with final version/visible_ts information - BE promotes cached rowset metadata to tablet meta directly - BE can forward notification to FE if needed (for tablets not on the original BE) 3. Configuration: - FE: enable_cloud_notify_be_after_load_txn_commit (default: false) - BE: enable_cloud_make_rs_visible_on_be (default: false) - BE: cloud_make_committed_rs_visible_worker_count (default: 4) - BE: cloud_mow_sync_rowsets_when_load_txn_begin (default: true) Benefits - Reduces Meta Service query pressure by avoiding redundant rowset metadata fetches - Improves import latency by making rowsets visible faster - Optimizes cloud storage mode performance for high-frequency imports
### Background Cloud MoW load initialization can synchronize rowsets from Meta Service before capturing its local rowset snapshot. The existing mutable cloud_mow_sync_rowsets_when_load_txn_begin setting controls this synchronization. ### What problem does this PR solve? Issue Number: None Related PR: apache#59754 (introduced the setting), apache#70218 (enables notifications and fixes cleanup), apache#70209 (temporary BE UT build prerequisite) PR Type: Enhancement Problem Summary: Make rowset synchronization at the start of a cloud MoW load opt-in by changing cloud_mow_sync_rowsets_when_load_txn_begin from true to false. Adjust the related CloudGroupRowsetBuilderWriterTest fixture to restore the incoming configuration value instead of hard-coding true during teardown. ### Details - **Before and after:** Cloud MoW loads no longer force sync_rowsets before constructing their initial local rowset snapshot by default. - **Configs and defaults:** cloud_mow_sync_rowsets_when_load_txn_begin changes from true to false and remains dynamically mutable. - **Impact on existing users:** An explicit true setting retains synchronization at load initialization. Notification delivery and subsequent metadata synchronization remain relevant to local rowset freshness. - **Behavior change:** Yes; skip the initial synchronization unless the setting is enabled. The test fixture preserves whichever configuration value it received. ### Testing - ./run-be-ut.sh --run --filter='CloudGroupRowsetBuilderWriterTest.*:*DeltaWriterCancelIntegrationTest.*' -j100 passed all 12 ASAN tests after this change. - The unit-test build temporarily applied apache#70209 to fix the unrelated vparquet_writer_test.cpp compilation failure. That prerequisite was removed afterward and is not included in this commit. - clang-format v16/check-format on both changed files and git diff --check passed. - The earlier 36 BE and 56 FE passing tests belong to commit 1b4d168, before this default update; they were not rerun for this commit. - clang-tidy was omitted. The stale-cache partial-update scenario below was not exercised in a running cloud cluster. ### Notes for Reviewers The stale-rowset partial-update issue identified during source review remains unresolved: if a BE has not received or synchronized a prior committed rowset, constructing the MoW context from its local snapshot can classify an existing key as new and fail a partial update under the ERROR policy or when an omitted column is non-nullable and has no default. This commit changes the default; it does not repair that path. ### Release note The BE setting cloud_mow_sync_rowsets_when_load_txn_begin now defaults to false. Cloud MoW loads skip forced rowset synchronization at initialization unless this setting is explicitly enabled. ### Check List (For Author) - Test: Unit Test; 12 BE ASAN tests passed with the prerequisite described above. - Behavior changed: Yes (see Details). - Does this need documentation: Yes; include this third default-value change in the configuration reference and release notes alongside the two notification settings. - Pre-submission status: Partially verified: focused tests passed; the known stale-cache partial-update risk remains unresolved and cloud end-to-end validation is pending. ### AI Disclosure - Tool: Codex (GPT-6). - Generated: Code, test fixture adjustment and this description. - Human review: Pending.
…by default ### Background After a cloud load transaction commits in Meta Service, FE can notify the participating BEs of its visible versions. BE uses cached committed rowsets or lightweight empty-rowset entries to update local tablet metadata, with normal metadata synchronization as the fallback. ### What problem does this PR solve? Issue Number: None Related PR: apache#59754 (introduced the notification path), apache#70209 (temporary prerequisite for local BE unit-test builds) PR Type: Bugfix Problem Summary: Completed notification tasks retain their registered signatures, including when the BE feature is disabled. Consumed empty-rowset markers also remain cached because removal only covers the separate non-empty-rowset map. Release signatures on every callback exit and represent both rowset kinds in one cache so removal and expiration share the same lifecycle. Enable the existing FE and BE notification settings by default. ### Details - **Frequency:** Every accepted notification leaves its signature behind; every consumed empty entry remains until expiration. - **Root cause:** The callback does not pair task registration with removal, and the split empty/non-empty caches have different removal behavior. - **Why existing tests missed it:** They cover insertion and expiration but do not check removal of an empty entry or resubmission after a notification callback completes. - **Fix:** Add a Defer guard before the BE enable check. Store empty rowsets as null metadata in the existing committed-rowset cache, reuse insertion and expiration, and avoid a redundant lookup during cleanup. - **Other paths and branches:** The guard covers enabled notifications, disabled notifications and cache misses. Non-empty and empty committed rowsets now share cleanup; the independent MoW delete-bitmap cache is unchanged. This change targets master. - **Behavior change:** enable_notify_be_after_load_txn_commit and enable_cloud_make_rs_visible_on_be now default to true. Both retain their existing dynamic mutability. cloud_mow_sync_rowsets_when_load_txn_begin remains true. - **Concurrency and lifecycle:** Cache operations retain the existing shared mutex and acquire no new nested locks. Signature cleanup runs after tablet processing has released its locks. Refreshed entries remain protected from older expiration records. - **Compatibility and data correctness:** No RPC fields, persistent formats, transaction commit rules or partition-version visibility checks change. Missing cached data continues to use the existing metadata synchronization path. - **Observability and cost:** Move per-rowset insertion logging outside the cache lock and lower it to DEBUG as notifications become enabled by default. Existing notification and expiration logs retain transaction/tablet identifiers. ### Testing - Before the fix: four new ASAN cases ran; empty-entry removal and both enabled/disabled signature-release cases failed, while the expiration-refresh case passed. - After the final change: ./run-be-ut.sh --run --filter='CloudCommittedRSMgrTest.*:EnabledAndDisabled/CloudCommittedRowsetTaskTest.*:TaskWorkerPoolTest.*:CloudTabletApplyVisiblePendingTest.*:CloudGroupRowsetBuilderWriterTest.*' -j100 passed all 36 tests. - ./run-fe-ut.sh --run org.apache.doris.cloud.transaction.CloudGlobalTransactionMgrTest passed all 56 tests. - clang-format v16/check-format on all six changed C++ files, build-support/check-build-hygiene.sh, FE fe-common Maven validate/Checkstyle, and git diff --check passed. - BE unit-test builds used the temporary apache#70209 fix for an unrelated master compilation failure in vparquet_writer_test.cpp. The prerequisite was restored afterward and is not part of this commit. - clang-tidy was not completed. A cloud end-to-end regression and a standalone production BE build were not run. ### Release note Cloud committed-rowset notifications are enabled by default through enable_notify_be_after_load_txn_commit on FE and enable_cloud_make_rs_visible_on_be on BE. Completed notifications release their task signatures, and consumed empty-rowset entries are removed promptly. ### Check List (For Author) - Test: Unit Test; 36 BE ASAN tests and 56 FE tests passed with the prerequisite described above. - Behavior changed: Yes (see Details). - Does this need documentation: Yes; document the two default-value changes in the configuration reference and release notes. - Pre-submission status: Partially verified: targeted tests passed; cloud end-to-end regression, standalone production BE build and clang-tidy are not completed. The master BE UT build requires apache#70209. ### AI Disclosure - Tool: Codex (GPT-6). - Generated: Code, tests and this description. - Human review: Pending.
### Background Cloud MoW load initialization can synchronize rowsets from Meta Service before capturing its local rowset snapshot. The existing mutable cloud_mow_sync_rowsets_when_load_txn_begin setting controls this synchronization. ### What problem does this PR solve? Issue Number: None Related PR: apache#59754 (introduced the setting), apache#70218 (enables notifications and fixes cleanup), apache#70209 (temporary BE UT build prerequisite) PR Type: Enhancement Problem Summary: Make rowset synchronization at the start of a cloud MoW load opt-in by changing cloud_mow_sync_rowsets_when_load_txn_begin from true to false. Adjust the related CloudGroupRowsetBuilderWriterTest fixture to restore the incoming configuration value instead of hard-coding true during teardown. ### Details - **Before and after:** Cloud MoW loads no longer force sync_rowsets before constructing their initial local rowset snapshot by default. - **Configs and defaults:** cloud_mow_sync_rowsets_when_load_txn_begin changes from true to false and remains dynamically mutable. - **Impact on existing users:** An explicit true setting retains synchronization at load initialization. Notification delivery and subsequent metadata synchronization remain relevant to local rowset freshness. - **Behavior change:** Yes; skip the initial synchronization unless the setting is enabled. The test fixture preserves whichever configuration value it received. ### Testing - ./run-be-ut.sh --run --filter='CloudGroupRowsetBuilderWriterTest.*:*DeltaWriterCancelIntegrationTest.*' -j100 passed all 12 ASAN tests after this change. - The unit-test build temporarily applied apache#70209 to fix the unrelated vparquet_writer_test.cpp compilation failure. That prerequisite was removed afterward and is not included in this commit. - clang-format v16/check-format on both changed files and git diff --check passed. - The earlier 36 BE and 56 FE passing tests belong to commit 1b4d168, before this default update; they were not rerun for this commit. - clang-tidy was omitted. The stale-cache partial-update scenario below was not exercised in a running cloud cluster. ### Notes for Reviewers The stale-rowset partial-update issue identified during source review remains unresolved: if a BE has not received or synchronized a prior committed rowset, constructing the MoW context from its local snapshot can classify an existing key as new and fail a partial update under the ERROR policy or when an omitted column is non-nullable and has no default. This commit changes the default; it does not repair that path. ### Release note The BE setting cloud_mow_sync_rowsets_when_load_txn_begin now defaults to false. Cloud MoW loads skip forced rowset synchronization at initialization unless this setting is explicitly enabled. ### Check List (For Author) - Test: Unit Test; 12 BE ASAN tests passed with the prerequisite described above. - Behavior changed: Yes (see Details). - Does this need documentation: Yes; include this third default-value change in the configuration reference and release notes alongside the two notification settings. - Pre-submission status: Partially verified: focused tests passed; the known stale-cache partial-update risk remains unresolved and cloud end-to-end validation is pending. ### AI Disclosure - Tool: Codex (GPT-6). - Generated: Code, test fixture adjustment and this description. - Human review: Pending.
### What problem does this PR solve? Issue Number: None Related PR: apache#59754 Problem Summary: Cloud p0 and p1 still explicitly configure committed-rowset notifications and initial MoW rowset synchronization to the values now provided by the defaults. Remove these six overrides from the four FE/BE pipeline configuration files so the pipelines exercise the defaults while retaining the same effective settings. ### Release note None ### Check List (For Author) - Test: Manual check of all removed values against the FE/BE defaults, comparison of the remaining parsed settings, and git diff --check passed. No runtime rerun for this configuration cleanup because the effective values are unchanged. - Behavior changed: No (the pipeline settings resolve to the same values through defaults) - Does this need documentation: No
…faults (#70218) ### Background After a cloud load transaction commits in Meta Service, FE can notify the participating BEs of its visible versions. BE uses cached committed rowsets or lightweight empty-rowset entries to update local tablet metadata, with normal metadata synchronization as the fallback. ### What problem does this PR solve? Issue Number: None Related PR: #59754 (introduced the notification path), #70209 (fixes the unrelated Parquet unit-test build failure and is now included in master) PR Type: Bugfix Problem Summary: Completed notification tasks retain their registered signatures, including when the BE feature is disabled. Consumed empty-rowset markers also remain cached because removal only covers the separate non-empty-rowset map. Release signatures on every callback exit and represent both rowset kinds in one cache so removal and expiration share the same lifecycle. Enable the existing FE and BE notification settings by default, and default cloud_mow_sync_rowsets_when_load_txn_begin to false so cloud MoW loads skip forced synchronization when they begin. Remove the six now-redundant FE/BE overrides from the Cloud p0/p1 pipeline configs so those pipelines exercise the new defaults with unchanged effective settings. ### Details - **Frequency:** Every accepted notification leaves its signature behind; every consumed empty entry remains until expiration. - **Root cause:** The callback does not pair task registration with removal, and the split empty/non-empty caches have different removal behavior. - **Why existing tests missed it:** They cover insertion and expiration but do not check removal of an empty entry or resubmission after a notification callback completes. - **Fix:** Add a Defer guard before the BE enable check. Store empty rowsets as null metadata in the existing committed-rowset cache, reuse insertion and expiration, and avoid a redundant lookup during cleanup. - **Other paths and branches:** The guard covers enabled notifications, disabled notifications and cache misses. Non-empty and empty committed rowsets now share cleanup; the independent MoW delete-bitmap cache is unchanged. This change targets master. - **Behavior change:** `enable_notify_be_after_load_txn_commit` and `enable_cloud_make_rs_visible_on_be` now default to `true`; `cloud_mow_sync_rowsets_when_load_txn_begin` now defaults to `false`. All three settings retain their existing dynamic mutability. The cloud rowset-builder test fixture now restores the incoming setting rather than hard-coding `true` at teardown. - **Concurrency and lifecycle:** Cache operations retain the existing shared mutex and acquire no new nested locks. Signature cleanup runs after tablet processing has released its locks. Refreshed entries remain protected from older expiration records. - **Compatibility and data correctness:** RPC fields and persistent formats are unchanged. Cloud MoW loads continue to use a local flush snapshot. Skipping initialization synchronization enlarges the existing snapshot freshness window; the partial-update validation and publish-alignment algorithms are unchanged. The rolling-upgrade caveat is described below. - **Observability and cost:** Move per-rowset insertion logging outside the cache lock and lower it to DEBUG as notifications become enabled by default. Existing notification and expiration logs retain transaction/tablet identifiers. ### Testing - The implementation commits were rebased onto master `87b462e4f58104140f098f5f4a8b5d58bec70149`. The BE/FE runs below tested implementation head `17d42f6ef184c7f851920830ad3ce4861fe679aa`. Final head `4ba6d0db1480dd018a2e9353cffbb6137fe1c65d` adds only the pipeline-configuration cleanup. - On the rebased head, `./run-be-ut.sh --run --filter='CloudCommittedRSMgrTest.*:EnabledAndDisabled/CloudCommittedRowsetTaskTest.*:TaskWorkerPoolTest.*:CloudTabletApplyVisiblePendingTest.*:CloudGroupRowsetBuilderWriterTest.*:*DeltaWriterCancelIntegrationTest.*' -j100` passed all 46 ASAN tests with the requested defaults. - On the same rebased head, `./run-fe-ut.sh --run org.apache.doris.cloud.transaction.CloudGlobalTransactionMgrTest` passed all 56 tests (0 failures, errors or skips); the Maven reactor completed successfully. - The BE build compiled both `task_worker_pool_test.cpp` and `vparquet_writer_test.cpp` successfully. The latter uses the #70209 fix already merged into master; no temporary source patch was applied. - Before the original cleanup fix, the empty-entry removal test and both enabled/disabled signature-release tests failed; the expiration-refresh test passed. - `build-support/check-build-hygiene.sh` and `git diff --check` passed after rebase. The unchanged PR patch previously passed clang-format v16/check-format and FE fe-common Maven validate/Checkstyle. - For the final pipeline cleanup, checked all six removed values against the FE/BE defaults, compared the remaining parsed settings in all four files, and ran `git diff --check`: all passed. BE/FE tests were not rerun for this config-only commit because its effective pipeline values are unchanged. - clang-tidy was not rerun, as requested. A cloud end-to-end regression and a standalone production BE build were not run. The snapshot-window and mixed-version behavior below was assessed from code, without a running-cluster reproduction. ### Notes for Reviewers Local review of `4ba6d0db1480dd018a2e9353cffbb6137fe1c65d` found no Blocker or Major issue. One non-blocking P2 remains for rolling upgrades. | Existing review finding | Current disposition | | --- | --- | | [MF-001](#70218 (comment)): FE notification retries | False positive: this notification is not inserted into `AgentTaskQueue`. | | [MF-002](#70218 (comment)): stale MoW snapshot | Documented behavior tradeoff of the existing mode; the new-P1 classification is withdrawn under the intended load semantics. | | [MF-003](#70218 (comment)): old-BE signature leak | P2 remains for a new-FE/old-BE rolling-upgrade window. | | [MF-004](#70218 (comment)): protected test access | False positive: BE UT uses `-fno-access-control`, and the tests compiled and passed. | **Snapshot semantics:** Even with initialization synchronization enabled, `sync_rowsets`, local snapshot construction and the later flush are not atomic with other transactions' commits. A key absent from the captured snapshot can already enter the new-key validation path and fail under ERROR or for an omitted non-nullable column without a default. Disabling the initial synchronization can enlarge the freshness window, including while an earlier MS-committed rowset is still absent locally. This PR deliberately makes that existing mode the default; master already configures Cloud p0/p1 to use it. This is an explicit behavior/cost tradeoff, not a claim that local snapshots always contain the latest committed keys. For APPEND updates that reach publication, delete-bitmap calculation synchronizes missing versions and realigns conflicting rows from historical columns; flush-time default/null fill alone does not establish final data loss. **Rolling upgrades:** Enabling the new FE notification default while an old BE supports this task but lacks the cleanup guard exposes the old BE signature leak, including when its local notification setting is false. That process retains the signatures until it is restarted/upgraded. Keep FE notifications disabled until all receiving BEs include the cleanup fix. ### Release note Cloud committed-rowset notifications are enabled by default through enable_notify_be_after_load_txn_commit on FE and enable_cloud_make_rs_visible_on_be on BE. Completed notifications release their task signatures, and consumed empty-rowset entries are removed promptly. The BE setting cloud_mow_sync_rowsets_when_load_txn_begin now defaults to false, so cloud MoW loads skip forced rowset synchronization at initialization unless it is explicitly enabled. ### Check List (For Author) - Behavior changed: Yes (see Details). - Does this need documentation: Yes; document all three default-value changes in the configuration reference and release notes. - Pre-submission status: Partially verified: the implementation passed the targeted BE/FE tests and the final pipeline cleanup passed static effective-configuration checks. Cloud end-to-end regression and a standalone production BE build were not run; clang-tidy was excluded as requested. The local review has no Blocker/Major finding, with the rolling-upgrade P2 and snapshot behavior documented above. ### AI Disclosure - Tool: Codex (GPT-6). - Generated: Code, tests and this description. - Human review: [@gavinchou previously approved implementation head `17d42f6ef18`](#70218 (review)); GitHub dismissed that approval after the pipeline cleanup commit. The final head has been reviewed locally.
…faults (apache#70218) After a cloud load transaction commits in Meta Service, FE can notify the participating BEs of its visible versions. BE uses cached committed rowsets or lightweight empty-rowset entries to update local tablet metadata, with normal metadata synchronization as the fallback. Issue Number: None Related PR: apache#59754 (introduced the notification path), apache#70209 (fixes the unrelated Parquet unit-test build failure and is now included in master) PR Type: Bugfix Problem Summary: Completed notification tasks retain their registered signatures, including when the BE feature is disabled. Consumed empty-rowset markers also remain cached because removal only covers the separate non-empty-rowset map. Release signatures on every callback exit and represent both rowset kinds in one cache so removal and expiration share the same lifecycle. Enable the existing FE and BE notification settings by default, and default cloud_mow_sync_rowsets_when_load_txn_begin to false so cloud MoW loads skip forced synchronization when they begin. Remove the six now-redundant FE/BE overrides from the Cloud p0/p1 pipeline configs so those pipelines exercise the new defaults with unchanged effective settings. - **Frequency:** Every accepted notification leaves its signature behind; every consumed empty entry remains until expiration. - **Root cause:** The callback does not pair task registration with removal, and the split empty/non-empty caches have different removal behavior. - **Why existing tests missed it:** They cover insertion and expiration but do not check removal of an empty entry or resubmission after a notification callback completes. - **Fix:** Add a Defer guard before the BE enable check. Store empty rowsets as null metadata in the existing committed-rowset cache, reuse insertion and expiration, and avoid a redundant lookup during cleanup. - **Other paths and branches:** The guard covers enabled notifications, disabled notifications and cache misses. Non-empty and empty committed rowsets now share cleanup; the independent MoW delete-bitmap cache is unchanged. This change targets master. - **Behavior change:** `enable_notify_be_after_load_txn_commit` and `enable_cloud_make_rs_visible_on_be` now default to `true`; `cloud_mow_sync_rowsets_when_load_txn_begin` now defaults to `false`. All three settings retain their existing dynamic mutability. The cloud rowset-builder test fixture now restores the incoming setting rather than hard-coding `true` at teardown. - **Concurrency and lifecycle:** Cache operations retain the existing shared mutex and acquire no new nested locks. Signature cleanup runs after tablet processing has released its locks. Refreshed entries remain protected from older expiration records. - **Compatibility and data correctness:** RPC fields and persistent formats are unchanged. Cloud MoW loads continue to use a local flush snapshot. Skipping initialization synchronization enlarges the existing snapshot freshness window; the partial-update validation and publish-alignment algorithms are unchanged. The rolling-upgrade caveat is described below. - **Observability and cost:** Move per-rowset insertion logging outside the cache lock and lower it to DEBUG as notifications become enabled by default. Existing notification and expiration logs retain transaction/tablet identifiers. - The implementation commits were rebased onto master `87b462e4f58104140f098f5f4a8b5d58bec70149`. The BE/FE runs below tested implementation head `17d42f6ef184c7f851920830ad3ce4861fe679aa`. Final head `4ba6d0db1480dd018a2e9353cffbb6137fe1c65d` adds only the pipeline-configuration cleanup. - On the rebased head, `./run-be-ut.sh --run --filter='CloudCommittedRSMgrTest.*:EnabledAndDisabled/CloudCommittedRowsetTaskTest.*:TaskWorkerPoolTest.*:CloudTabletApplyVisiblePendingTest.*:CloudGroupRowsetBuilderWriterTest.*:*DeltaWriterCancelIntegrationTest.*' -j100` passed all 46 ASAN tests with the requested defaults. - On the same rebased head, `./run-fe-ut.sh --run org.apache.doris.cloud.transaction.CloudGlobalTransactionMgrTest` passed all 56 tests (0 failures, errors or skips); the Maven reactor completed successfully. - The BE build compiled both `task_worker_pool_test.cpp` and `vparquet_writer_test.cpp` successfully. The latter uses the apache#70209 fix already merged into master; no temporary source patch was applied. - Before the original cleanup fix, the empty-entry removal test and both enabled/disabled signature-release tests failed; the expiration-refresh test passed. - `build-support/check-build-hygiene.sh` and `git diff --check` passed after rebase. The unchanged PR patch previously passed clang-format v16/check-format and FE fe-common Maven validate/Checkstyle. - For the final pipeline cleanup, checked all six removed values against the FE/BE defaults, compared the remaining parsed settings in all four files, and ran `git diff --check`: all passed. BE/FE tests were not rerun for this config-only commit because its effective pipeline values are unchanged. - clang-tidy was not rerun, as requested. A cloud end-to-end regression and a standalone production BE build were not run. The snapshot-window and mixed-version behavior below was assessed from code, without a running-cluster reproduction. Local review of `4ba6d0db1480dd018a2e9353cffbb6137fe1c65d` found no Blocker or Major issue. One non-blocking P2 remains for rolling upgrades. | Existing review finding | Current disposition | | --- | --- | | [MF-001](apache#70218 (comment)): FE notification retries | False positive: this notification is not inserted into `AgentTaskQueue`. | | [MF-002](apache#70218 (comment)): stale MoW snapshot | Documented behavior tradeoff of the existing mode; the new-P1 classification is withdrawn under the intended load semantics. | | [MF-003](apache#70218 (comment)): old-BE signature leak | P2 remains for a new-FE/old-BE rolling-upgrade window. | | [MF-004](apache#70218 (comment)): protected test access | False positive: BE UT uses `-fno-access-control`, and the tests compiled and passed. | **Snapshot semantics:** Even with initialization synchronization enabled, `sync_rowsets`, local snapshot construction and the later flush are not atomic with other transactions' commits. A key absent from the captured snapshot can already enter the new-key validation path and fail under ERROR or for an omitted non-nullable column without a default. Disabling the initial synchronization can enlarge the freshness window, including while an earlier MS-committed rowset is still absent locally. This PR deliberately makes that existing mode the default; master already configures Cloud p0/p1 to use it. This is an explicit behavior/cost tradeoff, not a claim that local snapshots always contain the latest committed keys. For APPEND updates that reach publication, delete-bitmap calculation synchronizes missing versions and realigns conflicting rows from historical columns; flush-time default/null fill alone does not establish final data loss. **Rolling upgrades:** Enabling the new FE notification default while an old BE supports this task but lacks the cleanup guard exposes the old BE signature leak, including when its local notification setting is false. That process retains the signatures until it is restarted/upgraded. Keep FE notifications disabled until all receiving BEs include the cleanup fix. Cloud committed-rowset notifications are enabled by default through enable_notify_be_after_load_txn_commit on FE and enable_cloud_make_rs_visible_on_be on BE. Completed notifications release their task signatures, and consumed empty-rowset entries are removed promptly. The BE setting cloud_mow_sync_rowsets_when_load_txn_begin now defaults to false, so cloud MoW loads skip forced rowset synchronization at initialization unless it is explicitly enabled. - Behavior changed: Yes (see Details). - Does this need documentation: Yes; document all three default-value changes in the configuration reference and release notes. - Pre-submission status: Partially verified: the implementation passed the targeted BE/FE tests and the final pipeline cleanup passed static effective-configuration checks. Cloud end-to-end regression and a standalone production BE build were not run; clang-tidy was excluded as requested. The local review has no Blocker/Major finding, with the rolling-upgrade P2 and snapshot behavior documented above. - Tool: Codex (GPT-6). - Generated: Code, tests and this description. - Human review: [@gavinchou previously approved implementation head `17d42f6ef18`](apache#70218 (review)); GitHub dismissed that approval after the pipeline cleanup commit. The final head has been reviewed locally. (cherry picked from commit 08231e3)
…faults (apache#70218) After a cloud load transaction commits in Meta Service, FE can notify the participating BEs of its visible versions. BE uses cached committed rowsets or lightweight empty-rowset entries to update local tablet metadata, with normal metadata synchronization as the fallback. Issue Number: None Related PR: apache#59754 (introduced the notification path), apache#70209 (fixes the unrelated Parquet unit-test build failure and is now included in master) PR Type: Bugfix Problem Summary: Completed notification tasks retain their registered signatures, including when the BE feature is disabled. Consumed empty-rowset markers also remain cached because removal only covers the separate non-empty-rowset map. Release signatures on every callback exit and represent both rowset kinds in one cache so removal and expiration share the same lifecycle. Enable the existing FE and BE notification settings by default, and default cloud_mow_sync_rowsets_when_load_txn_begin to false so cloud MoW loads skip forced synchronization when they begin. Remove the six now-redundant FE/BE overrides from the Cloud p0/p1 pipeline configs so those pipelines exercise the new defaults with unchanged effective settings. - **Frequency:** Every accepted notification leaves its signature behind; every consumed empty entry remains until expiration. - **Root cause:** The callback does not pair task registration with removal, and the split empty/non-empty caches have different removal behavior. - **Why existing tests missed it:** They cover insertion and expiration but do not check removal of an empty entry or resubmission after a notification callback completes. - **Fix:** Add a Defer guard before the BE enable check. Store empty rowsets as null metadata in the existing committed-rowset cache, reuse insertion and expiration, and avoid a redundant lookup during cleanup. - **Other paths and branches:** The guard covers enabled notifications, disabled notifications and cache misses. Non-empty and empty committed rowsets now share cleanup; the independent MoW delete-bitmap cache is unchanged. This change targets master. - **Behavior change:** `enable_notify_be_after_load_txn_commit` and `enable_cloud_make_rs_visible_on_be` now default to `true`; `cloud_mow_sync_rowsets_when_load_txn_begin` now defaults to `false`. All three settings retain their existing dynamic mutability. The cloud rowset-builder test fixture now restores the incoming setting rather than hard-coding `true` at teardown. - **Concurrency and lifecycle:** Cache operations retain the existing shared mutex and acquire no new nested locks. Signature cleanup runs after tablet processing has released its locks. Refreshed entries remain protected from older expiration records. - **Compatibility and data correctness:** RPC fields and persistent formats are unchanged. Cloud MoW loads continue to use a local flush snapshot. Skipping initialization synchronization enlarges the existing snapshot freshness window; the partial-update validation and publish-alignment algorithms are unchanged. The rolling-upgrade caveat is described below. - **Observability and cost:** Move per-rowset insertion logging outside the cache lock and lower it to DEBUG as notifications become enabled by default. Existing notification and expiration logs retain transaction/tablet identifiers. - The implementation commits were rebased onto master `87b462e4f58104140f098f5f4a8b5d58bec70149`. The BE/FE runs below tested implementation head `17d42f6ef184c7f851920830ad3ce4861fe679aa`. Final head `4ba6d0db1480dd018a2e9353cffbb6137fe1c65d` adds only the pipeline-configuration cleanup. - On the rebased head, `./run-be-ut.sh --run --filter='CloudCommittedRSMgrTest.*:EnabledAndDisabled/CloudCommittedRowsetTaskTest.*:TaskWorkerPoolTest.*:CloudTabletApplyVisiblePendingTest.*:CloudGroupRowsetBuilderWriterTest.*:*DeltaWriterCancelIntegrationTest.*' -j100` passed all 46 ASAN tests with the requested defaults. - On the same rebased head, `./run-fe-ut.sh --run org.apache.doris.cloud.transaction.CloudGlobalTransactionMgrTest` passed all 56 tests (0 failures, errors or skips); the Maven reactor completed successfully. - The BE build compiled both `task_worker_pool_test.cpp` and `vparquet_writer_test.cpp` successfully. The latter uses the apache#70209 fix already merged into master; no temporary source patch was applied. - Before the original cleanup fix, the empty-entry removal test and both enabled/disabled signature-release tests failed; the expiration-refresh test passed. - `build-support/check-build-hygiene.sh` and `git diff --check` passed after rebase. The unchanged PR patch previously passed clang-format v16/check-format and FE fe-common Maven validate/Checkstyle. - For the final pipeline cleanup, checked all six removed values against the FE/BE defaults, compared the remaining parsed settings in all four files, and ran `git diff --check`: all passed. BE/FE tests were not rerun for this config-only commit because its effective pipeline values are unchanged. - clang-tidy was not rerun, as requested. A cloud end-to-end regression and a standalone production BE build were not run. The snapshot-window and mixed-version behavior below was assessed from code, without a running-cluster reproduction. Local review of `4ba6d0db1480dd018a2e9353cffbb6137fe1c65d` found no Blocker or Major issue. One non-blocking P2 remains for rolling upgrades. | Existing review finding | Current disposition | | --- | --- | | [MF-001](apache#70218 (comment)): FE notification retries | False positive: this notification is not inserted into `AgentTaskQueue`. | | [MF-002](apache#70218 (comment)): stale MoW snapshot | Documented behavior tradeoff of the existing mode; the new-P1 classification is withdrawn under the intended load semantics. | | [MF-003](apache#70218 (comment)): old-BE signature leak | P2 remains for a new-FE/old-BE rolling-upgrade window. | | [MF-004](apache#70218 (comment)): protected test access | False positive: BE UT uses `-fno-access-control`, and the tests compiled and passed. | **Snapshot semantics:** Even with initialization synchronization enabled, `sync_rowsets`, local snapshot construction and the later flush are not atomic with other transactions' commits. A key absent from the captured snapshot can already enter the new-key validation path and fail under ERROR or for an omitted non-nullable column without a default. Disabling the initial synchronization can enlarge the freshness window, including while an earlier MS-committed rowset is still absent locally. This PR deliberately makes that existing mode the default; master already configures Cloud p0/p1 to use it. This is an explicit behavior/cost tradeoff, not a claim that local snapshots always contain the latest committed keys. For APPEND updates that reach publication, delete-bitmap calculation synchronizes missing versions and realigns conflicting rows from historical columns; flush-time default/null fill alone does not establish final data loss. **Rolling upgrades:** Enabling the new FE notification default while an old BE supports this task but lacks the cleanup guard exposes the old BE signature leak, including when its local notification setting is false. That process retains the signatures until it is restarted/upgraded. Keep FE notifications disabled until all receiving BEs include the cleanup fix. Cloud committed-rowset notifications are enabled by default through enable_notify_be_after_load_txn_commit on FE and enable_cloud_make_rs_visible_on_be on BE. Completed notifications release their task signatures, and consumed empty-rowset entries are removed promptly. The BE setting cloud_mow_sync_rowsets_when_load_txn_begin now defaults to false, so cloud MoW loads skip forced rowset synchronization at initialization unless it is explicitly enabled. - Behavior changed: Yes (see Details). - Does this need documentation: Yes; document all three default-value changes in the configuration reference and release notes. - Pre-submission status: Partially verified: the implementation passed the targeted BE/FE tests and the final pipeline cleanup passed static effective-configuration checks. Cloud end-to-end regression and a standalone production BE build were not run; clang-tidy was excluded as requested. The local review has no Blocker/Major finding, with the rolling-upgrade P2 and snapshot behavior documented above. - Tool: Codex (GPT-6). - Generated: Code, tests and this description. - Human review: [@gavinchou previously approved implementation head `17d42f6ef18`](apache#70218 (review)); GitHub dismissed that approval after the pipeline cleanup commit. The final head has been reviewed locally. (cherry picked from commit 08231e3)
What problem does this PR solve?
Test
1FE+3BE, 400bucket mow table, 10 concurrency stream load
before:


after:
Problem Summary
In cloud storage mode, after a load transaction commits rowsets to Meta Service (MS), BE nodes need to fetch the committed rowset metadata from MS during subsequent sync_rowsets() operations. This introduces additional latency and MS query overhead, especially for high-frequency import scenarios.
This PR implements a notification mechanism where FE/BE directly notifies BE nodes about committed rowsets after transaction commit, allowing BE to update tablet metadata immediately without fetching from MS.
Solution
- Stores rowset meta with expiration time
- Supports efficient lookup by (txn_id, tablet_id)
- Handles empty rowset markers
- Background thread for cleaning expired entries
- When FE commits a load transaction to MS, it sends MakeCloudTmpRsVisibleTask to involved BE nodes
- BE receives the task with final version/visible_ts information
- BE promotes cached rowset metadata to tablet meta directly
- BE can forward notification to FE if needed (for tablets not on the original BE)
- FE: enable_cloud_notify_be_after_load_txn_commit (default: false)
- BE: enable_cloud_make_rs_visible_on_be (default: false)
- BE: cloud_make_committed_rs_visible_worker_count (default: 4)
- BE: cloud_mow_sync_rowsets_when_load_txn_begin (default: true)
Benefits
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)