lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

test(ha-term-safety): controller Election-Safety regression for bug-1095/1096 + absorb into spec

Browse source 
Adds two controller-level regression tests driving the production raft_on_vote_reply
via ha->raft (test_ha_election.c): a duplicated vote-grant from one peer must not
double-count toward quorum (bug-1095) and a stale-term reply must be dropped
(bug-1096) — both asserted on role/promotion (no two leaders per term). Formally
absorbs bug-1095/1096 into the ha-term-safety spec (Implementation Note) mapping to
its Requirement 1 / Objective; the raft.c fix (b5b7e01) is owned there with
protocol-suite regression at both the libxce (test_vote_reply_*) and controller
(test_ha_election) levels. No production source changed.
This commit is contained in:
Rongsong Shen 2026-06-25 02:44:29 +00:00
parent 95629797f8
commit 13c43c23a4
2 changed files with 171 additions and 0 deletions
Download patch file Download diff file Expand all files Collapse all files

5
.kiro/specs/ha-term-safety/tasks.md
View file

@ -222,3 +222,8 @@
- **3.4 done — bug-990 CLOSED (the live-send cutover).** ROOT CAUSE: `repl_broadcast` (the LIVE replicator, persist.c:747) built `wal_catchup_entry_t` without setting `entry.term`, so the live broadcast wire-encoded term=0 even though primary `hdr.term=p->term`; standbys took the legacy term-0 direct-apply and persisted term=0. FIX (MINIMAL cutover, reviewer-justified over the fuller raft_send_append_entries rewrite — the requirement set is met by carrying the term so entries auto-route through the already-wired 3.2 intake + 3.3 ACK): `repl_broadcast` now takes `prev_log_term` + sets `entry.term=hdr->term`/`prev_log_index=lsn-1`/`prev_log_term`; `persist_wal_async` sources prev_log_term via NEW lock-free `entry_term_locked(p, lsn-1)` (predecessor term, under p->lock; lsn==1→(0,0)); catch-up streamer `ha_stream_wal_to_standby` (config_repl.c) carries term+prev tuple per entry (ascending (lsn,term) tracker + conservative segment-skip so the FIRST streamed entry's prev references what the standby already holds); `ha_apply_wal_catchup_entry` (ha.c) routes EVERY term≠0 entry (single + each batch sub-entry, in order, guarded raft_c==c) through `raft_on_append_entries_request`→on_persist_entry (leader term stamped) — retiring the term-0 direct-apply for live/catch-up; `wal_entry_to_wire` (request.c) encodes prevLog{Index,Term}+leaderCommit PER SUB-ENTRY. 0-entry heartbeat still routes through the library for UpdateTerm (persists nothing). bug-850 contiguity + persist-before-ACK + commit-barrier + bug-960 peer-resolution all preserved. Live commit chain now genuinely end-to-end (term-carrying send → standby on_persist_entry + ACK → ha_raft_on_repl_ack → raft_advance_commit → on_commit_advanced → barrier). 2 new tests (send: `Expected 7 Was 0`; receive via the REAL `ha_apply_wal_catchup_entry` entry point: `Expected 7 Was 3`), RED independently reproduced by the reviewer. 175 Tests 0 Failures. Reviewer APPROVED.
- **REMAINING (ENV-only, NOT code):** the definitive LIVE proof — clean failover → byte-identical `(term,lsn)` WALs via `clean_failover_preservation.sh XCE_FAILOVER_STRICT_TERM=1` — could NOT run here: this overlayfs sandbox can't complete a failover on baseline OR the change (split-vote / `fdatasync errno=5`, bug-924). The fix is proven at the integration level (real receive path, real wire shape). 5.2/5.4 + the live half of 5.5 need a REAL (non-overlayfs) fs to run the harness; the harness + STRICT_TERM gate are ready and should flip GREEN there.
- **bug-1095 + bug-1096 ABSORBED (Requirement 1 / Objective "two leaders never share a term"; bug-851 family) — owned here, fixed cross-boundary in libxce `raft.c` with user authorization.** _Surfaced by the raft-sim-conformance oracle (task 4.2), absorbed into ha-term-safety because the defect is an Election-Safety violation in the controller's SOLE production promotion driver: `vote_reply_dispatch` (ha.c ~4336) → `raft_on_vote_reply(&ha->raft, idx, vote_granted, term)` (ha.c ~4361)._
- **The defect:** `raft_on_vote_reply` tallied vote grants with a plain counter that ignored `from_idx` and had no stale-term drop. (1) **bug-1095** — a duplicated vote-response (legal `DuplicateMessage`) from ONE peer double-counted toward quorum, so two candidates could each cross majority and become **leader at the SAME term**. (2) **bug-1096** — a grant carrying `mterm < currentTerm` (a stale, prior-election reply) was counted toward the CURRENT election, again admitting a spurious quorum. Both break Election Safety (`raft.tla` `votesGranted` is a SET; `DropStaleResponse` discards `mterm < currentTerm`).
- **The fix (commit `b5b7e01`, libxce `raft.c` only — NOT touched by this task):** added a per-peer `vote_replies[RAFT_MAX_PEERS]` idempotency set (a duplicate `from_idx` is dropped before the tally) + a `term < s->term` `DropStaleResponse` that discards the stale reply WITHOUT stepping the candidate down — mirroring the already-present pre-vote dedup idiom (`pre_vote_replies[]`).
- **Regression coverage (BOTH levels):** protocol unit — libxce `test_vote_reply_duplicate_grant_not_double_counted` + `test_vote_reply_stale_term_dropped` (`src/lib/testsuite/test_raft.c`); controller level — NEW `test_vote_reply_duplicate_grant_does_not_promote` + `test_vote_reply_stale_term_does_not_count_or_promote` (`src/controller/testsuite/test_ha_election.c`). The controller tests drive the REAL `raft_on_vote_reply` against the controller's own `ha->raft` instance (re-inited to a 5-node cluster; `xce_vote_reply_hook` NULL, `raft_c` NULL so the callbacks take their test-mode paths and `raft_cb_become_leader` still sets `ha->role = CTRL_PRIMARY`) and assert on the resulting **role / promotion** (Election Safety), not an internal counter — a duplicate grant must NOT promote at 2-of-5, and a stale grant must NOT count or promote (and must NOT step the candidate down). Both PASS over the fixed library; they would FAIL without the dedup / stale-drop. `test_ha_election` group: 0 failures.

166
src/controller/testsuite/test_ha_election.c
View file

@ -938,6 +938,167 @@ static void test_reply_handler_all_denied_posts_done_on_last_reply(void)
printf("test_reply_handler_all_denied_posts_done_on_last_reply: PASS\n");
}
/* ── bug-1095 / bug-1096: controller-level Election Safety guard ──────────────
*
* Ownership: ha-term-safety, Requirement 1 / Objective — "two leaders never
* share a term" (bug-851 family). The defects live in libxce raft.c
* `raft_on_vote_reply`, which is the SOLE production promotion driver: the
* controller's `vote_reply_dispatch` (ha.c) calls
* `raft_on_vote_reply(&ha->raft, idx, vote_granted, term)`.
*
* bug-1095 — duplicate grant double-count: a counter-based tally that ignored
* `from_idx` let one peer's DUPLICATED vote-response count twice
* toward quorum → two leaders at the same term. Fixed by a per-peer
* `vote_replies[RAFT_MAX_PEERS]` set (commit b5b7e01).
* bug-1096 — stale-term grant counted: a reply with `mterm < currentTerm` was
* tallied toward the CURRENT election. Fixed by a
* `term < s->term` DropStaleResponse that discards the reply WITHOUT
* stepping the candidate down (commit b5b7e01).
*
* These tests drive the REAL `raft_on_vote_reply` against the CONTROLLER's
* `ha->raft` instance — exactly the object production drives — NOT a bypassing
* `xce_vote_reply_hook` (the hook is NULL here, and we call the library entry
* point directly the way `vote_reply_dispatch` does). Assertions are on the
* resulting ROLE / promotion (Election Safety), not an internal counter, so a
* premature promotion on the duplicate (bug-1095) or any count/promotion on the
* stale reply (bug-1096) would FAIL the test. With the fix in place these are
* the GREEN over the fixed library.
*
* Protocol-level companions live in libxce:
* test_vote_reply_duplicate_grant_not_double_counted
* test_vote_reply_stale_term_dropped (src/lib/testsuite/test_raft.c)
*
* Surfaced by the raft-sim-conformance oracle (task 4.2); fixed cross-boundary
* with user authorization.
*
* Fixture note: ha_init leaves ha->raft with the placeholder n_peers=1 (which
* would self-elect immediately). We re-init ha->raft for a 5-node cluster by
* reusing the callbacks/ctx that ha_init already wired (s_real_cbs is file-
* static in ha.c, but the live pointer is preserved in ha->raft.cbs/.ctx).
* ha->raft_c stays NULL, so every callback takes its NULL-safe test-mode path
* (raft_cb_become_leader still sets ha->role = CTRL_PRIMARY outside its c-guard).
*/
static void reinit_ctrl_raft_n(ha_state_t *ha, int n_peers, uint64_t initial_term)
{
raft_config_t rcfg;
memset(&rcfg, 0, sizeof(rcfg));
rcfg.n_peers = n_peers;
rcfg.self_idx = ha->my_priority; /* self_idx == priority by convention */
rcfg.my_priority = ha->my_priority;
rcfg.initial_term = initial_term;
rcfg.initial_voted_for = -1;
/* Reuse the production callbacks/ctx ha_init already installed on ha->raft. */
rcfg.cbs = ha->raft.cbs;
rcfg.ctx = ha->raft.ctx;
TEST_ASSERT_EQUAL_INT(0, raft_init(&ha->raft, &rcfg));
}
/* bug-1095: a DUPLICATED vote grant from one peer must NOT count twice toward
* quorum, so it must NOT promote the candidate before a DISTINCT peer grants. */
static void test_vote_reply_duplicate_grant_does_not_promote(void)
{
/* 5-node cluster: quorum = 3. self-vote = 1. One distinct grant → 2 (< 3).
* A SECOND grant from the SAME peer must stay at 2 (set semantics) — the
* candidate must NOT become leader on the duplicate. Only a grant from a
* DISTINCT peer raises the tally to 3 = quorum → promotion. */
ha_state_t ha;
ha_init(&ha, &g_elect_cfg, 0); /* self_idx = 0, raft_c = NULL */
reinit_ctrl_raft_n(&ha, 5, /*initial_term=*/4ULL);
/* Begin the election: term → 5, role → CANDIDATE, votes_granted = 1 (self). */
raft_start_election(&ha.raft);
raft_snapshot_t snap;
raft_snapshot(&ha.raft, &snap);
TEST_ASSERT_EQUAL_UINT64(5ULL, snap.term);
TEST_ASSERT_EQUAL_INT(RAFT_CANDIDATE, (int)snap.role);
/* Peer 1 GRANTS at the current term → votes_granted = 2 (< quorum 3). */
raft_on_vote_reply(&ha.raft, /*from_idx=*/1, /*granted=*/1, /*term=*/5ULL);
raft_snapshot(&ha.raft, &snap);
TEST_ASSERT_EQUAL_INT_MESSAGE(RAFT_CANDIDATE, (int)snap.role,
"one distinct grant (2 of 5) must NOT reach quorum (3)");
TEST_ASSERT_EQUAL_INT_MESSAGE((int32_t)CTRL_STANDBY,
(int32_t)atomic_load_explicit(&ha.role, memory_order_acquire),
"controller must NOT be CTRL_PRIMARY after one grant");
/* Peer 1 GRANTS AGAIN — a duplicate (legal DuplicateMessage). bug-1095: this
* must be dropped by the per-peer vote_replies[] set and must NOT raise the
* tally to 3 / promote. WITHOUT the dedup this duplicate would push
* votes_granted to 3 and prematurely elect the candidate → two leaders. */
raft_on_vote_reply(&ha.raft, /*from_idx=*/1, /*granted=*/1, /*term=*/5ULL);
raft_snapshot(&ha.raft, &snap);
TEST_ASSERT_EQUAL_INT_MESSAGE(RAFT_CANDIDATE, (int)snap.role,
"bug-1095: a DUPLICATE grant from the same peer must NOT count twice / "
"must NOT promote (Election Safety: votesGranted is a SET)");
TEST_ASSERT_EQUAL_INT_MESSAGE((int32_t)CTRL_STANDBY,
(int32_t)atomic_load_explicit(&ha.role, memory_order_acquire),
"controller role must still be CTRL_STANDBY after the duplicate grant");
TEST_ASSERT_EQUAL_INT_MESSAGE(2, snap.votes_granted,
"duplicate grant must leave votes_granted at 2 (self + one distinct peer)");
/* A grant from a DISTINCT peer (peer 2) now reaches quorum (3 of 5) → leader.
* This proves the duplicate above was the ONLY thing withheld — quorum is
* reachable, just not by double-counting one peer. */
raft_on_vote_reply(&ha.raft, /*from_idx=*/2, /*granted=*/1, /*term=*/5ULL);
raft_snapshot(&ha.raft, &snap);
TEST_ASSERT_EQUAL_INT_MESSAGE(RAFT_LEADER, (int)snap.role,
"a DISTINCT third grant must reach quorum (3 of 5) and promote");
TEST_ASSERT_EQUAL_INT_MESSAGE((int32_t)CTRL_PRIMARY,
(int32_t)atomic_load_explicit(&ha.role, memory_order_acquire),
"controller must become CTRL_PRIMARY once quorum is reached by distinct grants");
ha_fini(&ha);
printf("test_vote_reply_duplicate_grant_does_not_promote: PASS\n");
}
/* bug-1096: a vote reply carrying a STALE term (term < currentTerm) must be
* dropped — not counted toward this election and not a step-down to follower. */
static void test_vote_reply_stale_term_does_not_count_or_promote(void)
{
/* 5-node cluster: quorum = 3. Candidate at term 5. A grant advertising an
* OLD term (3) must be discarded by DropStaleResponse: it must NOT raise the
* tally, must NOT promote, and must NOT step the candidate down. */
ha_state_t ha;
ha_init(&ha, &g_elect_cfg, 0);
reinit_ctrl_raft_n(&ha, 5, /*initial_term=*/4ULL);
raft_start_election(&ha.raft); /* term → 5, CANDIDATE, votes = 1 */
raft_snapshot_t snap;
raft_snapshot(&ha.raft, &snap);
TEST_ASSERT_EQUAL_UINT64(5ULL, snap.term);
TEST_ASSERT_EQUAL_INT(RAFT_CANDIDATE, (int)snap.role);
/* Stale grant: peer 1 replies GRANTED but with term 3 (< current term 5).
* bug-1096: this must be dropped — votes_granted must stay 1, role must stay
* CANDIDATE (no premature promotion, no spurious step-down). */
int rc = raft_on_vote_reply(&ha.raft, /*from_idx=*/1, /*granted=*/1, /*term=*/3ULL);
TEST_ASSERT_EQUAL_INT_MESSAGE(RAFT_PENDING, rc,
"bug-1096: a stale-term grant must be DROPPED (RAFT_PENDING), not counted");
raft_snapshot(&ha.raft, &snap);
TEST_ASSERT_EQUAL_INT_MESSAGE(RAFT_CANDIDATE, (int)snap.role,
"bug-1096: a stale-term reply must NOT promote AND must NOT step the "
"candidate down to follower (DropStaleResponse)");
TEST_ASSERT_EQUAL_INT_MESSAGE(1, snap.votes_granted,
"stale-term grant must NOT be counted — votes_granted stays at 1 (self)");
TEST_ASSERT_EQUAL_UINT64_MESSAGE(5ULL, snap.term,
"a stale (lower) reply term must NOT lower the candidate's currentTerm");
TEST_ASSERT_EQUAL_INT_MESSAGE((int32_t)CTRL_STANDBY,
(int32_t)atomic_load_explicit(&ha.role, memory_order_acquire),
"controller must NOT be CTRL_PRIMARY after a stale-term grant");
/* Sanity: a fresh-term grant from a DISTINCT peer still works after the stale
* one was dropped — the election is alive and only the stale reply was lost. */
raft_on_vote_reply(&ha.raft, /*from_idx=*/1, /*granted=*/1, /*term=*/5ULL);
raft_on_vote_reply(&ha.raft, /*from_idx=*/2, /*granted=*/1, /*term=*/5ULL);
raft_snapshot(&ha.raft, &snap);
TEST_ASSERT_EQUAL_INT_MESSAGE(RAFT_LEADER, (int)snap.role,
"two fresh-term distinct grants reach quorum (3 of 5) after the stale drop");
ha_fini(&ha);
printf("test_vote_reply_stale_term_does_not_count_or_promote: PASS\n");
}
/* ── bug-828: safe-commit-from-votes must reflect the voted majority ─────────
*
* Production root cause: vote_reply_dispatch decoded the granting voter's
@ -1111,6 +1272,11 @@ void test_ha_election(void)
RUN_TEST(test_reply_handler_partial_replies_do_not_post_done);
RUN_TEST(test_reply_handler_all_denied_posts_done_on_last_reply);
/* bug-1095 / bug-1096 — controller-level Election Safety guard over the real
* raft_on_vote_reply (ha-term-safety R1 / "two leaders never share a term") */
RUN_TEST(test_vote_reply_duplicate_grant_does_not_promote);
RUN_TEST(test_vote_reply_stale_term_does_not_count_or_promote);
/* bug-828 — safe-commit-from-votes must reflect the granting voter LSN */
RUN_TEST(test_safe_commit_reflects_granting_voter_lsn);