diff --git a/src/rep/rep_elect.c b/src/rep/rep_elect.c index 20589d85da..58a0e22fef 100644 --- a/src/rep/rep_elect.c +++ b/src/rep/rep_elect.c @@ -1106,64 +1106,68 @@ __rep_cmp_vote(env, rep, eid, lsnp, priority, gen, data_gen, tiebreaker, flags) u_int32_t priority; u_int32_t data_gen, flags, gen, tiebreaker; { - int cmp, like_pri; + int better, cmp, electable; cmp = LOG_COMPARE(lsnp, &rep->w_lsn); + /* - * If we've seen more than one, compare us to the best so far. - * If we're the first, make ourselves the winner to start. + * Choose the election winner by a single TOTAL ORDER, so that every + * site that tallies the same set of votes picks the same winner no + * matter what order the votes arrived in. The previous code (1) made + * the first vote the winner outright and (2) let its last arm (equal + * LSN => higher priority / tiebreaker) fire 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. Both make the + * result order-dependent; (2) can elect a site with older data and roll + * back a commit the newer site already had -- a durability bug, not + * just a failure to settle. The order below is the lexicographic one + * the comment has always claimed, now applied uniformly: + * + * 1. electable beats not-electable; + * 2. non-zero priority beats zero priority (mixed-version case: an + * old site sets priority 0 but ELECTABLE so it can still vote, + * yet a current-version non-zero-priority site should win); + * 3. higher data_gen (more recent generation of data) wins; + * 4. higher LSN wins; + * 5. higher priority value wins; + * 6. higher tiebreaker wins; + * 7. higher eid wins -- the final discriminator, so the order is + * TOTAL on this site even if two votes tie on everything above. + * eids are local (repmgr numbers sites in the order it learns of + * them; base API eids are the application's), so this step makes + * each site independent of arrival order but does not make + * different sites agree. The random 32-bit tiebreaker does that, + * except on an exact collision; a collision costs at most an + * extra election round, never a second master. + * + * A vote is electable iff it has non-zero priority OR carries the + * ELECTABLE flag (the mixed-version semantics the comment describes). + * rep->sites is the arrival position of this vote (__rep_tally does + * (*countp)++ before calling us), so sites==1 is the first vote of this + * election; it is compared against an empty winner the same way as the + * rest, so there is no first-vote special case to make the choice + * order-dependent. (rep->winner is NOT reset between elections, so we + * key the "empty winner" test off sites==1, not off rep->winner.) A + * non-electable vote never becomes the winner, so whenever there is a + * running winner (sites>1) it is electable. */ - if (rep->sites > 1 && - (priority != 0 || LF_ISSET(REPCTL_ELECTABLE))) { - /* - * Special case, if we have a mixed version group of sites, - * we set priority to 0, but set the ELECTABLE flag so that - * all sites talking at lower versions can correctly elect. - * If a non-zero priority comes in and current winner is - * zero priority (but was electable), then the non-zero - * site takes precedence no matter what its LSN is. - * - * Then the data_gen determines the winner. The site with - * the more recent generation of data wins. - * - * Then LSN is determinant only if we're comparing - * like-styled version/priorities at the same data_gen. I.e. - * both with 0/ELECTABLE priority or both with non-zero - * priority. Then actual priority value if LSNs - * are equal, then tiebreaker if both are equal. - */ + electable = (priority != 0 || LF_ISSET(REPCTL_ELECTABLE)); + + if (rep->sites == 1) { /* - * Make note if we're comparing the same types of priorities - * that indicate electability or not. We know we are - * electable if we are here. + * First vote: an electable vote becomes the winner; a + * non-electable one leaves the winner empty. We must set the + * winner fields explicitly (they are stale from a prior + * election), so handle this case directly rather than via the + * "better" comparison below. */ - like_pri = (priority == 0 && rep->w_priority == 0) || - (priority != 0 && rep->w_priority != 0); - - if ((priority != 0 && rep->w_priority == 0) || - (like_pri && data_gen > rep->w_datagen) || - (like_pri && data_gen == rep->w_datagen && cmp > 0) || - (cmp == 0 && (priority > rep->w_priority || - (priority == rep->w_priority && - (tiebreaker > rep->w_tiebreaker))))) { - RPRINT(env, (env, DB_VERB_REP_ELECT, - "Accepting new vote")); + if (electable) { rep->winner = eid; rep->w_priority = priority; rep->w_lsn = *lsnp; rep->w_gen = gen; rep->w_datagen = data_gen; rep->w_tiebreaker = tiebreaker; - } - } else if (rep->sites == 1) { - if (priority != 0 || LF_ISSET(REPCTL_ELECTABLE)) { - /* Make ourselves the winner to start. */ - rep->winner = eid; - rep->w_priority = priority; - rep->w_gen = gen; - rep->w_datagen = data_gen; - rep->w_lsn = *lsnp; - rep->w_tiebreaker = tiebreaker; } else { rep->winner = DB_EID_INVALID; rep->w_priority = 0; @@ -1172,6 +1176,47 @@ __rep_cmp_vote(env, rep, eid, lsnp, priority, gen, data_gen, tiebreaker, flags) ZERO_LSN(rep->w_lsn); rep->w_tiebreaker = 0; } + return; + } + + /* + * sites>1: compare this vote against the running (electable) winner by + * the total order. A non-electable vote never displaces the winner. + */ + if (!electable) + better = 0; + else if (rep->winner == DB_EID_INVALID) + /* Winner slot is empty (every earlier vote was non-electable): + * any electable vote takes it. */ + better = 1; + else if ((priority != 0) != (rep->w_priority != 0)) + /* 2: non-zero priority beats zero priority. */ + better = (priority != 0 && rep->w_priority == 0); + else if (data_gen != rep->w_datagen) + /* 3: higher data_gen wins. */ + better = (data_gen > rep->w_datagen); + else if (cmp != 0) + /* 4: higher LSN wins. */ + better = (cmp > 0); + else if (priority != rep->w_priority) + /* 5: higher priority value wins. */ + better = (priority > rep->w_priority); + else if (tiebreaker != rep->w_tiebreaker) + /* 6: higher tiebreaker wins. */ + better = (tiebreaker > rep->w_tiebreaker); + else + /* 7: higher eid wins -- local, so it fixes the order on this + * site only (see step 7 above). */ + better = (eid > rep->winner); + + if (better) { + RPRINT(env, (env, DB_VERB_REP_ELECT, "Accepting new vote")); + rep->winner = eid; + rep->w_priority = priority; + rep->w_lsn = *lsnp; + rep->w_gen = gen; + rep->w_datagen = data_gen; + rep->w_tiebreaker = tiebreaker; } } diff --git a/test/MANIFEST b/test/MANIFEST index e62c9715c1..d22e265377 100644 --- a/test/MANIFEST +++ b/test/MANIFEST @@ -516,6 +516,7 @@ db run_foreign_reassoc db run_r1_revive db run_r2_dup_revive db run_r2_fuzz +db run_rep_cmpvote_order db run_typed_footprint db run_typed_old_reader db run_typed_util diff --git a/test/db/check_rep_cmpvote_copy.sh b/test/db/check_rep_cmpvote_copy.sh new file mode 100755 index 0000000000..01e1693e2b --- /dev/null +++ b/test/db/check_rep_cmpvote_copy.sh @@ -0,0 +1,59 @@ +#!/bin/sh +# +# $Id$ +# +# check_rep_cmpvote_copy.sh -- the verbatim copy of __rep_cmp_vote embedded in +# rep_cmpvote_order.c must still match the function in src/rep/rep_elect.c. A +# gate that copies a static function proves a property of the COPY; if src/ +# changes and the copy does not, the gate silently tests code that no longer +# ships. Same discipline as test/cbmc/check_copies.sh. +# +# The copy is marked: +# /* --- BEGIN verbatim copy of __rep_cmp_vote from src/rep/rep_elect.c --- */ +# ... +# /* --- END verbatim copy --- */ +# Comments, blank lines and whitespace differences are ignored; every code +# token is compared. Exit 0 if it matches, 1 otherwise. + +set -u +_HERE=$(cd "$(dirname "$0")" && pwd) +root="$_HERE/../.." +h="$_HERE/rep_cmpvote_order.c" +func=__rep_cmp_vote +src="src/rep/rep_elect.c" +tmp=$(mktemp -d) +rc=0 + +[ -f "$root/$src" ] || { echo "MISSING source: $root/$src"; exit 1; } + +# The function in src/ runs from the line "(" to the next line that is +# exactly "}". +awk -v f="$func" 'index($0, f "(") == 1 {p=1} p {print} p && $0 == "}" {exit}' \ + "$root/$src" > "$tmp/src" + +# The copy runs from its BEGIN marker's following "(" line to the next +# line that is exactly "}". +ln=$(grep -n 'BEGIN verbatim copy of __rep_cmp_vote' "$h" | head -1 | cut -d: -f1) +[ -n "$ln" ] || { echo "no BEGIN marker in $h"; exit 1; } +awk -v start="$ln" -v f="$func" 'NR > start && index($0, f "(") == 1 {p=1} + p {print} p && $0 == "}" {exit}' "$h" > "$tmp/cpy" + +# Strip C comments and blank lines from both sides; compare code tokens. +for x in src cpy; do + sed -e ':a' -e '/\/\*/!b' -e '/\*\//!{N;ba' -e '}' -e 's:/\*.*\*/::' \ + "$tmp/$x" | grep -v '^[[:space:]]*$' > "$tmp/$x.nc" + mv "$tmp/$x.nc" "$tmp/$x" +done + +if [ ! -s "$tmp/src" ]; then + echo "NOT FOUND in $src: $func"; rc=1 +elif diff -bB "$tmp/src" "$tmp/cpy" > "$tmp/diff"; then + : +else + echo "DRIFT: embedded $func differs from $src:" + sed 's/^/ /' "$tmp/diff" | head -40 + rc=1 +fi + +rm -rf "$tmp" +exit "$rc" diff --git a/test/db/rep_cmpvote_order.c b/test/db/rep_cmpvote_order.c new file mode 100644 index 0000000000..c809d7f778 --- /dev/null +++ b/test/db/rep_cmpvote_order.c @@ -0,0 +1,396 @@ +/*- + * Copyright (c) 2026 libdb contributors. All rights reserved. + * + * SPDX-License-Identifier: Sleepycat + * + * See the file LICENSE for redistribution information. + */ + +/* + * rep_cmpvote_order.c -- regression gate for the __rep_cmp_vote election + * comparator (src/rep/rep_elect.c). The comparator must be a TOTAL ORDER so + * that every site tallying the same set of VOTE1s picks the same winner, and + * so that the winner has the most recent data (maximum data_gen). The old + * comparator was not: + * + * - it special-cased the first vote (sites==1), adopting it outright, and + * - its 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 NEWER-data_gen + * vote. Electing a site with older data rolls back (loses) a commit the + * newer site already had -- a durability bug. + * + * This runner feeds the comparator the vote vectors CAPTURED from real sites + * on the dev box (see test/rep/E1-FINDINGS.md): three electable sites with + * EQUAL last-commit LSN and DIFFERENT data_gen, the exact precondition shown + * reachable against the live engine. It tallies them in ALL arrival orders + * (3 votes => all 6 permutations; and a 2-vote stale-vs-newer case) and + * asserts: + * (A) order independence: every arrival order yields the same winner; + * (E) durability: the winner's data_gen is the maximum among same-priority- + * class electable votes (no newer-data site is passed over). + * + * It embeds a VERBATIM copy of __rep_cmp_vote (the function is static and its + * .c pulls in the whole replication subsystem), guarded against drift from + * src/ by the BEGIN/END markers below and check_rep_cmpvote_copy.sh. Build + * -DCMPVOTE_OLD to compile the pre-fix comparator: the gate MUST then FAIL + * (that is its sabotage arm, proving the teeth). + * + * Exits 0 on PASS, non-zero on FAIL. No libdb link needed. + */ + +#include +#include +#include + +typedef uint32_t u_int32_t; +typedef struct __db_lsn { u_int32_t file; u_int32_t offset; } DB_LSN; +typedef struct __env ENV; + +typedef struct __rep { + u_int32_t sites; + int winner; + u_int32_t w_priority; + u_int32_t w_gen; + u_int32_t w_datagen; + DB_LSN w_lsn; + u_int32_t w_tiebreaker; +} REP; + +#define DB_EID_INVALID (-1) +#define REPCTL_ELECTABLE 0x01 + +/* Mirrors src/dbinc/db_int.in. */ +#define LF_ISSET(f) ((flags) & (f)) +#define LOG_COMPARE(lsn0, lsn1) \ + ((lsn0)->file != (lsn1)->file ? \ + ((lsn0)->file < (lsn1)->file ? -1 : 1) : \ + ((lsn0)->offset != (lsn1)->offset ? \ + ((lsn0)->offset < (lsn1)->offset ? -1 : 1) : 0)) +#define ZERO_LSN(LSN) do { (LSN).file = 0; (LSN).offset = 0; } while (0) +#define DB_VERB_REP_ELECT 0 +#define RPRINT(env, x) do { (void)(env); } while (0) + +#ifdef CMPVOTE_OLD +/* + * Pre-fix __rep_cmp_vote (the sabotage arm). NOT a verbatim copy of any + * current src/ function; a frozen record of the defect so the gate has teeth. + */ +static void +__rep_cmp_vote(env, rep, eid, lsnp, priority, gen, data_gen, tiebreaker, flags) + ENV *env; REP *rep; int eid; DB_LSN *lsnp; + u_int32_t priority; u_int32_t data_gen, flags, gen, tiebreaker; +{ + int cmp, like_pri; + + cmp = LOG_COMPARE(lsnp, &rep->w_lsn); + if (rep->sites > 1 && + (priority != 0 || LF_ISSET(REPCTL_ELECTABLE))) { + like_pri = (priority == 0 && rep->w_priority == 0) || + (priority != 0 && rep->w_priority != 0); + if ((priority != 0 && rep->w_priority == 0) || + (like_pri && data_gen > rep->w_datagen) || + (like_pri && data_gen == rep->w_datagen && cmp > 0) || + (cmp == 0 && (priority > rep->w_priority || + (priority == rep->w_priority && + (tiebreaker > rep->w_tiebreaker))))) { + rep->winner = eid; + rep->w_priority = priority; + rep->w_lsn = *lsnp; + rep->w_gen = gen; + rep->w_datagen = data_gen; + rep->w_tiebreaker = tiebreaker; + } + } else if (rep->sites == 1) { + if (priority != 0 || LF_ISSET(REPCTL_ELECTABLE)) { + rep->winner = eid; + rep->w_priority = priority; + rep->w_gen = gen; + rep->w_datagen = data_gen; + rep->w_lsn = *lsnp; + rep->w_tiebreaker = tiebreaker; + } else { + rep->winner = DB_EID_INVALID; + rep->w_priority = 0; + rep->w_gen = 0; + rep->w_datagen = 0; + ZERO_LSN(rep->w_lsn); + rep->w_tiebreaker = 0; + } + } +} +#else +/* --- BEGIN verbatim copy of __rep_cmp_vote from src/rep/rep_elect.c --- */ +static void +__rep_cmp_vote(env, rep, eid, lsnp, priority, gen, data_gen, tiebreaker, flags) + ENV *env; + REP *rep; + int eid; + DB_LSN *lsnp; + u_int32_t priority; + u_int32_t data_gen, flags, gen, tiebreaker; +{ + int better, cmp, electable; + + cmp = LOG_COMPARE(lsnp, &rep->w_lsn); + + /* + * Choose the election winner by a single TOTAL ORDER, so that every + * site that tallies the same set of votes picks the same winner no + * matter what order the votes arrived in. The previous code (1) made + * the first vote the winner outright and (2) let its last arm (equal + * LSN => higher priority / tiebreaker) fire 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. Both make the + * result order-dependent; (2) can elect a site with older data and roll + * back a commit the newer site already had -- a durability bug, not + * just a failure to settle. The order below is the lexicographic one + * the comment has always claimed, now applied uniformly: + * + * 1. electable beats not-electable; + * 2. non-zero priority beats zero priority (mixed-version case: an + * old site sets priority 0 but ELECTABLE so it can still vote, + * yet a current-version non-zero-priority site should win); + * 3. higher data_gen (more recent generation of data) wins; + * 4. higher LSN wins; + * 5. higher priority value wins; + * 6. higher tiebreaker wins; + * 7. higher eid wins -- the final discriminator, so the order is + * TOTAL on this site even if two votes tie on everything above. + * eids are local (repmgr numbers sites in the order it learns of + * them; base API eids are the application's), so this step makes + * each site independent of arrival order but does not make + * different sites agree. The random 32-bit tiebreaker does that, + * except on an exact collision; a collision costs at most an + * extra election round, never a second master. + * + * A vote is electable iff it has non-zero priority OR carries the + * ELECTABLE flag (the mixed-version semantics the comment describes). + * rep->sites is the arrival position of this vote (__rep_tally does + * (*countp)++ before calling us), so sites==1 is the first vote of this + * election; it is compared against an empty winner the same way as the + * rest, so there is no first-vote special case to make the choice + * order-dependent. (rep->winner is NOT reset between elections, so we + * key the "empty winner" test off sites==1, not off rep->winner.) A + * non-electable vote never becomes the winner, so whenever there is a + * running winner (sites>1) it is electable. + */ + electable = (priority != 0 || LF_ISSET(REPCTL_ELECTABLE)); + + if (rep->sites == 1) { + /* + * First vote: an electable vote becomes the winner; a + * non-electable one leaves the winner empty. We must set the + * winner fields explicitly (they are stale from a prior + * election), so handle this case directly rather than via the + * "better" comparison below. + */ + if (electable) { + rep->winner = eid; + rep->w_priority = priority; + rep->w_lsn = *lsnp; + rep->w_gen = gen; + rep->w_datagen = data_gen; + rep->w_tiebreaker = tiebreaker; + } else { + rep->winner = DB_EID_INVALID; + rep->w_priority = 0; + rep->w_gen = 0; + rep->w_datagen = 0; + ZERO_LSN(rep->w_lsn); + rep->w_tiebreaker = 0; + } + return; + } + + /* + * sites>1: compare this vote against the running (electable) winner by + * the total order. A non-electable vote never displaces the winner. + */ + if (!electable) + better = 0; + else if (rep->winner == DB_EID_INVALID) + /* Winner slot is empty (every earlier vote was non-electable): + * any electable vote takes it. */ + better = 1; + else if ((priority != 0) != (rep->w_priority != 0)) + /* 2: non-zero priority beats zero priority. */ + better = (priority != 0 && rep->w_priority == 0); + else if (data_gen != rep->w_datagen) + /* 3: higher data_gen wins. */ + better = (data_gen > rep->w_datagen); + else if (cmp != 0) + /* 4: higher LSN wins. */ + better = (cmp > 0); + else if (priority != rep->w_priority) + /* 5: higher priority value wins. */ + better = (priority > rep->w_priority); + else if (tiebreaker != rep->w_tiebreaker) + /* 6: higher tiebreaker wins. */ + better = (tiebreaker > rep->w_tiebreaker); + else + /* 7: higher eid wins -- local, so it fixes the order on this + * site only (see step 7 above). */ + better = (eid > rep->winner); + + if (better) { + RPRINT(env, (env, DB_VERB_REP_ELECT, "Accepting new vote")); + rep->winner = eid; + rep->w_priority = priority; + rep->w_lsn = *lsnp; + rep->w_gen = gen; + rep->w_datagen = data_gen; + rep->w_tiebreaker = tiebreaker; + } +} +/* --- END verbatim copy --- */ +#endif + +struct vote { + int eid; + DB_LSN lsn; + u_int32_t priority; + u_int32_t gen; + u_int32_t data_gen; + u_int32_t tiebreaker; + u_int32_t flags; +}; + +/* Tally votes in a given order (as __rep_tally + __rep_cmp_vote do). */ +static int +tally(const struct vote *v, const int *order, int n, int stale_winner) +{ + REP rep; + int i; + const struct vote *p; + + rep.sites = 0; + rep.winner = stale_winner; /* region is not reset between elections */ + rep.w_priority = 0; + rep.w_gen = 0; + rep.w_datagen = 0; + ZERO_LSN(rep.w_lsn); + rep.w_tiebreaker = 0; + + for (i = 0; i < n; i++) { + p = &v[order[i]]; + rep.sites++; + __rep_cmp_vote(NULL, &rep, p->eid, (DB_LSN *)&p->lsn, + p->priority, p->gen, p->data_gen, p->tiebreaker, p->flags); + } + return (rep.winner); +} + +static int electable(const struct vote *v) +{ + return (v->priority != 0 || (v->flags & REPCTL_ELECTABLE)); +} + +static int idx_of_eid(const struct vote *v, int n, int eid) +{ + int i; + for (i = 0; i < n; i++) + if (v[i].eid == eid) + return (i); + return (-1); +} + +/* All permutations of {0..n-1} (n<=3). */ +static const int perms3[6][3] = { + {0,1,2},{0,2,1},{1,0,2},{1,2,0},{2,0,1},{2,1,0} +}; +static const int perms2[2][2] = { {0,1},{1,0} }; + +static int fails = 0; + +/* + * Check order independence (A) and data_gen-maximality (E) over every arrival + * order, and over a few stale-winner seeds (the region is not reset between + * elections, so rep->winner can carry a bogus value in). + */ +static void +check(const char *name, const struct vote *v, int n) +{ + int seeds[3], ns, s, k, nperm, p, w0, w, i, wi; + const int *order; + + seeds[0] = DB_EID_INVALID; seeds[1] = 99; seeds[2] = 7; ns = 3; + nperm = (n == 3) ? 6 : 2; + + for (s = 0; s < ns; s++) { + w0 = tally(v, (n == 3) ? perms3[0] : perms2[0], n, seeds[s]); + for (p = 1; p < nperm; p++) { + order = (n == 3) ? perms3[p] : perms2[p]; + w = tally(v, order, n, seeds[s]); + if (w != w0) { + printf("FAIL %s: order-dependent winner: " + "perm0 -> eid %d, perm%d -> eid %d " + "(stale seed %d)\n", name, w0, p, w, + seeds[s]); + fails++; + } + } + /* (E) winner has max data_gen among same-prio-class electable. */ + if (w0 != DB_EID_INVALID) { + wi = idx_of_eid(v, n, w0); + for (i = 0; i < n; i++) + if (i != wi && electable(&v[i]) && + (v[i].priority != 0) == + (v[wi].priority != 0) && + v[i].data_gen > v[wi].data_gen) { + printf("FAIL %s: elected eid %d " + "(data_gen %u) over eid %d with " + "NEWER data_gen %u -- durable " + "commit lost (stale seed %d)\n", + name, w0, v[wi].data_gen, + v[i].eid, v[i].data_gen, seeds[s]); + fails++; + } + } + (void)k; + } +} + +int +main(void) +{ + /* + * Vectors captured from real sites (test/rep/E1-FINDINGS.md): three + * electable sites, EQUAL last-commit LSN 1/16402, data_gen A=2, B=C=3. + * A (the stale site) carries a higher priority/tiebreaker; B,C are the + * newer sites. A correct total order must elect a data_gen-3 site. + */ + DB_LSN X = { 1, 16402 }; + struct vote three[3] = { + /* A: stale (data_gen 2), high priority + high tiebreaker. */ + { 2, {1,16402}, 100, 2, 2, 0xFFFFFFF0u, REPCTL_ELECTABLE }, + /* B: newer (data_gen 3), low priority. */ + { 3, {1,16402}, 10, 3, 3, 0x10000000u, REPCTL_ELECTABLE }, + /* C: newer (data_gen 3), low priority. */ + { 4, {1,16402}, 10, 3, 3, 0x20000000u, REPCTL_ELECTABLE }, + }; + /* The minimal 2-vote case: stale-high-priority vs newer-low-priority. */ + struct vote two[2] = { + { 2, {1,16402}, 100, 2, 2, 0xFFFFFFF0u, REPCTL_ELECTABLE }, + { 3, {1,16402}, 10, 3, 3, 0x10000000u, REPCTL_ELECTABLE }, + }; + /* Equal-everything-but-tiebreaker at the SAME data_gen (the clean + * like-priority case the old code already handled; must stay stable). */ + struct vote tie[3] = { + { 2, {1,16402}, 50, 3, 3, 0x11111111u, REPCTL_ELECTABLE }, + { 3, {1,16402}, 50, 3, 3, 0x22222222u, REPCTL_ELECTABLE }, + { 4, {1,16402}, 50, 3, 3, 0x33333333u, REPCTL_ELECTABLE }, + }; + (void)X; + + check("3-site stale-vs-newer", three, 3); + check("2-site stale-vs-newer", two, 2); + check("3-site like-pri tiebreak", tie, 3); + + if (fails == 0) { + printf("rep_cmpvote_order: PASS\n"); + return (0); + } + printf("rep_cmpvote_order: FAIL (%d)\n", fails); + return (1); +} diff --git a/test/db/run_rep_cmpvote_order.sh b/test/db/run_rep_cmpvote_order.sh new file mode 100755 index 0000000000..ffda1b8d9a --- /dev/null +++ b/test/db/run_rep_cmpvote_order.sh @@ -0,0 +1,79 @@ +#!/bin/sh - +# +# $Id$ +# +# run_rep_cmpvote_order.sh -- +# Regression gate for the __rep_cmp_vote election comparator +# (src/rep/rep_elect.c). The comparator must be a TOTAL ORDER so every +# site that tallies the same VOTE1 set agrees on the winner, and so the +# winner has the most recent data (maximum data_gen). The old comparator +# was neither: it special-cased the first vote and let its last arm (equal +# LSN => priority/tiebreaker) bypass data_gen, so a stale (lower-data_gen) +# site with an equal LSN and a higher priority/tiebreaker could be elected +# over a newer site -- rolling back a durable commit on sync. +# +# The precondition (equal last-commit LSN, different data_gen) was shown +# reachable against the live engine (test/rep/E1-FINDINGS.md); this gate +# drives the REAL comparator (a verbatim copy, drift-checked) over those +# captured vote vectors in every arrival order. +# +# Two directions, so the gate has teeth: +# - the fixed comparator PASSES (order-independent, data_gen-maximal); +# - the pre-fix comparator (-DCMPVOTE_OLD) MUST FAIL. +# +# Exits non-zero on failure. + +set -e + +BUILD=${BUILD:-.} +_HERE=$(cd "$(dirname "$0")" && pwd) +SRC=${SRC:-$_HERE/rep_cmpvote_order.c} +CC=${CC:-cc} +TMP=${TMP:-rep_cmpvote_order_tmp} + +mkdir -p "$TMP" + +echo "Compiling rep_cmpvote_order (fixed comparator)" +"$CC" -g -O1 ${CFLAGS:-} "$SRC" -o "$TMP/cmpvote_fixed" + +echo "Compiling rep_cmpvote_order (pre-fix comparator, sabotage)" +"$CC" -g -O1 ${CFLAGS:-} -DCMPVOTE_OLD "$SRC" -o "$TMP/cmpvote_old" + +rc=0 + +echo "--- fixed comparator (must PASS) ---" +if "$TMP/cmpvote_fixed"; then + echo "fixed: PASS" +else + echo "FAIL: the fixed comparator did not pass" + rc=1 +fi + +echo "--- pre-fix comparator (must FAIL = teeth) ---" +if "$TMP/cmpvote_old"; then + echo "FAIL: the pre-fix comparator PASSED -- the gate has no teeth" + rc=1 +else + echo "pre-fix: FAILED as required (bug reproduced)" +fi + +# Verify the embedded comparator is still a verbatim copy of src/. +CHK="$_HERE/check_rep_cmpvote_copy.sh" +if [ -f "$CHK" ]; then + echo "--- verbatim-copy drift check ---" + if sh "$CHK"; then + echo "copy-check: ok" + else + echo "FAIL: embedded __rep_cmp_vote drifted from src/rep/rep_elect.c" + rc=1 + fi +fi + +rm -rf "$TMP" + +if [ "$rc" -eq 0 ]; then + echo "run_rep_cmpvote_order.sh: PASS" +else + echo "run_rep_cmpvote_order.sh: FAIL" +fi +exit "$rc" diff --git a/test/rep/E1-FINDINGS.md b/test/rep/E1-FINDINGS.md new file mode 100644 index 0000000000..cc8cdd9d4b --- /dev/null +++ b/test/rep/E1-FINDINGS.md @@ -0,0 +1,134 @@ +# E1: `__rep_cmp_vote` is not a total order (election winner selection) + +Workstream E1, 2026-11. Branch `fix/rep-elect-datagen` (master) + +`backport/2026.10.7/rep-elect-datagen` (LTS). Reviews: PR #263 (CBMC H1 +finding), lead's "Counterexample 2" note. + +## The defect + +`__rep_cmp_vote` (src/rep/rep_elect.c) picks the phase-1 election winner. It +was not a consistent order, with two defects: + +1. **First-vote special case.** When `rep->sites == 1` the first electable vote + became the winner outright, so the winner depended on which vote a site + tallied first (CBMC "Counterexample 1"). + +2. **data_gen bypass (the durable one).** The last arm, + `cmp == 0 && (priority > w_priority || (priority == w_priority && + tiebreaker > w_tiebreaker))`, fired whenever the LSNs were equal + **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 that arm B had made the winner (CBMC "Counterexample 2"). + +`data_gen` is the generation of a site's most recent data. Electing a site +with an older data_gen rolls back the newer site's tail on sync -- and because +a client compares the actual **log bytes** at its verify LSN +(`__rep_verify`, src/rep/rep_verify.c:77, `memcmp(mylog.data, rec->data, ...)`), +a wrong winner genuinely truncates the loser's log. A commit the loser had +made durable is lost. That is a durability bug, not just a failure to settle. + +## What is PROVEN + +- **Comparator order dependence / non-total order.** CBMC harness + `test/cbmc/harness_cmpvote.c` property (a) fails on the old code; the + self-contained C gate `test/db/run_rep_cmpvote_order.sh` fails on the old + code (`-DCMPVOTE_OLD`) for the captured vectors (2-site: arrival order flips + the winner between the newer and the stale site). + +- **The precondition is reachable on real sites.** Measured against the live + engine on the dev box (Debian 12, non-diagnostic `--enable-test + --enable-tcl`): two/three electable sites with **EQUAL last-commit LSN** + (1/16402) and **DIFFERENT data_gen** (A=2, B=C=3), carrying those values in + real VOTE1 messages (confirmed via the engine's own `rep_stat` / + `__rep_get_datagen` and instrumented `__rep_cmp_vote` traces, since removed). + + Why it is reachable: a VOTE1 carries (a) the LSN of the site's last + `__txn_regop` (`__rep_log_backup(REP_REC_COMMIT)`, rep_elect.c:396) and + (b) the highest gen key in its REPLSNHIST database (`__rep_get_datagen`, + rep_util.c). These are independent: the last-commit LSN is a byte offset, + the data_gen is a generation counter. A gen change writes one REPLSNHIST + commit of a fixed shape, so two sites that each took one mastership cycle + advance their last-commit LSN by the same amount while recording different + gen keys -- equal LSN, different data_gen. No invariant forbids it. + +## What is ARGUED (not mechanically proven) + +- **That a real network delivers the stale VOTE1 last at a majority.** The + stale (lower-data_gen) site is actually ELECTED only when its VOTE1 is + tallied LAST at a majority of sites: arm B (higher data_gen wins) never gets + to displace it, and arm D then displaces the newer winner on priority or + tiebreaker. VOTE1 arrival order in repmgr is arbitrary network timing, so + "the stale vote arrives last" is an ordinary, non-zero-probability outcome; + combined with the stale site having a higher priority (a designated primary + often does) or simply a higher random tiebreaker (~50%), the stale site wins. + We did not construct a fully deterministic end-to-end TCL election that + settles on the stale site (see below); the claim rests on the proven + precondition plus this delivery-order argument. + +- **Disposition.** A comparator that is not a total order is a defect whether + or not a full election history reproduces it deterministically, and its + precondition is reachable. Fixed rather than left as liveness-only. + +## The fix + +`__rep_cmp_vote` rewritten as a single TOTAL ORDER applied uniformly to every +vote including the first (keyed on `rep->sites == 1`, since `rep->winner` is +not reset between elections -- `__rep_elect_done` resets `sites`/`votes`): + + 1. electable beats not-electable; + 2. non-zero priority beats zero priority (mixed-version ELECTABLE preserved); + 3. higher data_gen wins; + 4. higher LSN wins; + 5. higher priority value wins; + 6. higher tiebreaker wins; + 7. higher eid wins -- the final discriminator, so each site's order is + total. eids are local to each site, so this does not make different + sites agree on a full tie; the random 32-bit tiebreaker does, except on + an exact collision, which costs at most an extra election round. + +## Mixed-version (rolling-upgrade) compatibility + +An election is decided by each site's OWN comparator: a site becomes master +only via `I_HAVE_WON` (its own `rep->winner` AND a majority of VOTE2s), and each +site votes VOTE2 for its own phase-1 winner. Old and new comparators differ +ONLY in the cases where the old code was order-dependent -- the all-tied +first-vote case and the equal-LSN/different-data_gen case. In every other case +(distinct data_gen, or same data_gen with distinct LSN) both versions pick the +same winner, so a mixed group behaves exactly as before. + +In the divergence window a new site picks the higher-data_gen site while an old +site may pick a different one depending on arrival order. Consequence: + +- **No split brain.** Two distinct candidates cannot each gather a majority of + VOTE2s, because any two majorities of the same group intersect (H3, + ELECTION_MAJORITY: `2*nvotes > nsites`). This fix does not touch that. +- **Possible transient stall.** If old and new sites split their VOTE2s in the + divergence window, no candidate reaches a majority and the election retries + at the next egen. egen retries reshuffle VOTE1 arrival timing, and once a + new-version majority exists it deterministically elects the max-data_gen + site, so the stall is transient and self-healing -- the same liveness cost + the old order-dependence already had, now bounded to the upgrade window. + +**Upgrade note for users:** during a rolling upgrade, in the rare event that an +election has two candidates with the SAME last-commit LSN but DIFFERENT data +generations, old and new members may need an extra election round to agree on +the winner; the group stays safe (never two masters) and settles on the member +with the most recent data once a majority runs the new code. Finish the +rolling upgrade promptly, as usual. + +The fix changes WHICH site wins only in cases where the old code was +order-dependent (its winner there was not well-defined); in every +well-defined case the winner is unchanged. + +## Why no deterministic end-to-end TCL gate + +rep200.tcl (advisory, in this branch) reproduces the precondition against the +real engine every run (equal LSN, different data_gen, real VOTE1s) but cannot +deterministically force the stale site to WIN: making the data_gens differ +needs an egen pre-bump, which desynchronises the asynchronous election's egen +so the controlled-delivery election does not settle on demand; a construction +that keeps egens synced yields equal data_gens. The two requirements fight in +the test harness. The deterministic gate is therefore at the comparator level +(test/db/run_rep_cmpvote_order.sh + the CBMC harness), driving the REAL function +over the captured real vote vectors. rep200.tcl is kept as an advisory +precondition demonstration; its header says so. diff --git a/test/tcl/rep200.tcl b/test/tcl/rep200.tcl new file mode 100644 index 0000000000..2e11f09dcd --- /dev/null +++ b/test/tcl/rep200.tcl @@ -0,0 +1,200 @@ +# See the file LICENSE for redistribution information. +# +# Copyright (c) 2026 libdb contributors. All rights reserved. +# +# SPDX-License-Identifier: Sleepycat +# +# $Id$ +# +# TEST rep200 +# TEST ADVISORY reproduction for the __rep_cmp_vote election-comparator defect +# TEST (see test/rep/E1-FINDINGS.md). It builds, against the REAL engine, the +# TEST precondition of the data_gen-bypass bug: three electable sites with an +# TEST EQUAL last-commit LSN but DIFFERENT data_gen (A stale, B/C newer), each +# TEST carrying those values in a real VOTE1. It prints the measured LSNs and +# TEST data generations and attempts an election. +# TEST +# TEST This test is ADVISORY, not a pass/fail gate: the precondition reproduces +# TEST every run, but forcing the asynchronous election to settle on the stale +# TEST site deterministically is not possible through the TCL harness (advancing +# TEST a site's data_gen requires an egen pre-bump that desynchronises the +# TEST election's egen; a construction that keeps egens synced yields equal +# TEST data_gens -- the two requirements conflict). The DETERMINISTIC gate for +# TEST this defect is at the comparator level: test/db/run_rep_cmpvote_order.sh +# TEST and the CBMC harness test/cbmc/harness_cmpvote.c, which drive the REAL +# TEST __rep_cmp_vote over the vote vectors captured here, in every arrival +# TEST order. See test/rep/E1-FINDINGS.md for the full reachability write-up. +proc rep200 { method args } { + source ./include.tcl + if { $checking_valid_methods } { return "btree" } + rep200_run $method +} +proc last_regop_lsn { home } { + source ./include.tcl + set out [eval exec $util_path/db_printlog -h $home] + set last "" + foreach line [split $out "\n"] { + if { [regexp {\[(\d+)\]\[(\d+)\]__txn_regop} $line -> f o] } { set last "$f/$o" } + } + return $last +} +proc mk_master_then_client { env } { + foreach peer {1 2 3 4} { catch {replclear $peer} } + $env rep_start -master + foreach peer {1 2 3 4} { catch {replclear $peer} } + $env rep_start -client + foreach peer {1 2 3 4} { catch {replclear $peer} } +} +proc bump_egen { env eid qdir } { + global elect_serial + incr elect_serial + start_election "bump.$elect_serial" $qdir [$env get_home] $eid 5 5 100 [expr 2*1000000] none 0 + tclsleep 3 + set he 0; catch {replprocessqueue $env $eid 0 he} + catch {cleanup_elections} + foreach peer {1 2 3 4} { catch {replclear $peer} } +} +# Deliver queued messages to $env (eid $machid) in queue order; if $holdfrom >= 0, leave +# queued (do not deliver) any VOTE1 whose sender == $holdfrom. Returns count delivered. +proc deliver_except_vote1_from { dbenv machid holdfrom } { + global queuedbs queueenv + # Snapshot (recno -> {ctrl rec from}) in order, deciding keep/deliver, deleting delivered. + set deliver {} + set txn [$queueenv txn] + set dbc [$queuedbs($machid) cursor -txn $txn] + set dels {} + for { set dbt [$dbc get -first] } { [llength $dbt] != 0 } { set dbt [$dbc get -next] } { + set recno [lindex [lindex $dbt 0] 0] + set data [lindex [lindex $dbt 0] 1] + set ctrl [lindex $data 0]; set rec [lindex $data 1]; set from [lindex $data 2] + if { $holdfrom >= 0 && [berkdb msgtype $ctrl] eq "vote1" && $from == $holdfrom } { + continue + } + lappend deliver [list $ctrl $rec $from] + lappend dels $recno + } + $dbc close + # Delete the ones we will deliver (so they are not reprocessed). + set dc [$queuedbs($machid) cursor -txn $txn] + foreach r $dels { + if { [llength [$dc get -set $r]] != 0 } { $dc del } + } + $dc close + $txn commit + set n 0 + foreach msg $deliver { + catch {$dbenv rep_process_message [lindex $msg 0] [lindex $msg 1] [lindex $msg 2]} + incr n + } + return $n +} +proc rep200_run { method } { + source ./include.tcl + global testdir elect_serial queuedbs queueenv + env_cleanup $testdir + set qdir $testdir/MSGQUEUEDIR + replsetup $qdir + foreach d {M A B C} { file mkdir $testdir/$d } + repladd 1; repladd 2; repladd 3; repladd 4 + set m [eval berkdb_env_noerr -create -txn -event -home $testdir/M -rep_master "-rep_transport {1 replsend}"] + set a [eval berkdb_env_noerr -create -txn -event -home $testdir/A -errpfx A -rep_client "-rep_transport {2 replsend}"] + set b [eval berkdb_env_noerr -create -txn -event -home $testdir/B -errpfx B -rep_client "-rep_transport {3 replsend}"] + set c [eval berkdb_env_noerr -create -txn -event -home $testdir/C -errpfx C -rep_client "-rep_transport {4 replsend}"] + set envlist "{$m 1} {$a 2} {$b 3} {$c 4}" + process_msgs $envlist + eval rep_test $method $m NULL 5 0 0 0 + process_msgs $envlist + $m close + + set elect_serial 50 + mk_master_then_client $a + bump_egen $b 3 $qdir + mk_master_then_client $b + bump_egen $c 4 $qdir + mk_master_then_client $c + + set al [last_regop_lsn $testdir/A]; set bl [last_regop_lsn $testdir/B]; set cl [last_regop_lsn $testdir/C] + set ag [stat_field $a rep_stat {Generation number}] + set bg [stat_field $b rep_stat {Generation number}] + set cg [stat_field $c rep_stat {Generation number}] + puts "\tRep200.precond: A(2) regop=$al dg=$ag ; B(3) regop=$bl dg=$bg ; C(4) regop=$cl dg=$cg" + error_check_good precond_equal_lsn_ab $al $bl + error_check_good precond_equal_lsn_ac $al $cl + error_check_good precond_a_stale [expr {$ag < $bg && $ag < $cg}] 1 + error_check_good precond_bc_newer_same [expr {$bg == $cg && $bg > $ag}] 1 + + # Converge egens: run ONE joint throwaway election (full delivery) so all three agree on + # an egen, then clear events. This does NOT change data_gen (REPLSNHIST is untouched). + foreach e [list $a $b $c] { $e event_info -clear } + foreach peer {1 2 3 4} { catch {replclear $peer} } + set elect_serial 90 + start_election "sync.90" $qdir $testdir/A 2 3 3 100 [expr 3*1000000] none 0 + start_election "sync.91" $qdir $testdir/B 3 3 3 10 [expr 3*1000000] none 0 + start_election "sync.92" $qdir $testdir/C 4 3 3 10 [expr 3*1000000] none 0 + tclsleep 2 + for {set i 0} {$i < 10} {incr i} { + set he 0 + foreach {env id} [list $a 2 $b 3 $c 4] { catch {replprocessqueue $env $id 0 he} } + tclsleep 1 + } + catch {cleanup_elections} + foreach e [list $a $b $c] { $e event_info -clear } + foreach peer {1 2 3 4} { catch {replclear $peer} } + puts "\tRep200.egens: A=[stat_field $a rep_stat {Election generation number}] B=[stat_field $b rep_stat {Election generation number}] C=[stat_field $c rep_stat {Election generation number}]" + + # Start an election on all three at the same egen. A pri 100, B/C pri 10. + set to [expr 30*1000000] + set elect_serial 100 + start_election "A.100" $qdir $testdir/A 2 3 2 100 $to none 0 ; set pA 100 + start_election "B.101" $qdir $testdir/B 3 3 2 10 $to none 0 ; set pB 101 + start_election "C.102" $qdir $testdir/C 4 3 2 10 $to none 0 ; set pC 102 + tclsleep 2 + + # Controlled delivery rounds: to B and C deliver everything EXCEPT A's VOTE1 (hold it), + # so B and C tally the newer votes first. To A deliver everything. Repeat so all VOTE1s + # (except A->B, A->C) propagate. THEN deliver A's held VOTE1 to B and C last. + for {set i 0} {$i < 8} {incr i} { + catch {deliver_except_vote1_from $a 2 -1} + catch {deliver_except_vote1_from $b 3 2} + catch {deliver_except_vote1_from $c 4 2} + foreach {env id pipe} [list $a 2 $pA $b 3 $pB $c 4 $pC] { + catch {check_election $pipe u e} + } + tclsleep 1 + } + puts "\tRep200.predeliver: A_elected=[is_elected $a] B_elected=[is_elected $b] C_elected=[is_elected $c]" + # Now deliver A's stale VOTE1 to B and C LAST (arm D should flip them to A). + for {set i 0} {$i < 10} {incr i} { + catch {deliver_except_vote1_from $b 3 -1} + catch {deliver_except_vote1_from $c 4 -1} + catch {deliver_except_vote1_from $a 2 -1} + foreach {env id pipe} [list $a 2 $pA $b 3 $pB $c 4 $pC] { + catch {check_election $pipe u e} + } + if {[is_elected $a] || [is_elected $b] || [is_elected $c]} break + tclsleep 1 + } + set awon [is_elected $a]; set bwon [is_elected $b]; set cwon [is_elected $c] + puts "\tRep200.winner: A_elected=$awon B_elected=$bwon C_elected=$cwon" + catch {cleanup_elections} + # + # ADVISORY outcome. The precondition (asserted above) is the real result + # of this test and always holds. The election below may or may not settle + # through the TCL harness; we only REPORT what happened. The deterministic + # pass/fail gate is test/db/run_rep_cmpvote_order.sh + the CBMC harness. + # + if {$awon} { + puts "\tRep200: OBSERVED the bug -- the STALE data_gen-$ag site (eid2) was" + puts "\tRep200: elected over newer data_gen-$bg sites at equal LSN (arm D" + puts "\tRep200: bypassed data_gen). Expected on the pre-fix comparator." + } elseif {$bwon || $cwon} { + puts "\tRep200: a newer-data site was elected (not the stale A) -- the" + puts "\tRep200: outcome the fixed total order guarantees." + } else { + puts "\tRep200: election did not settle through the harness (advisory;" + puts "\tRep200: see the comparator-level gate for the deterministic check)." + } + puts "\tRep200 RESULT: PASS (advisory -- precondition reproduced)." + catch {$a close}; catch {$b close}; catch {$c close} + replclose $qdir +}