scheduler, worker(dm): check stages of all subtasks before updating

[joechenrh]

What problem does this PR solve?

Issue Number: close #12815

Scheduler.UpdateSubTasks only checked the expected stage of cfgs[0], but wrote the configs of all sources to etcd. For a task with several sources, an update was accepted while a source other than the first was still running. The etcd config then diverged from the config used by the running syncer until the subtask was recreated.

The check could also pass silently: GetExpectSubTaskStage returns InvalidStage when it cannot acquire the task latch, and InvalidStage != Running.

What is changed and how it works?

• UpdateSubTasks holds the task latch for the whole update. If another operation on the same task holds it, the update fails with ErrSchedulerLatchInUse instead of reading a stage that is being changed.
• The expected stage of every subtask in the request must be Paused or Stopped.
• The expected stage in etcd is written by the stop request before the worker actually pauses the subtask, so SubTask.CheckUnitCfgCanUpdate also requires the subtask to be Paused. SubTask.Update already had this requirement, so the check now fails before the new config is written to etcd.

The scheduler lock scope is not changed here, see #12816.

Check List

Tests

• Unit test
  • TestUpdateSubTasksChecksStageOfAllSources: a running source is rejected in either request order, a held task latch is rejected, and the update succeeds once all sources are stopped. Fails on master.
  • TestCheckUnitCfgCanUpdateRequiresPaused: the worker check rejects every stage other than Paused. Fails on master.
• go test ./dm/master/ ./dm/master/scheduler/ ./dm/worker/ passes with failpoints enabled.

Questions

Will it cause performance regression or break compatibility?

No performance impact. An update issued right after a stop request, before the worker has paused the subtask, is now rejected, and can be retried once the task status shows Paused.

Do you need to update user documentation, design documentation or monitoring documentation?

No.

Release note

Fix the issue that updating a DM task through OpenAPI might change the configuration of a subtask that is still running.

🤖 Generated with Claude Code

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign d3hunter for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• dm/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

Warning

Review limit reached

Next included review available in 37 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: bfed4231-6510-4992-bddd-8f0bd809be75

📥 Commits

Reviewing files that changed from the base of the PR and between 03ed243 and 06d5fda.

📒 Files selected for processing (4)

• dm/master/scheduler/scheduler.go
• dm/master/scheduler/scheduler_test.go
• dm/worker/subtask.go
• dm/worker/subtask_test.go

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

❌ Patch coverage is 94.44444% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 54.0883%. Comparing base (03ed243) to head (06d5fda).
✅ All tests successful. No failed tests found.

❌ Your project check has failed because the head coverage (54.0883%) is below the target coverage (60.0000%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

Components	Coverage Δ
cdc	57.4490% <ø> (∅)
dm	50.8879% <94.4444%> (∅)
engine	50.6941% <ø> (+0.0395%)	⬆️

Flag	Coverage Δ
cdc	57.4490% <ø> (?)
unit	54.0883% <94.4444%> (+3.4337%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@@               Coverage Diff                @@
##             master     #12862        +/-   ##
================================================
+ Coverage   50.6546%   54.0883%   +3.4337%     
================================================
  Files           213       1013       +800     
  Lines         17720     141818    +124098     
================================================
+ Hits           8976      76707     +67731     
- Misses         8178      59357     +51179     
- Partials        566       5754      +5188     

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[joechenrh]

/retest

[joechenrh]

/check-issue-triage-complete