feat: cancel local compaction for enter staging#7885
feat: cancel local compaction for enter staging#7885WenyXu wants to merge 1 commit intoGreptimeTeam:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request implements cooperative cancellation for local compaction tasks in the mito2 engine, allowing EnterStaging requests to preempt running compactions instead of waiting in the pending queue. The changes introduce task ID tracking in the CompactionScheduler, add cancellation checkpoints within the compaction runner, and update the worker loop to handle cancellation notifications. Feedback identifies a critical issue in the on_compaction_cancelled logic where waiters of a cancelled task might hang if a pending compaction is immediately scheduled without notifying them first.
| if self | ||
| .handle_pending_compaction_request( | ||
| region_id, | ||
| manifest_ctx, | ||
| schema_metadata_manager.clone(), | ||
| ) | ||
| .await | ||
| { | ||
| return Vec::new(); | ||
| } |
There was a problem hiding this comment.
Similar to on_compaction_finished, if there are no pending DDLs and a pending compaction is scheduled, the waiters for the cancelled compaction are not notified. This will cause the waiters to hang.
You should notify the waiters before handling the pending compaction.
| if self | |
| .handle_pending_compaction_request( | |
| region_id, | |
| manifest_ctx, | |
| schema_metadata_manager.clone(), | |
| ) | |
| .await | |
| { | |
| return Vec::new(); | |
| } | |
| if status.pending_request.is_some() { | |
| // The current compaction task is cancelled. We must notify its waiters. | |
| finish_compaction_waiters(std::mem::take(&mut status.waiters)); | |
| if self | |
| .handle_pending_compaction_request( | |
| region_id, | |
| manifest_ctx, | |
| schema_metadata_manager.clone(), | |
| ) | |
| .await | |
| { | |
| // A new compaction is scheduled. Pending DDLs (if any) will wait for it. | |
| return Vec::new(); | |
| } | |
| } |
Signed-off-by: WenyXu <wenymedia@gmail.com>
f50f9d4 to
0a60db0
Compare
I hereby agree to the terms of the GreptimeDB CLA.
Refer to a related PR or issue link (optional)
What's changed and what's your intention?
PR Checklist
Please convert it to a draft if some of the following conditions are not met.