Backport: MySQL version-aware reparent for PRS & ERS - #866
Draft
ejortegau wants to merge 55 commits into
Draft
Conversation
Signed-off-by: Eduardo Ortega <5791035+ejortegau@users.noreply.github.com>
Patch version differences within the same MySQL release do not affect replication compatibility, so the version-aware candidate election now ignores them. Only major.minor boundaries trigger the preference for a lower-version primary. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com> Signed-off-by: Eduardo Ortega <5791035+ejortegau@users.noreply.github.com>
PRS always catches the elected tablet up to the old primary's exact demotion position, so replication position head-start is irrelevant for data safety. This change introduces SortMode (SortForPRS/SortForERS) to give PRS a distinct sort order: promotion rules > MySQL version > position > buffer pool > alias. ERS retains position-first ordering to minimize data loss. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com> Signed-off-by: Eduardo Ortega <5791035+ejortegau@users.noreply.github.com>
- Remove unused `after.ServerVersion` assignment in StopReplicationAndGetStatus - Log warning when MySQL version string fails to parse - Clarify findCandidate post-loop comment explaining two-phase logic - Add v25 changelog entry with deployment note and cross-cell limitation Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com> Signed-off-by: Eduardo Ortega <5791035+ejortegau@users.noreply.github.com>
- Add `server_version` field to `PrimaryStatus` proto so that DemotePrimary and PrimaryStatus RPCs report the MySQL version - Populate version in PrimaryStatus, DemotePrimary, and StopReplicationAndGetStatus RPCs - Move GetVersionString call after replication stop in StopReplicationAndGetStatus to avoid delaying the critical path - Read PrimaryStatus.ServerVersion in ERS ERNotReplica path so demoted primary-status candidates no longer get unknownVersion - Extract getMySQLVersion helper to deduplicate the fetch-and-warn pattern across 4 call sites - Add test coverage for version propagation through both paths Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com> Signed-off-by: Eduardo Ortega <5791035+ejortegau@users.noreply.github.com>
ReleaseAtLeast already covers the same-release case (Minor >=), so the redundant IsSameRelease check can be removed. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com> Signed-off-by: Eduardo Ortega <5791035+ejortegau@users.noreply.github.com>
|
This PR is being marked as stale because it has been open for 30 days with no activity. To rectify, you may do any of the following:
If no action is taken within 7 days, this PR will be closed. |
StopReplicationAndGetStatus had several early-return paths (IO thread already stopped, replication not healthy, stop failures, after-status failure) that returned Before without populating ServerVersion. ERS builds its version map from Before.ServerVersion, so tablets hitting these paths became unknownVersion and could lose version-aware election to newer tablets. Fix: call getMySQLVersion(ctx) before every return that includes Before. Add a table-driven test covering all return paths (success and error) for both IOTHREADONLY and IOANDSQLTHREAD modes. Ref: vitessio#20211 (review) Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com> Signed-off-by: Eduardo Ortega <5791035+ejortegau@users.noreply.github.com>
The cherry-picked test case references a primaryAlias struct field that doesn't exist on the slack-22.0 branch. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Trace version detection and candidate selection so we can verify the lowest-version preference is working as intended in production. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
The successful stop path set Before.ServerVersion but left After.ServerVersion empty, while the no-op paths return After: before and thus include it. Set after.ServerVersion = before.ServerVersion so the common success path is consistent, and extend the test to assert After.ServerVersion when present. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com> Signed-off-by: Eduardo Ortega <5791035+ejortegau@users.noreply.github.com>
This reverts commit 4bfe9c5.
The candidate position map is built before waitForAllRelayLogsToApply and reused unchanged when electing the intermediate source. Because RelayLogPositions.AtLeast uses Executed as the tiebreaker when Combined positions are equal, a lower-version tablet that was only SQL-thread behind pre-wait still appeared behind an equally-advanced newer-version tablet even after the wait succeeded. In a mixed-version ERS, that selected the newer MySQL release as the intermediate source and made the older lower-version candidate replicate from it — the incompatible direction this work avoids. Reconcile in memory after the wait: set Executed to Combined for every candidate we waited on (those in the status map). A successful WaitForRelayLogsToApply already guarantees the SQL thread reached Combined, and ERS has stopped the IO thread so nothing can arrive past it, so no extra ReplicationStatus RPCs are needed — keeping ERS fast and reducing points of failure. Candidates absent from the status map (former primary) are skipped. Add a regression test where an older-version tablet shares the combined relay-log position but has a lower pre-wait executed position than a newer tablet, plus a unit test for the reconciliation helper. Ref: vitessio#20211 (review) Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com> Signed-off-by: Eduardo Ortega <5791035+ejortegau@users.noreply.github.com> (cherry picked from commit 11c64fd) # Conflicts: # go/vt/vtctl/reparentutil/emergency_reparenter_test.go
Addresses review nit: the helper returns the raw version string, so the name should reflect that. Ref: vitessio#20211 (comment) Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com> Signed-off-by: Eduardo Ortega <5791035+ejortegau@users.noreply.github.com> (cherry picked from commit 479dfd0)
GetVersionString runs a live query against mysqld (falling back to dialing mysqlctld or shelling out to mysqld --version). This PR added it to ReplicationStatus (polled fleet-wide by vtorc), PrimaryStatus, demotePrimary, and StopReplicationAndGetStatus (on the per-tablet ERS critical path), so it was firing on every such RPC even though the server version only changes across a mysqld restart. Cache the version string on TabletManager behind its own lock (not tm.mutex, so version reads never contend with unrelated operations) with a 30s TTL. The lock is intentionally not held across the fetch, so a cold/expired cache can produce a small bounded burst rather than a strict single query; single-flight is deliberately omitted as not worth it for a cheap, rare fetch. A stale value is harmless — reparent degrades to position-only ordering until it refreshes. Errors are logged (slog-style) and surface as "" without being cached. Tests cover caching within the TTL, refetch after expiry, error-not-cached, and a -race concurrency subtest for the lock-drop-across-fetch design. Ref: vitessio#20211 (review) Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com> Signed-off-by: Eduardo Ortega <5791035+ejortegau@users.noreply.github.com> (cherry picked from commit db187b1) # Conflicts: # go/vt/vttablet/tabletmanager/rpc_replication.go
The standalone reconcilePositionsAfterRelayLogApply advanced Executed to Combined for every candidate present in the status map, relying on waitForAllRelayLogsToApply's all-or-nothing contract to guarantee they had all applied. That coupling is a footgun: if the wait ever tolerated partial success, a candidate whose apply failed but remained in the status map would be falsely marked caught up and could be promoted with unapplied relay-log transactions. Fold the reconcile into waitForAllRelayLogsToApply so a candidate's Executed position is advanced only after that candidate's own WaitForRelayLogsToApply succeeds. Candidates that fail (or are cancelled) keep their real, behind positions. This is correct by construction and no longer depends on the caller ordering. The waiter goroutines now write into the shared candidate positions, guarded by a mutex. Replace the deleted helper's unit test with a table-driven test covering both the success path (Executed advances to Combined) and the failure path (position left behind), plus the former-primary case (absent from the status map, never touched). Rework the version-tiebreaker regression test to drive through the real wait path. Ref: vitessio#20211 (review) Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com> Signed-off-by: Eduardo Ortega <5791035+ejortegau@users.noreply.github.com> (cherry picked from commit 9323188)
findCandidate falls back to the position-sorted ordering (first element, or the intermediate source) when there is no version data or when releases tie, but nothing local told the reader the input must be pre-sorted by replication position. Add a doc comment making that contract explicit. Ref: vitessio#20211 (review) Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com> Signed-off-by: Eduardo Ortega <5791035+ejortegau@users.noreply.github.com> (cherry picked from commit bb6cf6b)
golangci-lint's testifylint flagged require.Equal(t, "", ...) in the version cache error test. Switch to require.Empty as the linter requires. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com> Signed-off-by: Eduardo Ortega <5791035+ejortegau@users.noreply.github.com> (cherry picked from commit f0bf014)
Shorten the window during which a just-restarted (e.g. upgraded) mysqld can report a stale version, narrowing the operational pause needed between rolling upgrades of long-running vttablets. The fetch is cheap and rare, so the extra lookups from a shorter TTL are negligible. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com> Signed-off-by: Eduardo Ortega <5791035+ejortegau@users.noreply.github.com> (cherry picked from commit 4b3f41b)
…-position mode) The post-wait reconcile set Executed = Combined, which only works for GTID-based replication. For file-position (non-GTID) replication, FindPositionsOfAllCandidates stores the pre-wait executed position in Combined, while WaitForRelayLogsToApply catches up to RelayLogSourceBinlogEquivalentPosition. Setting Executed = Combined therefore left a stale Combined, so two candidates that both caught up were still compared on their pre-wait positions — letting ERS pick a newer-MySQL tablet as the intermediate source and point an older-version tablet at it, the incompatible direction this work avoids. Reconcile to the position WaitForRelayLogsToApply actually waited for instead. A new appliedPositionAfterWait helper (used by WaitForRelayLogsToApply itself, so the two can't drift) returns the relay-log position for GTID or the relay-log file-position equivalent otherwise. On success the candidate's Combined and Executed are both set to that decoded position; on decode failure the pre-wait position is left in place (understated, never overstated). For GTID this is a no-op on Combined, so the common path is unchanged. Add a file-position mixed-version catch-up regression test, and a test proving the reconcile does not mask a genuine split brain (each candidate advances only to its own applied position, so divergent positions stay divergent). Update the existing GTID catch-up tests to use real encodable position strings now that the reconcile decodes them. Ref: vitessio#20211 (review) Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com> Signed-off-by: Eduardo Ortega <5791035+ejortegau@users.noreply.github.com> (cherry picked from commit 4d41246)
Addresses review feedback on how the version-aware election compares versions. Flavor families: version comparison is only meaningful within a replication- compatibility family. Add MySQLFlavor.ReplicationFamily (MySQL and Percona share a lineage; MariaDB is separate) and skip version-aware ordering — with a warning — when candidates span more than one family, falling back to position/promotion ordering. Previously the flavor returned by ParseVersionString was discarded and release integers were compared across flavors (e.g. MariaDB 10 vs MySQL 8). Patch component within the 8.0 series: comparing only major.minor is unsafe for MySQL 8.0, where feature additions before 8.0.34 (the first bugfix-only 8.0 patch, e.g. binary log transaction compression in 8.0.20) can break newer-source- to-older-replica replication. Add ServerVersion.CompareForReplication, which compares by major.minor and additionally by patch when both versions are in the 8.0 series and the lower patch is below 8.0.34. Route the sorter and findCandidate through it, and drop the now-unused ReleaseAtLeast. Observability: log a warning when a candidate's MySQL version cannot be determined, so a transient fetch/parse failure (which yields the unknownVersion sentinel) is visible to operators rather than silently affecting election. Enforcement: document at the waitForAllRelayLogsToApply errgroup boundary that the position reconcile depends on the all-or-nothing wait, so loosening it requires revisiting the reconcile and election together. Tests: CompareForReplication unit cases (8.0 patch significant below 34, ignored at/above 34, ignored outside 8.0); flavor-family guard cases (MySQL+Percona usable, MariaDB mixes disabled); election-level patch regressions for ERS and PRS; a sorter-level Executed-tiebreak test; and a test that the position reconcile does not mask a genuine split brain. Ref: vitessio#20211 (review) Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com> Signed-off-by: Eduardo Ortega <5791035+ejortegau@users.noreply.github.com> (cherry picked from commit e27f727) # Conflicts: # changelog/25.0/25.0.0/summary.md # go/vt/vtctl/reparentutil/util.go
The initial flavor-family guard computed a single usable-version map over the whole replication snapshot and threaded it through the ERS election. That disabled version-aware election in strictly more cases than necessary: a tablet of another family that is not even a candidate — e.g. a MariaDB replica dropped by findErrantGTIDs — would still make the guard see "mixed families" and turn off version ordering for the surviving same-family candidates. It also left the final election (identifyPrimaryCandidate -> findCandidate) able to version-compare across families, since it received the raw snapshot map. Scope the guard to each election's actual candidate set instead. scopedVersionMap builds the flavor list from only the passed candidates and returns nil (disabling version ordering) only when those candidates span more than one family. reparentShardLocked now passes the raw snapshot version and flavor maps to findMostAdvanced and identifyPrimaryCandidate, each of which applies scopedVersionMap over the tablets it actually elects among (findMostAdvanced over its candidate list; identifyPrimaryCandidate over each promotion tier). findCandidate can still read the intermediate source's version cross-family relative to a tier; document why that is harmless (the source preference only fires when the source is in the tier, hence in-family). Tests: - TestScopedVersionMap covers same-family usable, mixed-family disabled, and the key case where a non-candidate other-family tablet does not disable ordering. - findMostAdvanced and identifyPrimaryCandidate gain mixed-family cases that pass raw maps and rely on the internal guard (honest: they fail if the guard is neutered). - A reparentShardLocked-level test proves the end-to-end wiring: a MariaDB candidate dropped for errant GTIDs leaves MySQL-only survivors that still get version ordering (the lower version wins). It fails if the guard reverts to the full snapshot. Ref: vitessio#20211 (comment) Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com> Signed-off-by: Eduardo Ortega <5791035+ejortegau@users.noreply.github.com> (cherry picked from commit 4b2b164) # Conflicts: # go/vt/vtctl/reparentutil/emergency_reparenter_test.go
… tiebreak
FindPositionsOfAllCandidates stored a PrimaryStatus candidate as
{Combined: executedPosition} with a zero Executed, while waited replicas
have their Executed reconciled up after the relay-log wait. In
RelayLogPositions.AtLeast, when Combined positions are equal the Executed
field is the tiebreaker, so an equally-advanced former primary always
ranked behind a reconciled replica and ERS never reached the
promotion-rule/version tiebreakers for it — a newer-version replica could
beat an equally-advanced older former primary.
A demoted/former primary applies no relay log, so its executed position is
authoritative. Initialize Executed alongside Combined so an equally-advanced
former primary compares equal and election falls through to the tiebreakers.
This cannot overstate a candidate: a strictly-more-advanced tablet still wins
on Combined.
Tests cover both GTID (FindPositionsOfAllCandidates unit test) and
file-position (full ERS flow through waitForAllRelayLogsToApply) paths;
both fail without the fix. Release notes updated.
Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Signed-off-by: Eduardo Ortega <5791035+ejortegau@users.noreply.github.com>
(cherry picked from commit 7e81331)
# Conflicts:
# changelog/25.0/25.0.0/summary.md
# go/vt/vtctl/reparentutil/replication_test.go
…RS tests The version-aware reparent backport pulls in regression tests (TestEmergencyReparenterFindErrantGTIDs_NilPosition and the "all candidates filtered out by errant GTID detection" reparentShardLocked case) that exercise nil/empty candidate paths. Upstream relies on guards that exist in upstream main but were absent on this fork branch: - findMostAdvanced: return a FAILED_PRECONDITION error on an empty candidate set instead of indexing validTablets[0] (guard from upstream PR vitessio#19904, present on origin/main but not on this feature branch). - findErrantGTIDs lagged-candidate loop: skip nil/zero positions before dereferencing, matching the guard already present in the max-len loop. - findErrantGTIDs max-len loop: fix a pre-existing `||` that should be `&&` (from fork backport #736), which nil-panicked on a nil otherPosition. All reparentutil and mysqlctl package tests pass with these guards. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
ejortegau
force-pushed
the
eduardo.ortega/mysql-version-aware-reparent
branch
from
July 21, 2026 00:04
6705943 to
262226c
Compare
proto/replicationdata.proto gained server_version (and semi-sync) fields on the Status message as part of the version-aware reparent backport, but the checked-in vtadmin web bindings were never regenerated. CI's 'Make VTAdmin Web Proto' check regenerates and diffs these, so it failed. Regenerated via ./web/vtadmin/bin/generate-proto-types.sh. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
The dominance scan added to performInitialPromotion fans out a PrimaryPosition RPC to every tablet and can consume the full remote-operation timeout, during which the shard lock could be lost to a competing reparent. Re-check the lock before InitPrimary makes the elect read-write, matching the boundary the other promotion paths enforce after their read/wait phase. Adds a test that promotion aborts when the lock is not held. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com> Signed-off-by: Eduardo Ortega <5791035+ejortegau@users.noreply.github.com> (cherry picked from commit 54676f6)
FindPositionsOfAllCandidates derived isGTIDBased only from the replica status map, so a shard whose every reachable candidate is a former primary (empty statusMap, populated primaryStatusMap) was misclassified as non-GTID even with MySQL56 positions. That silently disabled version-aware election and the former-primary Executed initialization, and let a GTID/non-GTID mix across the two maps slip past the mixed-mode guard. Decode the primary executed positions up front and fold their flavor into detection (skipping zero positions, which are flavor-agnostic; former primaries remain exempt from the empty-relay-log check). Adds cases for primary-only MySQL56, primary-only FilePos, a GTID/FilePos mix across the maps, and a GTID replica alongside a zero-position former primary (pinning the zero-position skip). Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com> Signed-off-by: Eduardo Ortega <5791035+ejortegau@users.noreply.github.com> Backport notes (v22 fork): - Resolved the FindPositionsOfAllCandidates conflict by taking upstream's relocated primaryPositions decode loop (our prior commit still decoded inline). - Dropped TestFindPositionsOfAllCandidates_ErrorNotDuplicated from the incoming test hunk: it comes from unrelated upstream PR vitessio#20203 (de-deduplication of vterrors.Wrapf), needs a strings import, and the fork still has doubled Wrapf calls it would trip on. Kept TestFindPositionsOfAllCandidates_FlavorFromPrimaryStatus. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
The initial-promotion dominance check fetched every tablet's position sequentially under one shared RemoteOperationTimeout, so on a shard with many tablets (or a slow-but-healthy one early in the loop) later PrimaryPosition calls could inherit an already-expired deadline and fail PRS bootstrap spuriously. Fetch positions concurrently so each RPC gets the full timeout in parallel, matching performPotentialPromotion's fan-out. Extract the shared "elect must dominate every collected position" invariant into verifyPrimaryElectDominates, used by both the initial-promotion and potential-promotion paths so the check can't drift between them. Add initial-promotion test cases for a peer position-fetch failure (pinning the new cross-goroutine error aggregation and the all-tablets-reachable contract) and an incomparable/divergent-GTID peer position (pinning the advertised incomparable-rejection guarantee). Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com> Signed-off-by: Eduardo Ortega <5791035+ejortegau@users.noreply.github.com> (cherry picked from commit fbcf922)
…dates On an uninitialized shard, PRS candidates commonly return ERNotReplica because they are not replicating yet. findTabletPositionLagBackupStatus returned an empty server version for those, so ElectNewPrimary treated every candidate as unknown-version and fell back to alias/position ordering — during a mixed MySQL-version bootstrap that could elect a newer mysqld and then reparent older tablets to it, defeating version-aware selection. Recover the version from PrimaryStatus (a read-only RPC) on the ERNotReplica path. The position stays legitimately empty; only the version is recovered. The extra RPC is confined to this bootstrap path, not steady-state ERS where replicas are actually replicating, so it doesn't add per-reparent brittleness. Best-effort: if PrimaryStatus also fails, fall back to unknown version rather than failing the election. Also fixes an adjacent "statue" -> "status" typo on the touched line. Adds an explicit success case (version recovered) and fallback case (PrimaryStatus fails -> unknown version) so both branches are legibly pinned. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com> Signed-off-by: Eduardo Ortega <5791035+ejortegau@users.noreply.github.com> (cherry picked from commit a852e22)
…r paths StopReplicationAndGetStatus fetched the MySQL version before its three error returns (stop-io-thread, stop-replication, and after-status failures), but grpctmserver copies the status only when err == nil and the client discards the response on error — so that version never crosses the RPC boundary. On a cold cache the fetch runs a live MySQL query (and can fall through to dialing mysqlctld or shelling out) while holding the TabletManager lock, delaying ERS failure handling and blocking other TM operations for a value that is thrown away. Drop the version fetch on those error returns. The success returns still populate it (unchanged, per the earlier request in review 4413330626), and the test now asserts ServerVersion is empty on the error paths. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com> Signed-off-by: Eduardo Ortega <5791035+ejortegau@users.noreply.github.com> (cherry picked from commit a1cc510)
The flavor detection added in "detect replication flavor from primary status too" folds former-primary executed positions into the GTID/ non-GTID decision, skipping zero positions via Position.IsZero. That commit was written against upstream, whose Position.IsZero returns true for an empty-but-typed GTID set (it folds in GTIDSet.Empty). This fork predates that change: its Position.IsZero only checks for a nil GTIDSet, so a fresh, never-initialized tablet reporting "MySQL56/" (non-nil, empty Mysql56GTIDSet) is not treated as zero. The result: on an uninitialized shard every tablet returns ERNotReplica, is demoted, and lands in the primary status map with an empty "MySQL56/" position. Detection then classified the shard as GTID-based, ERS ran errant-GTID detection, and gatherReparenJournalInfo read _vt.reparent_journal on tablets that have no _vt database yet, failing with "Unknown database '_vt'". This broke TestERSForInitialization. Skip empty Mysql56GTIDSet positions explicitly (len == 0) so an uninitialized shard is not misclassified. This reproduces upstream's Empty()-aware Position.IsZero behavior locally, without backporting the broad go/mysql GTIDSet.Empty() interface change (vitessio#18196), which touches ~99 Position.IsZero call sites fork-wide. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
|
This PR is being marked as stale because it has been open for 30 days with no activity. To rectify, you may do any of the following:
If no action is taken within 7 days, this PR will be closed. |
Backports the minimal GTID-emptiness slice of upstream PR vitessio#18196 (1c5ca23): add Empty() to the GTIDSet interface and its three concrete implementations (Mysql56GTIDSet, FilePosGTID, MariadbGTIDSet), and make Position.IsZero() return true for a typed-but-empty set (e.g. "MySQL56/" with no transactions), not just a nil GTIDSet. This is the prerequisite for backporting the remainder of the version-aware reparent follow-up work (PR vitessio#20211 branch commits #32-#54), whose ERS/PRS empty-GTID handling relies on IsZero() folding in emptiness. It also lets us retire the fork-local workaround in FindPositionsOfAllCandidates in favor of the upstream end-state. Only the Empty()/IsZero() semantics are ported here, not the broader optimizations in PR vitessio#18196. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
A shard whose every reachable candidate reports a zero GTID position (nil or a typed-but-empty set such as "MySQL56/") has never seen a promotion: it is being initialized. Such tablets have no _vt sidecar tables yet, so running errant-GTID detection reads a reparent journal that does not exist and fails ERS-for-initialization. Skip detection and accept all candidates in that case. This stands in for the upstream mechanism (allPositionsZero + shardNeverInitialized inside gatherReparentJournalInfo), which is entangled with an ERS-refactor chain this fork has deliberately NOT backported: vitessio#20578 (extraEvidence/starved), vitessio#20780 (explicit split-brain recovery, which introduced the allPositionsZero early-return), and vitessio#20831/e5255c93b4 (journal-missing tolerance). A TODO marks this for retirement once the fork upgrades past those PRs. This is a prerequisite for aligning FindPositionsOfAllCandidates with upstream's empty-GTID classification (which will classify a fresh shard as GTID-based). Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
…ation with upstream Adopts upstream PR vitessio#20211's end-state (a5649f5) for GTID/non-GTID classification, retiring the fork-local workaround from 6f9890a: - Extract the MySQL56 GTID-set type assertion into hasMysql56GTIDSet, used by both the replica and former-primary detection loops. - In the former-primary loop, skip only a nil GTIDSet (an empty-string "" position). A typed-but-empty set ("MySQL56/" with no transactions) still identifies the flavor and is counted as GTID-based, matching upstream and upstream's classification tests. The previous workaround additionally skipped typed-empty sets so a fresh shard classified as non-GTID (avoiding errant-GTID detection). That is no longer needed: Position.IsZero() now folds in GTIDSet.Empty() (prior commit), and the fresh-shard case is handled by the empty-shard early-return in findErrantGTIDs. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
… PRS Both PRS paths that promote without catching the elect up to a source first — initial promotion (never-initialized shard) and potential promotion (no clear primary) — verify the primary-elect's position contains every peer's via the shared verifyPrimaryElectDominates. On file-position (non-GTID) tablets, PrimaryPosition/DemotePrimary return that tablet's own binlog coordinates, whose file names and offsets are local to each tablet and not comparable across tablets. Feeding them to AtLeast produces a meaningless verdict that can wrongly report the elect as dominant and wave through a promotion discarding a peer's transactions. Move the fail-closed guard into verifyPrimaryElectDominates so both paths are protected and the check can't drift between them. Empty positions (a fresh tablet on a never-initialized shard) carry no FilePosGTID and stay comparable, so GTID and fresh-init promotions are unaffected. Adds a fail-closed test on each path where the elect's file coordinates would otherwise falsely dominate. Backport of upstream d2d376a (PR vitessio#20211). Changelog entry omitted: it targets the 25.0 tree, which does not exist on this fork. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
StopReplicationAndGetStatus (after stopping replication) and demotePrimary (after demotion) fetch the MySQL version to populate the status they return. On a cold or expired cache that query can be slow, and reusing the RPC context unbounded lets it consume the remaining deadline — so the client can see DEADLINE_EXCEEDED even though the mutation already succeeded. In ERS that lost response means the tablet is dropped from the status map and cleanup may not restart it; in PRS a demotion that happened can be reported as failed. Add getMySQLVersionStringAfterMutation, which bounds the lookup to at most half the remaining deadline (reserving the other half to return the response) and never more than 2s. Both callers hold the TabletManager action lock for their whole body, so the 2s absolute cap also stops a hung fetch from holding that lock for up to half a large deadline. On timeout it returns "" like any lookup failure, degrading to position-only ordering. Only the two post-mutation call sites use it; the poll paths and pre-mutation early returns are unchanged. Tests: bounding to half the deadline, the 2s absolute cap on a large deadline, the no-deadline fallback, and a StopReplicationAndGetStatus wiring test asserting a slow lookup still returns the stopped status promptly with an empty version. Backport of upstream 706fd07 (PR vitessio#20211). Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
…n PRS
The file-position fail-closed guard in verifyPrimaryElectDominates ran on the
initial-promotion and no-clear-primary paths regardless of whether the operator
explicitly requested the primary. Since neither path catches the elect up to a
source, that turned explicit recovery of a non-GTID shard into a hard failure —
and the error's own advice ("specify an explicit primary to promote") was
impossible to act on, because an explicit --new-primary flows through these
paths the same way and still hit the check.
Thread whether the primary was explicitly requested (captured before
preflightChecks overwrites opts.NewPrimaryAlias with the elected candidate) down
to the guard. A file-position tablet is now handled in a single place: without an
explicit primary it fails closed; with one, the tablet is skipped entirely —
both the rejection and the dominance comparison, since file-position coordinates
are not comparable across tablets and an AtLeast verdict on them is meaningless
(it would otherwise block the explicit choice whenever the elect's binlog
filename happened to sort below a peer's). The GTID dominance check still runs in
all cases; an explicit --new-primary does not relax that data-loss safeguard.
Backport of upstream 6159b39 (PR vitessio#20211).
Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Mysqld.GetVersionString(ctx) honored the context for its DB query and mysqlctld dial, but its final fallback called the package-level GetVersionString(), which shelled out via exec.Command — ignoring the context. So a stalled `mysqld --version` could run unbounded even under a caller deadline, which defeated the post-mutation version-lookup cap in StopReplicationAndGetStatus / DemotePrimary: those hold the TabletManager action lock, so an unkillable shell-out could block every other tablet action after the mutation had already succeeded. Add context-aware variants (execCmdWithContext via exec.CommandContext, and GetVersionStringWithContext) and make the existing context-less functions thin context.Background() wrappers, so all current callers are unaffected. Point Mysqld.GetVersionString's fallback at GetVersionStringWithContext(ctx) so the caller's deadline now bounds the shell-out and the cap is actually enforceable. Backport of upstream 36c2b68 (PR vitessio#20211). Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
verifyPrimaryElectDominates applied the file-position rejection to every position in the map, including the primary-elect's own, before the self-skip. On an initial/no-clear-primary non-GTID shard with only the primary-elect, auto-election therefore failed with "specify an explicit primary" even though there are no cross-tablet coordinates to compare and initializing the sole eligible tablet is safe. Skip the elect's own position first, so the file-position rejection only applies to peer tablets. Multi-tablet file-position shards still fail closed (auto) or are skipped (explicit) on a peer's position as before; only the single-candidate case changes, which is exactly the bootstrap/recovery case that should proceed. Backport of upstream e2b2357 (PR vitessio#20211). Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Version-aware PRS election ordered candidates version-first on all paths. That is safe on the graceful path (the elect is caught up to the old primary before promotion) and the initialization path (no tablet has ever replicated), but the no-clear-primary path promotes the elect without any catch-up. There, version-first could elect a newer-version tablet that is behind on received (relay-log) transactions; since that path's dominance check compares only executed positions, a demoted replica's received-but-unapplied transactions could be silently discarded. Restore position-first election for the no-clear-primary path (matching ERS), keeping version only as a tiebreak among equally-advanced candidates. The graceful and initialization paths keep version-first ordering. When a lower (more broadly compatible) version is passed over because a higher-version candidate is more advanced, PRS logs a warning so operators can move to the preferred version with a follow-up PRS once the shard is healthy. Rename the sort modes and comparators to be ordering-based (SortByPosition / SortByVersion, lessPositionFirst / lessVersionFirst) so ERS and the PRS no-catch-up path share one position-first implementation without duplication. Backport of upstream 05c1170 (PR vitessio#20211). Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
…a deadline getMySQLVersionStringAfterMutation only applied the maxPostMutationVersionLookup cap inside the has-deadline branch, so a deadline-less caller (e.g. an in-process DemotePrimary/StopReplicationAndGetStatus) skipped it entirely. Since these run while holding the TabletManager action lock, a hung cold-cache fetch could then block every other tablet action indefinitely after the mutation had already succeeded — the exact hazard the cap exists to prevent. Always install the 2s cap; tighten it to half the remaining deadline only when one exists. Adds a no-deadline test asserting a slow lookup is still bounded. Backport of upstream 76aabb8 (PR vitessio#20211). Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
…tus paths StopReplicationAndGetStatus has two no-op early returns — the IO-thread-only mode when the IO thread is already stopped, and the full-stop mode when replication is not running. Both looked up the MySQL version with the unbounded getMySQLVersionString and the caller's full RPC context, while the mutation path already used the bounded getMySQLVersionStringAfterMutation. These branches run under the TabletManager action lock. On a cold or expired version cache a slow GetVersionString could consume the whole RPC deadline, so the caller sees DEADLINE_EXCEEDED and loses the status that was already read successfully — even though no stop was needed. In ERS that can drop a reachable tablet from the status map (so cleanup may not restart it) purely over optional version metadata. Use the bounded helper on both no-op paths, matching the mutation path. It degrades to an empty version on timeout (position-only ordering) instead of failing the RPC. Add a table test covering both branches. Backport of upstream a4ac0eb (PR vitessio#20211). Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
…ties FindCurrentPrimary compared each PRIMARY record only against the running best-so-far, returning nil on any term-start tie it encountered. With two stale PRIMARY records tied at an older term plus one unique newer primary, the result depended on Go's randomized map iteration order: visiting the stale pair first returned nil, visiting the newer one first returned it. PRS calls this twice per run — once in ElectNewPrimary to choose version-first vs position-first ordering, and once in reparentShardLocked to choose the promotion path. Order-dependence could make those two calls disagree, so election would use different safety assumptions from the path that executes. Track the maximum term and whether the maximum itself is tied, resetting the tie flag whenever a strictly newer term is found. Only a tie at the latest term now yields nil; a superseded tie below a unique maximum does not. The result is independent of iteration order. Backport of upstream e04cee0 (PR vitessio#20211). Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
…s reads Route ReplicationStatus and PrimaryStatus through the deadline-bounded version-lookup helper, renaming getMySQLVersionStringAfterMutation to getMySQLVersionStringBounded and maxPostMutationVersionLookup to maxVersionLookupBudget now that the bound guards fleet-wide read pollers as well as post-mutation action-lock holders. Backport of upstream 05a44c0 (PR vitessio#20211). Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
The prior commit's test edit was comment-only: the existing zero-position
case uses Position: "" which decodes to a nil GTIDSet, so it is skipped by
both GTIDSet == nil and the old IsZero() condition — it passes identically
with or without the fix and does not pin the changed behavior.
Add the genuinely distinguishing case: a typed-but-empty MySQL56 former
primary ("MySQL56/") alongside a non-GTID (FilePos) replica must trip the
mixed-mode guard. This is exactly where GTIDSet == nil and IsZero() diverge —
under an IsZero() skip the former primary is dropped and no mix is detected.
Backport of upstream 2318e2d (PR vitessio#20211).
Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
…RS paths The no-catch-up PRS promotions (init and no-clear-primary) verify the elect's position contains every peer's before promoting. A shard mixing GTID-based and file-position tablets cannot satisfy this: a GTID elect's position is not comparable against a file-position peer's, so the per-peer file-position escape hatch (an explicit --new-primary) could skip that peer and silently discard its transactions. Reject a mixed shard outright, before the escape hatch. Skip only a position with no GTIDSet at all (a fresh, never-initialized tablet encoded as ""); a typed-but-empty GTID position such as "MySQL56/" is still GTID-based and must count toward the GTID group, mirroring the ERS sibling detector in replication.go. Backport of upstream b3ca651 (PR vitessio#20211). Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
FindPositionsOfAllCandidates classifies a shard on hasMysql56GTIDSet( RelayLogPosition), not on Status.UsingGtid. A fresh MySQL 8.0 replica with GTID enabled but no transactions reports empty Executed/Retrieved GTID sets, which ParseMysqlReplicationStatus decodes to a typed-but- empty Mysql56GTIDSet (encoded "MySQL56/", non-nil GTIDSet) — so the shard classifies as GTID-based, not non-GTID. Add a regression test that drives this through the real MySQL flavor parser: it asserts the relay position is typed-empty "MySQL56/", that hasMysql56GTIDSet returns true, and that an all-empty shard fails closed on the empty-relay-log guard rather than falling through to non-GTID handling (where version-aware election would be disabled). Backport of upstream 8e32968 (PR vitessio#20211). Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
…otion checkPrimaryElectContainsAllPositions promotes the elect via InitPrimary, which does not apply the relay log. Keying every tablet on the combined (received) position credited the elect with received-but-unapplied transactions that InitPrimary discards, so a lossy promotion could be waved through: elect Combined=1-10 (Executed=1-3) over a peer Combined=1-8 passes a combined-vs-combined check, yet only 1-3 survives promotion. Switch to ReplicationStatus and make the dominance check asymmetric: key the elect on its executed position and every peer on its combined position, proving elect.Executed >= peer.Combined. Non-replicating tablets (the common fresh-init case) return ErrNotReplica; fall back to PrimaryStatus for their executed position rather than failing closed. Adds a regression test (received-ahead but executed-behind is rejected) that fails on the prior combined-vs-combined behavior, and pins per-case expected error text on every erroring performInitialPromotion subtest. Backport of upstream d967586 (PR vitessio#20211). Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
MariaDB reports its executed GTID position in Position but leaves RelayLogPosition empty (ParseMariadbReplicationStatus). On a uniform MariaDB shard the initial-promotion path uses version-first election, so checkPrimaryElectContainsAllPositions could key a peer on its empty RelayLogPosition, decode a zero position, and wave through a version-preferred elect that is behind the peer — letting InitPrimary discard the peer's executed transactions. Fall back to the peer's executed Position when RelayLogPosition is empty, the only cross-tablet-comparable position MariaDB exposes. The fallback is a no-op for MySQL/Percona, whose RelayLogPosition is empty only when Position is too. Backport of upstream a5649f5 (PR vitessio#20211). Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
…st site Rename the SortByVersion case that pins "lower release beats a dominated position" and add a comment spelling out the intent: the sorter compares a binary dominance count, not a position magnitude, so the result holds regardless of how far behind the lower-version candidate is. The test can only pin the strictly-dominated case, and it does so without relying on the alias tiebreak (the winner has the higher alias). Backport of upstream 684196c (PR vitessio#20211). Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Remove a stray double blank line in StopReplicationAndGetStatus flagged by gofmt (introduced by an earlier branch commit). Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's this?
Backport of the MySQL version-aware primary selection feature for PRS and ERS from upstream PR vitessio#20211.
When selecting a new primary during reparenting, this now prefers tablets running a lower MySQL release (major.minor) to maintain replication compatibility — replicas must be at the same or higher version than the primary.
How it works
server_versionfield to the replication status proto, populated duringStopReplicationAndGetStatussortTabletsForReparentto consider MySQL version after promotion rulesidentifyPrimaryCandidate, among same-tier candidates prefers the lowest MySQL releaseDifferences from upstream
log.Warningfinstead of structuredlog.Warn(slack-22.0 doesn't have the new logging)Cherry-picked and adapted by Claude Code from upstream commits.