Skip to content

mcs, tso: fix expected-primary transient marker races #11123

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bufferflies wants to merge 5 commits into
base: master
Choose a base branch
from
+476 −89
Open

mcs, tso: fix expected-primary transient marker races #11123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
01a7af9
mcs: fix expected-primary transient marker races (#11122)
bufferflies Aug 6, 2026
295501d
mcs, tso: verify a transfer landed on new_primary before reporting su…
bufferflies Aug 7, 2026
8e7d0d0
mcs, tso: verify against the resolved target identity, widen stabilit…
bufferflies Aug 10, 2026
bfd0384
mcs, tso: pin the transfer stability window to a single election term
bufferflies Aug 10, 2026
0ee07b2
mcs, tso: drop post-transfer verification, cap marker TTL at 1 lease
bufferflies Aug 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions pkg/mcs/resourcemanager/server/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,10 +214,7 @@ func (s *Server) primaryElectionLoop() {
// transfer target atomically and is cleaned up once this server wins.
func (s *Server) campaignLeader(expectedPrimary string) bool {
log.Info("start to campaign the primary/leader", zap.String("campaign-resource-manager-primary-name", s.participant.Name()))
var cmps []clientv3.Cmp
if cmp := utils.ExpectedPrimaryCmp(&s.participant.MsParam, expectedPrimary); cmp != nil {
cmps = append(cmps, *cmp)
}
cmps := []clientv3.Cmp{utils.ExpectedPrimaryCmp(&s.participant.MsParam, expectedPrimary)}
if err := s.participant.CampaignWithCmps(s.Context(), s.cfg.LeaderLease, cmps...); err != nil {
if err.Error() == errs.ErrEtcdTxnConflict.Error() {
log.Info("campaign resource manager primary meets error due to txn conflict, another server may campaign successfully",
Expand Down Expand Up @@ -245,8 +242,14 @@ func (s *Server) campaignLeader(expectedPrimary string) bool {

// We have won the campaign, so the expected primary flag (if any) has served its
// purpose as the affinity guard. Delete it so steady state is clean and a later
// failure re-elects immediately instead of waiting for the flag's TTL.
utils.DeleteExpectedPrimaryFlag(s.GetClient(), &s.participant.MsParam, expectedPrimary)
// failure re-elects immediately instead of waiting for the flag's TTL. If a newer
// transfer rewrote the flag to another member while we were winning, step down so
// the re-election routes leadership to that target.
if utils.DeleteExpectedPrimaryFlag(s.GetClient(), &s.participant.MsParam, expectedPrimary, s.participant) {
log.Info("the expected primary has been changed to another member, stepping down",
zap.String("server-name", s.Name()))
return false
}
Comment on lines +245 to +252

@coderabbitai coderabbitai Bot Aug 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect the resource manager election loop and every DeleteExpectedPrimaryFlag call site.
set -euo pipefail

ast-grep outline pkg/mcs/resourcemanager/server/server.go --items all

rg -nP -C 12 '\bcampaignLeader\s*\(' pkg/mcs/resourcemanager/server/server.go

rg -nP -C 4 '\bDeleteExpectedPrimaryFlag\s*\(' --type=go

Repository: tikv/pd

Length of output: 5760

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "## server.go primaryElectionLoop and campaignLeader"
sed -n '145,265p' pkg/mcs/resourcemanager/server/server.go | cat -n

echo "## All DeleteExpectedPrimaryFlag references"
rg -n -C 5 'DeleteExpectedPrimaryFlag' --type=go

echo "## campaignLeader return call sites"
rg -n -C 6 'campaignLeader\b|backoff|time.Sleep|Sleep' pkg/mcs/resourcemanager/server/server.go --type=go

echo "## Compare similar loops in scheduler/tso if present"
for f in $(git ls-files | rg 'pkg/.*/.*_server\.go$|pkg/.*/.*server\.go$' | rg -E 'scheduling|tso|mcs/|resources'); do
  [ -f "$f" ] || continue
  if rg -q 'primaryElectionLoop|campaignLeader|ExpectedPrimaryCmp|DeleteExpectedPrimaryFlag' "$f"; then
    echo "### $f"
    rg -n -C 8 'primaryElectionLoop|campaignLeader|ExpectedPrimaryCmp|DeleteExpectedPrimaryFlag' "$f"
  fi
done

Repository: tikv/pd

Length of output: 6958

Add a short sleep before re-campaigning on the step-down path.

campaignLeader returns false here, but primaryElectionLoop immediately returns to the top and starts the next CampaignWithCmps without sleeping. The earlier expected-primary mismatch path sleeps 200ms before re-checking. Since a newer transfer can rewrite the flag while this server campaigned, add the same bounded sleep after this false return to avoid tight-loop re-election attempts.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/mcs/resourcemanager/server/server.go` around lines 245 - 252, The
step-down branch in campaignLeader currently returns false without the delay
used by the earlier expected-primary mismatch path. Add the same bounded 200ms
sleep before returning, so primaryElectionLoop does not immediately re-campaign
after DeleteExpectedPrimaryFlag reports a newer transfer target.


log.Info("triggering the primary callback functions")
for _, cb := range s.primaryCallbacks {
Expand Down
15 changes: 9 additions & 6 deletions pkg/mcs/scheduling/server/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,10 +287,7 @@ func (s *Server) primaryElectionLoop() {
// and to clean the flag up once this server wins.
func (s *Server) campaignPrimary(expectedPrimary string) {
log.Info("start to campaign the primary", zap.String("campaign-scheduling-primary-name", s.participant.Name()))
var cmps []clientv3.Cmp
if cmp := utils.ExpectedPrimaryCmp(&s.participant.MsParam, expectedPrimary); cmp != nil {
cmps = append(cmps, *cmp)
}
cmps := []clientv3.Cmp{utils.ExpectedPrimaryCmp(&s.participant.MsParam, expectedPrimary)}
if err := s.participant.CampaignWithCmps(s.Context(), s.cfg.LeaderLease, cmps...); err != nil {
if err.Error() == errs.ErrEtcdTxnConflict.Error() {
log.Info("campaign scheduling primary meets error due to txn conflict, another server may campaign successfully",
Expand Down Expand Up @@ -318,8 +315,14 @@ func (s *Server) campaignPrimary(expectedPrimary string) {

// We have won the campaign, so the expected primary flag (if any) has served its
// purpose as the affinity guard. Delete it so steady state is clean and a later
// failure re-elects immediately instead of waiting for the flag's TTL.
utils.DeleteExpectedPrimaryFlag(s.GetClient(), &s.participant.MsParam, expectedPrimary)
// failure re-elects immediately instead of waiting for the flag's TTL. If a newer
// transfer rewrote the flag to another member while we were winning, step down so
// the re-election routes leadership to that target.
if utils.DeleteExpectedPrimaryFlag(s.GetClient(), &s.participant.MsParam, expectedPrimary, s.participant) {
log.Info("the expected primary has been changed to another member, stepping down",
zap.String("server-name", s.Name()))
return
}

log.Info("triggering the primary callback functions")
for _, cb := range s.primaryCallbacks {
Expand Down
10 changes: 9 additions & 1 deletion pkg/mcs/utils/constant/constant.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,15 @@ const (
// campaign; if the target never comes up within this window the flag expires and
// the cluster falls back to a free election. Tying it to the leader lease keeps the
// window proportional to how fast leadership turns over.
TransferPrimaryLeaseMultiplier = int64(3)
//
// Kept at 1 (rather than a larger margin) on purpose: this multiplier directly
// bounds the cluster's worst-case unavailable window after a transfer whose target
// never wins a single campaign (down, unreachable, or stuck) - see TransferPrimary's
// doc comment. A bigger multiplier buys the target more slack but linearly extends
// that worst case; 1 lease already matches how long the cluster tolerates a primary
// being unreachable everywhere else (losing its own leader lease), so there is no
// reason for a transfer-induced outage to be allowed to run longer than that.
TransferPrimaryLeaseMultiplier = int64(1)
// PrimaryTickInterval is the interval to check primary
PrimaryTickInterval = 50 * time.Millisecond
// LeaderTickInterval is the interval to check leader
Expand Down
Loading
Loading