maintainer: prevent stale spans from reentering scheduler state (#6073) - #6091
maintainer: prevent stale spans from reentering scheduler state (#6073)#6091ti-chi-bot wants to merge 1 commit into
Conversation
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
|
This cherry pick PR is for a release branch and has not yet been approved by triage owners. To merge this cherry pick:
DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@wk989898 This PR has conflicts, I have hold it. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@ti-chi-bot: ## If you want to know how to resolve it, please read the guide in TiDB Dev Guide. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
This is an automated cherry-pick of #6073
What problem does this PR solve?
Issue Number: close #6072
What is changed and how it works?
This PR prevents a stale SpanReplication from being reintroduced into the scheduler’s Absent set after it has already been removed, replaced, or rebound to another node.
Previously, terminal dispatcher status handling performed the following operations separately:
A concurrent split could call ReplaceReplicaSet between these operations. The split removed the old span and created its replacement spans, but the terminal-status handler still held a pointer to the old span. Because MarkSpanAbsent did not verify that the span was still part of the controller’s desired state, it inserted the obsolete span into the scheduler’s Absent set.
This created a ghost Absent span: the scheduler repeatedly tried to create an Add operator for a dispatcher that no longer existed in the span controller, resulting in continuous add operator failed, span not found errors and a stalled checkpoint.
This PR makes the transition safe in two ways:
MarkSpanAbsent now checks, while holding the span controller mutex, that the dispatcher ID is still registered in allTasks and that the registered value is the exact same SpanReplication instance. If the span has already been removed or replaced, the operation returns without changing scheduler or checkpoint-tracker state.
The terminal-status fallback uses MarkSpanAbsentIfCurrent, which additionally verifies under the same lock that the span is still bound to the node that reported the terminal status. This prevents a delayed Stopped status from an old owner from marking a span Absent after it has already been moved to another node.
ReplaceReplicaSet and the new validation use the same span controller mutex. Therefore, removal/replacement and the Absent transition now have a deterministic order:
ReplaceReplicaSet first
-> old span is no longer the current task
-> MarkSpanAbsent is skipped
MarkSpanAbsent first
-> the span is still current and owned by the reporting node
-> the valid Absent transition completes
The terminal-status warning is now emitted only when the span is actually transitioned to Absent. Stale terminal statuses that lose the atomic validation are ignored without producing a misleading state-change log.
Check List
Tests
Questions
Will it cause performance regression or break compatibility?
Do you need to update user documentation, design documentation or monitoring documentation?
Release note
Summary by CodeRabbit