Skip to content

[Enhancement][Refactor] Refactor MergeCommitTask lifecycle and add cancellation support#67425

Merged
meegoo merged 8 commits intoStarRocks:mainfrom
banmoy:merge_commit_history_dev
Jan 14, 2026
Merged

[Enhancement][Refactor] Refactor MergeCommitTask lifecycle and add cancellation support#67425
meegoo merged 8 commits intoStarRocks:mainfrom
banmoy:merge_commit_history_dev

Conversation

@banmoy
Copy link
Contributor

@banmoy banmoy commented Jan 4, 2026

Why I'm doing:

  • Improve maintainability: the old MergeCommitTask lifecycle logic was scattered, making state transitions and failure handling hard to reason about.
  • Support explicit cancellation for merge-commit load via CancelStreamLoadAction.

What I'm doing:

  • Refactor MergeCommitTask into a state machine:
    • Introduce TaskState and a strict STATE_TRANSITION map.
    • Update taskState / taskStateMessage / cancelHandler atomically under a single lock.
  • Add cancel support:
    • Add MergeCommitTask.cancel(reason) to transition to CANCELLED.
    • Implement a cancel handler for LOADING that calls Coordinator.cancel(PPlanFragmentCancelReason.USER_CANCEL, reason).
    • Extend AbstractTxnStateChangeCallback; external aborts trigger afterAborted() -> cancel() to unify termination behavior. Then CancelStreamLoadAction can work
  • Metrics/observability improvements:
    • Consolidate metrics updates into updateMetrics() and align success/failure with the final task state.
    • Add tracing/profile collection (kept last since it can be slow) and enrich profile summary info.

Fixes #issue

What type of PR is this:

  • BugFix
  • Feature
  • Enhancement
  • Refactor
  • UT
  • Doc
  • Tool

Does this PR entail a change in behavior?

  • Yes, this PR will result in a change in behavior.
  • No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

  • Interface/UI changes: syntax, type conversion, expression evaluation, display information
  • Parameter changes: default values, similar parameters but with different default values
  • Policy changes: use new policy to replace old one, functionality automatically enabled
  • Feature removed
  • Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

  • I have added test cases for my bug fix or my new feature
  • This pr needs user documentation (for new or modified features or behaviors)
    • I have added documentation for my new feature or new function
    • This pr needs auto generate documentation
  • This is a backport pr

Bugfix cherry-pick branch check:

  • I have checked the version labels which the pr will be auto-backported to the target branch
    • 4.0
    • 3.5
    • 3.4
    • 3.3

Note

Modernizes merge-commit load execution for reliability, observability, and cancelability.

  • Refactors MergeCommitTask into a locked state machine (PENDINGBEGINNING_TXNPLANNINGEXECUTINGCOMMITTINGCOMMITTEDFINISHED/CANCELLED/ABORTED), with atomic transitions and per-state cancel handling (exec stage calls Coordinator.cancel).
  • Integrates with AbstractTxnStateChangeCallback so external aborts trigger task cancellation; centralizes metrics in updateMetrics; enriches runtime profile via Tracers, ProfileManager, and detailed summary fields.
  • Switches to VisibleStateWaiter for publish wait; adds TimeoutWatcher.getLeftTimeoutMillis() to manage remaining budgets; enforces data quality via DataQualityException.
  • MergeCommitJob: resolves DB ID before creating tasks, passes taskId/dbId/warehouseName/backendIds, and uses containsBackend; improves error when DB missing.
  • Minor: thread pool name changed to merge-commit; StreamLoadKvParams.toString() simplified.
  • Tests: large UT overhaul for task lifecycle/cancel/publish timeout, job paths, and new SQL test test_merge_commit_cancel.

Written by Cursor Bugbot for commit 8f6a710. This will update automatically on new commits. Configure here.

@mergify mergify bot assigned banmoy Jan 4, 2026
@banmoy banmoy force-pushed the merge_commit_history_dev branch from e77efb9 to 1eb0f25 Compare January 4, 2026 02:19
@alvin-celerdata
Copy link
Contributor

@cursor review

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no bugs!

@CLAassistant
Copy link

CLAassistant commented Jan 4, 2026

CLA assistant check
All committers have signed the CLA.

@cursor cursor bot force-pushed the merge_commit_history_dev branch 2 times, most recently from 6bb2ade to f623b35 Compare January 4, 2026 07:29
@alvin-celerdata
Copy link
Contributor

@cursor review

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no bugs!

@banmoy banmoy force-pushed the merge_commit_history_dev branch from f623b35 to 52b59e5 Compare January 5, 2026 08:32
@alvin-celerdata
Copy link
Contributor

@cursor review

@banmoy banmoy force-pushed the merge_commit_history_dev branch from 52b59e5 to f1d96f7 Compare January 6, 2026 01:56
@alvin-celerdata
Copy link
Contributor

@cursor review

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no bugs!

@banmoy banmoy force-pushed the merge_commit_history_dev branch from f1d96f7 to 1e29731 Compare January 6, 2026 12:02
@alvin-celerdata
Copy link
Contributor

@cursor review

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no bugs!

@banmoy banmoy force-pushed the merge_commit_history_dev branch 2 times, most recently from 6ac4178 to 5ac5d42 Compare January 7, 2026 11:19
@alvin-celerdata
Copy link
Contributor

@cursor review

@banmoy banmoy force-pushed the merge_commit_history_dev branch from 5ac5d42 to 38392c2 Compare January 8, 2026 01:38
@alvin-celerdata
Copy link
Contributor

@cursor review

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no bugs!

Signed-off-by: PengFei Li <lpengfei2016@gmail.com>
Signed-off-by: PengFei Li <lpengfei2016@gmail.com>
Signed-off-by: PengFei Li <lpengfei2016@gmail.com>
Signed-off-by: PengFei Li <lpengfei2016@gmail.com>
Signed-off-by: PengFei Li <lpengfei2016@gmail.com>
Signed-off-by: PengFei Li <lpengfei2016@gmail.com>
Signed-off-by: PengFei Li <lpengfei2016@gmail.com>
Signed-off-by: PengFei Li <lpengfei2016@gmail.com>
@banmoy banmoy force-pushed the merge_commit_history_dev branch from 38392c2 to 8f6a710 Compare January 11, 2026 02:10
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@github-actions
Copy link
Contributor

[Java-Extensions Incremental Coverage Report]

pass : 0 / 0 (0%)

@github-actions
Copy link
Contributor

[BE Incremental Coverage Report]

pass : 0 / 0 (0%)

@github-actions
Copy link
Contributor

[FE Incremental Coverage Report]

pass : 345 / 369 (93.50%)

file detail

path covered_line new_line coverage not_covered_line_detail
🔵 com/starrocks/load/batchwrite/MergeCommitTask.java 330 354 93.22% [369, 370, 371, 372, 385, 386, 387, 421, 423, 473, 474, 475, 540, 598, 599, 600, 601, 651, 652, 653, 742, 887, 905, 908]
🔵 com/starrocks/load/streamload/StreamLoadKvParams.java 1 1 100.00% []
🔵 com/starrocks/common/TimeoutWatcher.java 1 1 100.00% []
🔵 com/starrocks/load/batchwrite/MergeCommitJob.java 13 13 100.00% []

@alvin-celerdata
Copy link
Contributor

@cursor review

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no bugs!

@meegoo meegoo merged commit 72eb5fa into StarRocks:main Jan 14, 2026
136 of 140 checks passed
@github-actions github-actions bot added the 4.0 label Jan 14, 2026
banmoy added a commit to banmoy/starrocks that referenced this pull request Jan 14, 2026
…ncellation support (StarRocks#67425)

Signed-off-by: PengFei Li <lpengfei2016@gmail.com>
meegoo pushed a commit that referenced this pull request Jan 14, 2026
…ncellation support (backport #67425) (#67877)

Signed-off-by: PengFei Li <lpengfei2016@gmail.com>
farhad-celo pushed a commit to farhad-celo/starrocks that referenced this pull request Jan 20, 2026
…ncellation support (StarRocks#67425)

Signed-off-by: PengFei Li <lpengfei2016@gmail.com>
Signed-off-by: Farhad Shahmohammadi <f.shahmohammadi@celonis.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants