EmergencyReparentShard: filter lagging candidates from relaylog wait, handle partial failures and split-brain

[timvaillancourt]

Description

After StopReplicationAndGetStatus completes during ERS, replication is stopped on all tablets and their Combined (relay log) positions are frozen. Today ERS waits for all tablets to apply relay logs before picking a winner — but a tablet whose Combined position is strictly behind the leading group can never win. Waiting for it is pointless and can only hurt us (by timing out the entire ERS over a tablet that was never a contender)

Filtering out known-problem tablets early make reparents less brittle and faster 🚀

This PR address two major gaps in ERS (#18529 and #20199) by making ERS more tolerant of lagging tablets and better at detect split-brains, while preserving the AGENTS.md / CLAUDE.md contracts that we must pick the most-advanced candidate with certainty and must error out when that candidate isn't clear

What changed

1. GTID-only relay-log-apply optimisation — for GTID-based replication, Combined is the received relay-log position, so we can prove which tablets can never win and filter them out of the wait. Tablets at the leading Combined position are waited on; the first success short-circuits the rest. Lagging tablets stay in the candidate pool (they can still be repointed at the new primary later), they just don't block the wait phase. For non-GTID flavours (FilePos, MariaDB) this optimisation is unsafe — Combined there is the executed position and doesn't reflect received-but-not-applied relay logs — so those flavours keep the pre-PR requireAll=true behaviour unchanged

2. Fail-fast on suspected split-brain — filterToMostAdvancedCombined() uses pairwise dominance (not "compare to a single max") so it correctly handles partially-ordered GTID sets. If the leading group has incomparable Combined positions (two tablets with disjoint UUIDs, neither dominating the other) we now abort upfront with FAILED_PRECONDITION and a list of the diverged tablets, rather than silently pick one side. The filter + uniform check + metric increment are consolidated in a new filterAndCheckUniform() helper so both call sites (first wait and errant-GTID re-wait) get identical semantics

3. Errant-GTID re-wait pass — errant filtering can remove every originally-applied tablet, leaving only "unwaited survivors" (tablets cancelled mid-apply by our own short-circuit, or strictly-behind tablets we excluded from the first wait). Promoting one of those would risk a primary with received-but-unapplied transactions 😱 We detect this case (no applied tablet survives errant detection) and run a second filterAndCheckUniform → wait pipeline over the survivors before promotion. The uniformity re-check is mandatory — the survivor set is a different shape than the original leading group and could have its own split-brain signal

4. --allow-split-brain-promotion operator escape hatch — by default the upfront uniformCombined check aborts ERS with FAILED_PRECONDITION on incomparable Combined positions, which is the safe behaviour but can leave a shard without a primary in genuine split-brain scenarios where the operator already knows which side to keep. Setting --allow-split-brain-promotion=true (off by default) converts the abort into a WARN log naming both diverged tablets + their positions, increments a new EmergencyReparentSplitBrainOverrides counter, and lets ERS proceed. The losing side's unique GTIDs become errant after promotion — the flag is a deliberate operator override, not a default. Flag is wired through the proto (field 9 on EmergencyReparentShardRequest), vtctldclient (--allow-split-brain-promotion), legacy vtctl (--allow_split_brain_promotion), and the EmergencyReparentOptions struct. The findMostAdvanced secondary split-brain check honours the same flag for consistency

Implementation details

A few smaller mechanics behind the headline changes:

• applyRelayLogsAndReconcile() helper — applied tablets get Executed bumped to Combined so the existing sorter prefers them via the Combined → Executed → promotion-rule tiebreak; failed tablets are removed from validCandidates, cancelled ones are left untouched
• Pre-satisfied PRIMARY-like candidates — a tablet absent from statusMap (current/stuck PRIMARY from ErrNotReplica) has no relay logs to apply, so it's marked true in successMap upfront. This bumps its Executed = Combined in reconcile, preventing cancelled-mid-apply peers (non-zero pre-wait Executed) from sorting ahead of it in findMostAdvanced
• Cancellation classification — errors from our own groupCancel() or from a cancelled parent context are treated as expected noise (must check both errors.Is(err, context.Canceled) and gRPC-wrapped vtrpc.Code_CANCELED, since the latter does not satisfy the former). When the parent ctx is cancelled with no real failure to surface, we return the wrapped ctx.Err() directly so operators see "aborted while waiting for relay logs to apply"

Stats

Three new stats are exported for observability:

• EmergencyReparentFilteredCandidates{Keyspace, Shard} — tablets excluded from the relay-log wait because their Combined position is strictly behind the leading group. A single ERS run may increment this twice if errant-GTID detection forces a second wait pass over the surviving candidates
• EmergencyReparentRelayLogFailedCandidates{Keyspace, Shard} — tablets that genuinely failed to apply relay logs (RPC error, MySQL error, or timeout). Cancellations after a peer succeeded — or after parent-ctx cancel — are not counted. The metric is also incremented on the error path before any abort returns, so operators can still see failure counts when ERS aborts for some other reason
• EmergencyReparentSplitBrainOverrides{Keyspace, Shard} — ERS runs that proceeded despite incomparable Combined positions because --allow-split-brain-promotion was set. Always zero unless an operator has deliberately invoked the escape hatch

Testing

E2E tests in ers_test.go:

• TestERSFiltersNonMostAdvancedCandidates — stops the IO thread on one replica, writes data the lagger won't see, kills the primary, runs ERS, then asserts EmergencyReparentFilteredCandidates incremented via the vtctld /debug/vars endpoint and that the lagging tablet was not chosen as the new primary
• TestERSSplitBrainDetection — detaches two replicas and writes to each independently to produce two-sided GTID divergence, then asserts ERS aborts with the upfront "suspected split-brain" error naming both diverged tablets
• TestReplicationStopped updated — previously asserted ERS failed with 2 replicas having replication stopped; now asserts ERS succeeds with the third tablet as the surviving candidate, since partial relay-log failures are tolerated
• All three new behavioural tests are gated on e2eutils.SkipIfBinaryIsBelowVersion(t, 25, "vtctld") so the Reparent Old Vtctl upgrade-downgrade job (which runs against the previous release's vtctld) skips them cleanly

Related Issue(s)

Resolves: #18529
Resolves: #20199

Checklist

• "Backport to:" labels have been added if this change should be back-ported to release branches
• If this change is to be back-ported to previous releases, a justification is included in the PR description
• Tests were added or are not required
• Did the new or modified tests pass consistently locally and on CI?
• Documentation was added or is not required

Deployment Notes

This is a user-visible behavioural change in EmergencyReparentShard and is called out in changelog/25.0/25.0.0/summary.md under VTCtld. No new required flags or configuration — the one new flag is an opt-in operator escape hatch:

1. GTID-based shards — ERS now tolerates a minority of replicas failing or hanging during the relay-log-apply wait. As long as at least one tablet at the leading Combined position applies successfully (or a no-status PRIMARY-like tablet is present at that position), ERS proceeds. The pre-PR behaviour (any single failure aborts ERS) was unnecessarily fragile

2. Non-GTID shards (FilePos, MariaDB) — behaviour is unchanged. The optimisation is gated on isGTIDBased and these flavours still wait for every candidate

3. Suspected split-brain — when the leading GTID candidates have incomparable Combined positions, ERS now aborts upfront with a FAILED_PRECONDITION error naming the diverged tablets, rather than silently picking one side. This is a stricter check than before and may surface as a new failure mode on shards that previously survived a split-brain scenario by luck. The pre-PR behaviour was silent data loss + errant GTIDs on the losing side — this PR converts that invisible data-integrity incident into a visible operational one. See Bug Report: EmergencyReparentShard silently promotes one side in a split-brain, leaving errant GTIDs #20199 for the bug

4. --allow-split-brain-promotion (off by default) — operators who deliberately need to force ERS through a detected split-brain (eg: they know which side to keep, plan to manually re-clone the losing side after) can set this flag. ERS will log a WARN naming the diverged tablets + their positions, increment EmergencyReparentSplitBrainOverrides, and proceed. The losing side's unique GTIDs will become errant after promotion — the flag is an override, not a default. Available as --allow-split-brain-promotion (vtctldclient) and --allow_split_brain_promotion (legacy vtctl)

Three new stats (EmergencyReparentFilteredCandidates, EmergencyReparentRelayLogFailedCandidates, EmergencyReparentSplitBrainOverrides) are exported from vtctld for operators to track filter, failure, and split-brain-override counts per keyspace/shard

AI Disclosure

Claude Code assisted with development, testing and this PR summary

[vitess-bot]

Review Checklist

Hello reviewers! 👋 Please follow this checklist when reviewing this Pull Request.

General

• Ensure that the Pull Request has a descriptive title.
• Ensure there is a link to an issue (except for internal cleanup and flaky test fixes), new features should have an RFC that documents use cases and test cases.

Tests

• Bug fixes should have at least one unit or end-to-end test, enhancement and new features should have a sufficient number of tests.

Documentation

• Apply the release notes (needs details) label if users need to know about this change.
• New features should be documented.
• There should be some code comments as to why things are implemented the way they are.
• There should be a comment at the top of each new or modified test to explain what the test does.

New flags

• Is this flag really necessary?
• Flag names must be clear and intuitive, use dashes (-), and have a clear help text.

If a workflow is added or modified:

• Each item in Jobs should be named in order to mark it as required.
• If the workflow needs to be marked as required, the maintainer team must be notified.

Backward compatibility

• Protobuf changes should be wire-compatible.
• Changes to _vt tables and RPCs need to be backward compatible.
• RPC changes should be compatible with vitess-operator
• If a flag is removed, then it should also be removed from vitess-operator and arewefastyet, if used there.
• vtctl command output order should be stable and awk-able.

[codecov]

Codecov Report

❌ Patch coverage is 90.66667% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.01%. Comparing base (70c7a72) to head (2a6393d).
⚠️ Report is 277 commits behind head on main.

Files with missing lines	Patch %	Lines
go/vt/vtctl/reparentutil/emergency_reparenter.go	90.36%	16 Missing ⚠️
go/vt/vtctl/grpcvtctldserver/server.go	75.00%	2 Missing ⚠️
go/vt/vtctl/reparent.go	0.00%	2 Missing ⚠️
go/cmd/vtctldclient/command/reparents.go	50.00%	1 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (70c7a72) and HEAD (2a6393d). Click for more details.

HEAD has 1 upload less than BASE

Flag	BASE (70c7a72)	HEAD (2a6393d)
	1	0

Additional details and impacted files

@@             Coverage Diff             @@
##             main   #18707       +/-   ##
===========================================
- Coverage   69.67%   54.01%   -15.66%     
===========================================
  Files        1614      111     -1503     
  Lines      216793    22840   -193953     
===========================================
- Hits       151044    12337   -138707     
+ Misses      65749    10503    -55246     

Flag	Coverage Δ
partial	54.01% <90.66%> (?)

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[timvaillancourt]

Copying some conclusions from an offline discussion with @arthurschreiber:

• As it stands this PR will avoid a minority of lagging tablets from preventing ERS ✅
• As this PR stands, ERS will still-fail if a single or "majority" is picked and any of those tablets fail to apply logs, because the code still expects 100% of candidates to succeed 🟡
• It would be ideal if ERS tried the "next-best" candidate. Today it does not 🟡
  • This means selecting a 1+/majority may still be beneficial, if we know how to handle partial results
  • Call-out: in some case picking a next-best candidate will result in an errant GTID. Today the code makes every effort to avoid this, to the point of erring on failing the ERS. This is good for correctness but bad for availability. Some users may have differing views on this tradeoff. This should probably be configurable

[mattlord]

The reason, AFAIUI, for the current behavior is to prevent any of the healthy tablets from becoming forever unhealthy/unusable due to the new primary not having binary logs covering/containing the GTIDs that the lagging replica(s) may still need (the new primary may have recently been restored from a backup and have minimal binary logs). Have you already thought about that in this context?

[timvaillancourt]

The reason, AFAIUI, for the current behavior is to prevent any of the healthy tablets from becoming forever unhealthy/unusable due to the new primary not having binary logs covering/containing the GTIDs that the lagging replica(s) may still need (the new primary may have recently been restored from a backup and have minimal binary logs). Have you already thought about that in this context?

@mattlord I don't think that has changed, but I would appreciate you double checking my assumption because that is a very important functionality

One part of the code that could affect that was actually changed in #18531. Previous to this PR, the code called position.AtLeast(otherPos) on replication.Positions and this PR moved things to call a wrapper (*reparentutil.RelayLogPositions) with the same method name (.AtLeast(...)) that calls 2 x different replication.Positions: https://github.com/timvaillancourt/vitess/blob/main/go/vt/vtctl/reparentutil/replication.go#L55-L69

The TL;DR on that wrapper func: we do the same sort but put now prioritise positions with the most advanced SQL thread if two combined sets are equal. In the end the same replication.Position is called unchanged and that is what is deciding which GTID set is larger

And in terms of being certain we're ignoring the right tablets: after StopReplicationAndGetStatus is ran replication is stopped, we know the GTID sets and replication is not started until after the post-wait-for-relaylogs candidate selection. That selection uses the same sort logic as this optimization. So, the idea is: because replication isn't moving and we know all GTIDs each candidate could potentially apply when asked, we already know the post-wait-for-relaylogs GTID sets each candidate could have. This means they can be filtered before the apply phase, instead of after. Or TL;DR: we already know the losers after running StopReplicationAndGetStatus RPCs (via the After-field GTIDSets)

[Copilot]

Pull request overview

Copilot reviewed 15 out of 18 changed files in this pull request and generated no new comments.

Files not reviewed (1)

• go/vt/proto/vtctldata/vtctldata.pb.go: Language not supported

[promptless]

Promptless prepared a documentation update related to this change.

Triggered by PR #18707

Added documentation for the new --allow-split-brain-promotion flag to the vtctldclient command reference, and expanded the reparenting user guide with new sections on split-brain detection, partial relay-log-apply tolerance for GTID-based shards, and three new observability metrics.

Review: EmergencyReparentShard: document split-brain detection and partial failure tolerance

[timvaillancourt]

Docs PR: vitessio/website#2128

[mattlord]

We should not let split-brain override promote a lagging survivor in
go/vt/vtctl/reparentutil/emergency_reparenter.go:331-342. With AllowSplitBrainPromotion=true, errant-GTID detection restores the pre-detection set only when it removes every candidate. In the mixed case of two mutually-errant leading candidates plus a lagging survivor, both leaders can be pruned while the lagger remains, so ERS can re-wait and promote the lagger. That loses the unique writes from both split-brain sides, which is worse than the advertised “pick one side; losing side becomes errant” semantics. Please either require/validate --new-primary for this override path or restore/limit to the pre-errant leading set when all leading split-brain candidates are pruned and add a regression test with two mutually-errant leaders plus a lagging survivor.

We should preserve SQL-thread state during failed-ERS cleanup in
go/vt/vtctl/reparentutil/replication.go:283-285, go/vt/vtctl/reparentutil/emergency_reparenter.go:487-490. The new cleanup restarts any replica whose IO thread was healthy before ERS stopped it, but it calls StartReplication, which starts both IO and SQL threads. If a replica intentionally had SQL_THREAD stopped but IO running before ERS, an early abort will restart SQL unexpectedly. Cleanup should restore the original thread state exactly, or avoid calling full StartReplication for SQL-stopped replicas and cover that case with a unit test.