branch-4.1: [fix](cloud) Correct cumulative point advancement when enable parallel cumulative compaction (#65915) - #67425
Conversation
…l cumulative compaction (apache#65915) When enable parallel cumulative compaction, the cu compaction with higher version range may finish before the cu compaction with lower range. For example, cu1: v10-v20, cu2: v21-v30. The cu2 may finish before cu1. When cu2 finish, it may set cu point to v31, does not consider if cu1 finish or fail. This pr correct the cu point advancement. related to apache#64619
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
There was a problem hiding this comment.
🔵 Needs a closer look
The change touches core compaction correctness across BE/meta-service with parallel/concurrent behavior, so it warrants final human verification despite tests and only minor review nits found.
Pull request overview
Backports the cloud compaction fix from #65915 to ensure cumulative_point is advanced safely when parallel cumulative compactions can finish out of order, adding validation/guardrails in meta-service and tightening BE-side application rules with expanded test coverage.
Changes:
- Add meta-service logic to validate/ignore stale cumulative-point proposals and update tablet stats more defensively during compaction FINISH.
- Update BE compaction flows to attach compaction counter snapshots and only apply cumulative results when the response counter is the expected “next” value (otherwise force a tablet sync).
- Add/extend unit tests covering legacy vs current behavior, parallel ordering, counter snapshot handling, and sync-forcing scenarios.
File summaries
| File | Description |
|---|---|
| cloud/test/meta_service_job_test.cpp | Adds helpers and multiple regression tests for parallel/legacy cumulative-point advancement and counter validation. |
| cloud/src/meta-service/meta_service_job.cpp | Introduces cumulative-point acceptance logic and adds validation around cumulative counter presence in FINISH. |
| be/test/storage/compaction/cloud_index_change_compaction_test.cpp | Adds test ensuring base compaction response with newer cumulative point forces a sync (without mutating local point). |
| be/test/cloud/cloud_compaction_test.cpp | Adds comprehensive tests for applying cumulative results, parallel pick behavior, delete handling, and sync triggering. |
| be/src/storage/compaction/compaction.h | Declares CloudCompactionMixin::should_apply_cumulative_compaction_result (header-lock required). |
| be/src/storage/compaction/compaction.cpp | Implements the “apply only next cumulative result, else force sync” gating logic. |
| be/src/cloud/cloud_index_change_compaction.cpp | Includes counter snapshots in jobs and uses the new gating logic for cumulative result application; forces sync on newer point from base response. |
| be/src/cloud/cloud_cumulative_compaction.h | Adds parallel-mode state/snapshots and updates cumulative-point update API. |
| be/src/cloud/cloud_cumulative_compaction.cpp | Implements conflict-window-aware picking/point advancement, stable execution mode snapshotting, and counter snapshot propagation. |
| be/src/cloud/cloud_base_compaction.cpp | Forces sync when meta-service reports a newer cumulative point after base compaction. |
Review details
- Files reviewed: 10/10 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| compaction.type() != TabletCompactionJobPB::EMPTY_CUMULATIVE) { | ||
| return true; | ||
| } | ||
| // Safe because tablet stats keep max(current, proposal). |
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
pick #65915