Skip to content

dispatcher: rebuild closed managers during maintainer failover - #6674

Merged
ti-chi-bot[bot] merged 2 commits into
pingcap:masterfrom
3AceShowHand:dispatcher-manager-stale-stuck
Oct 26, 2026
Merged

ti-chi-bot[bot] merged 2 commits into
pingcap:masterfrom
3AceShowHand:dispatcher-manager-stale-stuck

Conversation

@3AceShowHand

@3AceShowHand 3AceShowHand commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

What problem does this PR solve?

Issue Number: close #6519

What is changed and how it works?

  • Detect closing or closed DispatcherManagers before reusing them during bootstrap. Keep bootstrap pending until cleanup completes, then remove the old manager and create a new one through the existing initialization path.
  • Record the closing maintainer epoch when a close request is accepted, preserving stale-bootstrap fencing and epoch-0 removal tombstones.
  • Add regression coverage for healthy-manager handover, closing/closed-manager recovery, stale epochs, and epoch-0 compatibility and removal.

Check List

Tests

  • Unit test

Passed locally:

go test -race -tags=intest ./downstreamadapter/dispatcherorchestrator \
  -run '^TestHandleBootstrapRequest(CanCreateDispatcherAfterHandover|RejectsStaleEpochAfterManagerClose)$' -count=10

go test -race -tags=intest ./downstreamadapter/dispatchermanager ./downstreamadapter/dispatcherorchestrator -count=1

Kafka integration tests were not run.

Questions

Will it cause performance regression or break compatibility?

No performance regression or compatibility break is expected. Changes are limited to bootstrap and close handling. Healthy-manager handover, existing epoch fencing, and epoch-0 compatibility are preserved. No configuration or protocol changes are introduced.

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

No documentation updates are needed.

Release note

Fix a changefeed checkpoint stall caused by reusing a closing or closed dispatcher manager after maintainer failover.

@ti-chi-bot ti-chi-bot Bot added do-not-merge/needs-linked-issue release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Oct 23, 2026
@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Warning

Review limit reached

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

Next included review available in 17 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b98dc1ad-799d-4ea7-a840-92627462a163
📥 Commits

Reviewing files that changed from the base of the PR and between a45d45f and 224008d.

📒 Files selected for processing: 4
  • downstreamadapter/dispatchermanager/dispatcher_manager.go
  • downstreamadapter/dispatchermanager/dispatcher_manager_test.go
  • downstreamadapter/dispatcherorchestrator/dispatcher_orchestrator.go
  • downstreamadapter/dispatcherorchestrator/dispatcher_orchestrator_test.go

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 75801b0d-bf55-497d-b312-b743d7dcef52
📥 Commits

Reviewing files that changed from the base of the PR and between 1a9de2d and a45d45f.

📒 Files selected for processing: 4
  • downstreamadapter/dispatchermanager/dispatcher_manager.go
  • downstreamadapter/dispatchermanager/dispatcher_manager_test.go
  • downstreamadapter/dispatcherorchestrator/dispatcher_orchestrator.go
  • downstreamadapter/dispatcherorchestrator/dispatcher_orchestrator_test.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The dispatcher manager now reports whether shutdown has started or completed. The orchestrator uses closing state and maintainer epochs when it handles bootstrap and close requests. Added tests cover manager handovers, cleanup, and stale epochs.

Changes: Dispatcher manager handover

Layer / File(s) Summary
Manager closing state
downstreamadapter/dispatchermanager/dispatcher_manager.go, downstreamadapter/dispatchermanager/dispatcher_manager_test.go
Added IsClosing() to report whether the manager is closing or closed. The local fence test checks this state before and after close attempts.
Bootstrap and close handling
downstreamadapter/dispatcherorchestrator/dispatcher_orchestrator.go, downstreamadapter/dispatcherorchestrator/dispatcher_orchestrator_test.go
Bootstrap checks maintainer eligibility and attempts to close a closing manager before removing it and continuing. Close handling records the maintainer epoch before attempting to close. Tests cover handovers and stale-epoch requests.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: hongyunyan

Merge Risk: ⚪ Minimal · up to a45d4

The handover can complete after cleanup, and stale requests cannot recreate the old manager. No issue identified here needs resolution before merge.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title names the main change: closed or closing dispatcher managers are rebuilt during maintainer failover, which matches the orchestrator bootstrap changes and the new regression tests. It is spec…
Description check Passed The description follows the template. It includes the issue link (close #6519), a 'What is changed and how it works' section with three concrete bullets, unit test evidence with exact commands, answer…
Full details: Title check

Explanation

The title names the main change: closed or closing dispatcher managers are rebuilt during maintainer failover, which matches the orchestrator bootstrap changes and the new regression tests. It is specific and short, with no file list or vague terms. The title does not mention the stale-epoch fencing or the close-epoch recording change, but those are secondary.

Full details: Description check

Explanation

The description follows the template. It includes the issue link (close #6519), a 'What is changed and how it works' section with three concrete bullets, unit test evidence with exact commands, answers to both performance/compatibility and documentation questions, and a release note. The Integration test item is not checked, and the summary states that Kafka integration tests were not run. The description does not mention the new IsClosing method, but the required sections are complete.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

A rabbit checks the closing gate,
Then waits while cleanup sets things straight.
New epochs find their proper place,
A fresh dispatcher joins the race.
The burrow hums; the handover’s done!

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

@ti-chi-bot ti-chi-bot Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 23, 2026
@3AceShowHand 3AceShowHand changed the title dispatcherManager: dispatcher: rebuild closed managers during maintainer failover Oct 23, 2026
@ti-chi-bot ti-chi-bot Bot added needs-cherry-pick-release-nextgen-202609 Should cherry pick this PR to release-nextgen-202609 branch. and removed do-not-merge/needs-linked-issue labels Oct 23, 2026
@3AceShowHand

Copy link
Copy Markdown
Collaborator Author

/test all

@3AceShowHand 3AceShowHand added the needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. label Oct 23, 2026
@ti-chi-bot ti-chi-bot Bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels Oct 23, 2026
Comment thread downstreamadapter/dispatchermanager/dispatcher_manager.go Outdated
@3AceShowHand

Copy link
Copy Markdown
Collaborator Author

/test all

@ti-chi-bot ti-chi-bot Bot added the lgtm label Oct 23, 2026
@ti-chi-bot

ti-chi-bot Bot commented Oct 23, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lidezhu, wk989898

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@ti-chi-bot ti-chi-bot Bot removed the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Oct 23, 2026
@ti-chi-bot

ti-chi-bot Bot commented Oct 23, 2026

Copy link
Copy Markdown

[LGTM Timeline notifier]

Timeline:

  • 2026-10-23 08:16:56.016709631 +0000 UTC m=+103753.508421880: ☑️ agreed by lidezhu.
  • 2026-10-23 08:59:13.37702712 +0000 UTC m=+106290.868739378: ☑️ agreed by wk989898.

@3AceShowHand

Copy link
Copy Markdown
Collaborator Author

/retest

1 similar comment
@3AceShowHand

Copy link
Copy Markdown
Collaborator Author

/retest

@ti-chi-bot
ti-chi-bot Bot merged commit c0b0e58 into pingcap:master Oct 26, 2026
40 checks passed
@ti-chi-bot

Copy link
Copy Markdown
Member

In response to a cherrypick label: new pull request created to branch release-8.5: #6714.

@ti-chi-bot

Copy link
Copy Markdown
Member

In response to a cherrypick label: new pull request created to branch release-nextgen-202609: #6715.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved lgtm needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. needs-cherry-pick-release-nextgen-202609 Should cherry pick this PR to release-nextgen-202609 branch. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stale DispatcherManager caused checkpointTS stuck.

4 participants