bobhan1 opened a new pull request, #70218:
URL: https://github.com/apache/doris/pull/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 (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 #70209 fix for an unrelated master 
compilation failure in vparquet_writer_test.cpp. The temporary change was 
removed afterward and is not part of this PR.
   - 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)
   
   - 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. Local BE UT validation used the #70209 prerequisite, which is not 
included in this PR.
   
   ### AI Disclosure
   
   - Tool: Codex (GPT-6).
   - Generated: Code, tests and this description.
   - Human review: Pending.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to