diff --git a/.kiro/specs/ha-term-safety/tasks.md b/.kiro/specs/ha-term-safety/tasks.md index ebd6df5..ffb105d 100644 --- a/.kiro/specs/ha-term-safety/tasks.md +++ b/.kiro/specs/ha-term-safety/tasks.md @@ -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. diff --git a/src/controller/testsuite/test_ha_election.c b/src/controller/testsuite/test_ha_election.c index 4eb807e..65e5117 100644 --- a/src/controller/testsuite/test_ha_election.c +++ b/src/controller/testsuite/test_ha_election.c @@ -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);