Repository navigation
backport(2026.10.7): rep __rep_cmp_vote total order (election can elect older data) - #267
Merged
Merged
Conversation
…gen bypass)
The election winner comparator __rep_cmp_vote (src/rep/rep_elect.c) was not a
consistent order, with two defects:
1. The first vote (rep->sites == 1) was adopted outright, so the winner
depended on which vote a site tallied first.
2. The last arm (equal LSN => higher priority / tiebreaker) fired regardless
of data_gen, so a vote with an OLDER data_gen but an equal LSN and a
higher priority or tiebreaker could beat a vote with a NEWER data_gen.
Defect 2 is a durability bug, not just a failure to settle: electing a site
with older data rolls back a commit the newer site already had (a client
memcmp's the log bytes at its verify LSN in __rep_verify, so a wrong winner
genuinely truncates the loser's tail). The precondition -- two electable
sites with EQUAL last-commit LSN and DIFFERENT data_gen -- was reproduced
against the live engine (test/rep/E1-FINDINGS.md): a VOTE1 carries the LSN of
the last __txn_regop and the highest gen in REPLSNHIST, and these are
independent, so equal LSN with different data_gen is reachable.
Rewrite __rep_cmp_vote as a single TOTAL ORDER, applied uniformly to every
vote including the first: electable > not; non-zero priority > zero (the
mixed-version ELECTABLE semantics are preserved); then data_gen, LSN, priority,
tiebreaker, and finally eid as the always-distinct discriminator so the
relation is total even if two sites draw equal random tiebreakers. Because
rep->winner is not reset between elections, the first vote is detected by
rep->sites == 1 (reset in __rep_elect_done), not by rep->winner.
Gate: test/db/run_rep_cmpvote_order.sh drives a verbatim copy of the real
comparator (drift-checked by check_rep_cmpvote_copy.sh) over the captured vote
vectors in every arrival order; it PASSES on the fix and FAILS with -DCMPVOTE_OLD
(the pre-fix comparator). lts/2026.10.7 has the identical function; backport to
follow.
(cherry picked from commit 9b31ea4)
Contributor
Coccinelle convention checksNo new violations. ✅ Resolved since baseline (3) -- update dist/cocci/baseline.txt to lock these in. |
Contributor
ABI diff vs
|
…s not do The comment called eid an always-distinct discriminator that makes every site agree on the winner. eids are local: repmgr numbers sites in the order it learns of them, so two sites can give the same peer different eids. The eid step makes each site's order total, which is what order independence needs. Agreement between sites rests on the random 32-bit tiebreaker, as it did before; an exact collision costs an extra election round, not a second master. Comment-only; the copy in rep_cmpvote_order.c follows. (cherry picked from commit c6e1030)
Collaborator
Author
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
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.
Backport to
lts/2026.10.7:__rep_cmp_votetotal order (election can elect older data)Cherry-pick (
-x) of the master fix for the__rep_cmp_voteelectioncomparator defect.
lts/2026.10.7has the identical function(
src/rep/rep_elect.c, byte-for-byte the same as master pre-fix).The comparator was not a total order: the first vote was adopted outright and
the equal-LSN arm bypassed
data_gen, so a site with an OLDER data_gen but anequal last-commit LSN and a higher priority/tiebreaker could be elected over a
newer site — rolling back a durable commit on sync (
__rep_verifymemcmp).Fixed as a single total order (electable > not; non-zero priority > zero;
data_gen; LSN; priority; tiebreaker; eid). See the master PR and
test/rep/E1-FINDINGS.mdfor the full analysis, reachability write-up, and themixed-version rolling-upgrade note.
Depends on master PR #266 (merge there first; the
-xtrailer points at9b31ea4ec).LTS gates (dev box, Debian 12)
0xe67a9de8on bothlts/2026.10.7base and thisbackport — unchanged.
libdb-2026.0.so, 1972 exported symbols, identicalsymbol set to base. Comparator-only change; no format/ABI/region change.
test/db/run_rep_cmpvote_order.shPASSES on thebackport (copy-check ok against the now-fixed LTS src); the pre-fix arm
(
-DCMPVOTE_OLD) FAILS (bug reproduced), so the gate has teeth.-x(cherry picked from commit 9b31ea4ec…).(
rep002/005/016/020/022/026/063/067/069/076/093/094) all PASS on thebackport (non-diagnostic
--enable-test --enable-tcl).Note:
--enable-diagnosticrep election tests panic on a pre-existing RFC0010 A3 lock-order artifact in
src/rep/rep_backup.c, reproducing identicallyon the
lts/2026.10.7base — unrelated to this change (flagged to the lead).