From f1783f7952c33bc14fe5a28e87ca6ebf57eafb16 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Mon, 22 Jun 2026 09:25:44 +0000 Subject: [PATCH] feat(ha-term-safety): drive ACK path through raft_on_append_entries_reply + advance_commit (task 3.3) Includes fix for bug-960: endpoint-type-aware peer-index resolution (promoted-vote relative vs heartbeat-standby absolute) with fail-loud bounds check, preventing an out-of-bounds match_index[] write / wrong-peer false-quorum on the replication ACK path. --- .kiro/specs/ha-term-safety/tasks.md | 7 +- src/controller/ha.c | 102 +++++- src/controller/ha.h | 41 +++ .../testsuite/test_term_safety_adapter.c | 318 ++++++++++++++++++ 4 files changed, 466 insertions(+), 2 deletions(-) diff --git a/.kiro/specs/ha-term-safety/tasks.md b/.kiro/specs/ha-term-safety/tasks.md index f0ff135..e991735 100644 --- a/.kiro/specs/ha-term-safety/tasks.md +++ b/.kiro/specs/ha-term-safety/tasks.md @@ -78,7 +78,7 @@ - _Boundary: ha intake + ha_convergence_ - _Depends: 2.5_ -- [ ] 3.3 Drive `raft_on_append_entries_reply` and `raft_advance_commit` on the ACK path +- [x] 3.3 Drive `raft_on_append_entries_reply` and `raft_advance_commit` on the ACK path - On a replication ACK, route it through `raft_on_append_entries_reply` to update `match_index[]`/`next_index[]`, then drive `raft_advance_commit` so the library recomputes the current-term-gated commit and may fire `on_commit_advanced` - Observable completion: a quorum ACK advances the library `commit_index` only when the agreed entry is current-term, which then triggers the barrier; a stale-term reply is dropped - _Requirements: 4.1, 4.5_ @@ -177,3 +177,8 @@ - **3.2 done.** Heartbeat/replication intake routes through the library. NEW `ha_consume_append_entries` (ha.c) = library-route intake; `ha_consume_heartbeat_committed` (~5691) now sends a 0-entry AE through `raft_on_append_entries_request` for UpdateTerm (G5) BEFORE the monotonic committed-learn — old term-blind `if(hb->termterm != 0`). So the term-carrying single-entry AE RECEIVE route is UNIT-TEST-ONLY in production until 3.3 wires the library SEND loop (`raft_on_append_entries_reply` + `raft_advance_commit`) AND RETIRES the term-0 `repl_broadcast`. The higher-term-heartbeat UpdateTerm IS live (heartbeats are 0-entry → always go through the library route). Live divergent-tail convergence is validated in 5.2/5.4. - **LOW cleanup (5.5 / cleanup pass):** (1) ha_convergence.c ~292-297 doc-comment on `cluster_apply_ctrlhello_reply` still describes the old "clamped truncate + restart" — stale. (2) `ha.h:197 rejoin_reconciled` atomic is now a dead struct field (once-guard unused). Both harmless/cosmetic. Add to the growing 5.5 cleanup list alongside the `ha_cb_*_stub` rename + stale stub-header comment. + +- **3.3 done (REJECTED round 1, fixed, APPROVED round 2).** ACK path: NEW `ha_raft_on_repl_ack(c, abs_peer_idx, acked_lsn, reply_term)` (ha.c ~2914) calls `raft_on_append_entries_reply` then `raft_advance_commit` → fires `on_commit_advanced` on a strict current-term advance (activates the 2.4 commit gate). Wired into the production walAck success branch (`repl_ack_decode_and_dispatch`, ha.c ~4327); `reply_term = war_raw.term`. §4.1/§4.5 current-term gate owned by library (raft.c G4); stale-term reply dropped library-first. 6 ACK tests; reviewer APPROVED round 2 after a full re-verify. + - **bug-960 (CRITICAL, caught by review — logged in buglog.json):** `ha_raft_on_repl_ack` first applied the votes-index→absolute remap UNCONDITIONALLY, but the walAck call site fires for BOTH promoted-vote eps (`vctx->ctx != NULL`, relative peer_idx) AND heartbeat standby eps (`vctx->ctx == NULL`, ALREADY-absolute peer_idx — see authoritative ha.c ~6172-6189 / ~5808). Over-shift → OOB `match_index[]` write (raft.c:802 has NO bounds check) in a 3-node cluster / wrong-peer FALSE-QUORUM data-loss in larger. FIX: `ha_repl_ack_abs_peer_idx(ha, ep_is_vote, peer_idx)` resolver (vote→remap, heartbeat→passthrough) resolved at the call site via `vctx->ctx != NULL`; `ha_raft_on_repl_ack` now takes ABSOLUTE idx + fail-loud `[0,n_peers)` self-excluding bounds check (logs+returns, never crashes/self-terminates). Unit tests had MASKED it (only exercised votes_idx under my_priority==0). LESSON: any peer-index passed to a library `match_index[]`/`next_index[]` write MUST be resolved per endpoint-type (vote-relative vs heartbeat-absolute) — never blind-remap; test BOTH endpoint types at my_priority 0 AND non-0. + - **DO-NOT-REPEAT (cerebrum):** an implementer ran `clang-format -i` (whole-file reformat) then `git checkout HEAD -- ` to undo it, which WIPED the uncommitted task diff (HEAD was at the prior task's commit). It reconstructed the diff from context and re-review confirmed fidelity, but: never `git checkout HEAD --` to undo a reformat on a file holding uncommitted work; reformat only the changed hunks. + - **STILL DEFERRED to 5.2/5.4 (carried 3.2→3.3):** live steady-state send is still TERM-0 `repl_broadcast`; the library send loop (`on_send_append_entries`) is not yet the live sender, so the ACK→commit path is proven end-to-end in UNIT TESTS but the first LIVE `on_commit_advanced(>=1)` requires retiring term-0 `repl_broadcast` + driving the library send. Live chaotic convergence (and this retirement) is validated/owned by 5.2 (clean failover) + 5.4 (chaotic regression). Phase 3 (drive library entry points) is structurally complete; phase 4 (config marker) + phase 5 (verification) remain. diff --git a/src/controller/ha.c b/src/controller/ha.c index a168f49..064ca13 100644 --- a/src/controller/ha.c +++ b/src/controller/ha.c @@ -2894,6 +2894,86 @@ int ha_primary_log_append(cluster_t *c, uint32_t type, return rc; } +/* ha_repl_ack_abs_peer_idx (ha-term-safety task 3.3 remediation): resolve the + * ABSOLUTE library peer index from an ACK endpoint's stored peer_idx, branching + * on endpoint type EXACTLY as the standby_endpoints scan at ha.c:6187-6189. + * + * Two endpoint types reach the walAck branch of repl_ack_decode_and_dispatch, + * and their stored peer_idx has DIFFERENT meaning: + * - Promoted vote ep (vote_ep_ctx_t, ep_is_vote != 0): peer_idx is a + * self-EXCLUDED "votes" index (relative). Map it back to the absolute index + * — the inverse of raft_cb_send_vote_request's votes_idx remap (self_idx == + * my_priority): abs = votes_idx < self_idx ? votes_idx : votes_idx + 1. + * - Heartbeat standby ep (standby_ep_ctx_t, ctx==NULL, ep_is_vote == 0): + * peer_idx is ALREADY the absolute candidate index — pass it through. + * + * The earlier unconditional remap corrupted the heartbeat path: a standby ep's + * already-absolute index was remapped a second time, crediting the WRONG peer + * (false quorum / stuck commit) or, near the array bound, an OUT-OF-BOUNDS + * match_index[] write. This resolver is the single source of truth so the + * production call site and the unit test exercise identical logic. */ +int ha_repl_ack_abs_peer_idx(ha_state_t *ha, int ep_is_vote, int peer_idx) +{ + int self_idx = ha->my_priority; /* self_idx == my_priority by convention */ + if (ep_is_vote) + return (peer_idx < self_idx) ? peer_idx : peer_idx + 1; + return peer_idx; /* heartbeat standby ep: already absolute */ +} + +/* ha-term-safety task 3.3 (R4.1/R4.5): drive the raft library on a replication + * ACK. Route the reply through raft_on_append_entries_reply (updating + * match_index[]/next_index[]) then raft_advance_commit, so the library + * recomputes the current-term-gated commit (Figure-8) and MAY fire + * on_commit_advanced — the task-2.4 callback feeding the synchronous barrier. + * + * Peer-index contract: `abs_peer_idx` is the ABSOLUTE library peer index + * (j != self_idx), the same index space the library uses for + * match_index[]/next_index[]. The CALLER resolves it per endpoint type via + * ha_repl_ack_abs_peer_idx (promoted-vote eps carry a relative votes index; + * heartbeat standby eps carry an already-absolute index) — this seam must NOT + * remap again or it would corrupt the heartbeat path. + * + * Defense-in-depth: a peer index outside [0, raft.n_peers) (and never self) is + * rejected loudly rather than passed into raft_on_append_entries_reply, whose + * match_index[] write is unchecked. This fences an OOB / wrong-peer write at the + * controller boundary (no assert inside src/lib/raft/) and self-heals — the + * caller simply skips this ACK's library accounting. + * + * The library drops a stale-term reply (reply_term != s->term) inside + * raft_on_append_entries_reply, so a stale ACK never touches match_index[] and + * never advances commit. raft_advance_commit is leader-only (-EPERM otherwise); + * on a non-leader the call is a harmless no-op. Neither library call performs + * I/O — safe on the iomgr ACK path. */ +int ha_raft_on_repl_ack(cluster_t *c, int abs_peer_idx, uint64_t acked_lsn, + uint64_t reply_term) +{ + if (!c) + return -EINVAL; + + /* Defense-in-depth bounds/self check: the library's match_index[] write is + * unchecked, so reject a bogus index here (fail loud, never corrupt). */ + int self_idx = c->ha.my_priority; + int n_peers = c->ha.raft.n_peers; + if (abs_peer_idx < 0 || abs_peer_idx >= n_peers || abs_peer_idx == self_idx) { + ERROR("ha_raft_on_repl_ack: rejecting out-of-range peer index %d " + "(n_peers=%d self_idx=%d) — skipping library ACK accounting", + abs_peer_idx, n_peers, self_idx); + return -EINVAL; + } + + /* A successful WalAck reports the standby's matched index (acked_lsn). The + * library applies it only when reply_term == s->term (stale-term drop). */ + (void)raft_on_append_entries_reply(&c->ha.raft, abs_peer_idx, /*success*/1, + acked_lsn, reply_term); + + /* Recompute the current-term-gated commit; fires on_commit_advanced exactly + * once on a strict advance of a current-term entry (else holds). Leader-only: + * -EPERM off the leader leaves commit_index unchanged with no callback. */ + (void)raft_advance_commit(&c->ha.raft); + + return 0; +} + /* * ha_arm_own_term_gate: append the own-term WAL_NOOP and arm the commit * engine's writable_lsn gate (ha-single-leader-role task 2.2, Req 1.4/2.1/2.2). @@ -4217,7 +4297,8 @@ static int repl_ack_decode_and_dispatch(endpoint_t *ep, vote_ep_ctx_t *vctx) struct WalAckReply war_raw; memset(&war_raw, 0, sizeof(war_raw)); read_WalAckReply(&war_raw, rep_s.data.walackreplymsg); - uint64_t acked_lsn = (uint64_t)war_raw.lsn; + uint64_t acked_lsn = (uint64_t)war_raw.lsn; + uint64_t reply_term = (uint64_t)war_raw.term; int success = war_raw.success ? 1 : 0; capn_free(&capn_ctx); @@ -4225,6 +4306,25 @@ static int repl_ack_decode_and_dispatch(endpoint_t *ep, vote_ep_ctx_t *vctx) repl_on_ack(&c->persist.repl, ep, acked_lsn); TRACE("repl_ack_decode_and_dispatch: WAL ACK from standby" " lsn=%lu success=1", (unsigned long)acked_lsn); + /* ha-term-safety task 3.3 (R4.1/R4.5): route the ACK through the + * raft library so it updates match_index[]/next_index[] and + * recomputes the current-term-gated commit (raft_advance_commit). + * This fires on_commit_advanced (task 2.4) ONLY when a quorum + * agrees on a CURRENT-TERM entry — making the library the live + * commit source alongside the durable barrier; a stale-term reply + * is dropped inside the library. Driven on the same iomgr ACK + * path; neither library call performs I/O. + * + * Resolve the ABSOLUTE library peer index per endpoint type FIRST + * (remediation): this branch fires for BOTH promoted-vote eps + * (vctx->ctx != NULL, peer_idx is a relative votes index) AND + * heartbeat standby eps (vctx->ctx == NULL, peer_idx is already + * absolute) — see the authoritative scan at ha.c:6187-6189. A + * blind remap would double-map the heartbeat path onto the wrong + * (or out-of-range) match_index[] slot. */ + int ack_abs_idx = ha_repl_ack_abs_peer_idx( + &c->ha, /*ep_is_vote*/ vctx->ctx != NULL, vctx->peer_idx); + ha_raft_on_repl_ack(c, ack_abs_idx, acked_lsn, reply_term); /* ha-convergence-recovery task 4.2 (D6, Req 5.1/5.2/7.1): * ACK-driven catch-up — continue streaming to a standby still * behind target, or re-include it at parity. Shares the exact diff --git a/src/controller/ha.h b/src/controller/ha.h index ac752ad..0d238fa 100644 --- a/src/controller/ha.h +++ b/src/controller/ha.h @@ -333,6 +333,47 @@ int ha_self_restart(cluster_t *c); int ha_primary_log_append(cluster_t *c, uint32_t type, const void *payload, uint32_t len, uint64_t *out_lsn); +/* + * ha_raft_on_repl_ack (ha-term-safety task 3.3, R4.1/R4.5): drive the library on + * a replication ACK. Routes a successful WalAck through raft_on_append_entries_reply + * (updating the library's match_index[]/next_index[] at the absolute peer index) + * then raft_advance_commit, so the library recomputes the current-term-gated + * commit (the Figure-8 rule) and MAY fire on_commit_advanced — the task-2.4 + * callback that feeds the existing synchronous commit barrier. A current-term + * quorum advances committed only when the agreed entry is current-term; a + * prior-term agreed entry is held; a stale-term reply is dropped by the library. + * + * `abs_peer_idx` is the ABSOLUTE library peer index (j != self_idx), the same + * index space as match_index[]/next_index[]. The CALLER resolves it per endpoint + * type via ha_repl_ack_abs_peer_idx — promoted-vote eps carry a relative votes + * index, heartbeat standby eps carry an already-absolute index. This seam must + * NOT remap again. An out-of-range / self index is rejected loudly (returns + * -EINVAL, no library accounting) so the library's unchecked match_index[] write + * is never corrupted (no false quorum, no OOB). `acked_lsn` is the standby's + * matched index (WalAckReply.lsn); `reply_term` is the reply's term. + * + * Leader-only effect: raft_advance_commit returns -EPERM off the leader (no + * commit change, no callback) — harmless on a node that is no longer primary. + * Non-blocking (ha-async-io): the library performs no I/O; on_commit_advanced + * only records the barrier ceiling. Returns 0 on success; -EINVAL on a + * no-cluster context or a rejected peer index. */ +int ha_raft_on_repl_ack(cluster_t *c, int abs_peer_idx, uint64_t acked_lsn, + uint64_t reply_term); + +/* + * ha_repl_ack_abs_peer_idx (ha-term-safety task 3.3 remediation): resolve the + * ABSOLUTE library peer index from an ACK endpoint's stored peer_idx, branching + * on endpoint type (mirrors the standby_endpoints scan at ha.c:6187-6189): + * - ep_is_vote != 0 (promoted vote ep, vote_ep_ctx_t): peer_idx is a + * self-EXCLUDED votes index → remap abs = peer_idx < self_idx ? peer_idx + * : peer_idx+1. + * - ep_is_vote == 0 (heartbeat standby ep, standby_ep_ctx_t, ctx==NULL): + * peer_idx is ALREADY absolute → pass through. + * self_idx == ha->my_priority. Pure (no side effects); the single source of + * truth shared by the production ACK call site and the unit test so the + * remap-vs-passthrough distinction cannot drift. */ +int ha_repl_ack_abs_peer_idx(ha_state_t *ha, int ep_is_vote, int peer_idx); + /* * ha_schedule_demotion_convergence: schedule the one-shot delaymgr task that * truncates above committed_lsn (Req 1.2), WARN-logs the discarded range diff --git a/src/controller/testsuite/test_term_safety_adapter.c b/src/controller/testsuite/test_term_safety_adapter.c index 1fdd52c..a32fae6 100644 --- a/src/controller/testsuite/test_term_safety_adapter.c +++ b/src/controller/testsuite/test_term_safety_adapter.c @@ -1,4 +1,5 @@ #define _GNU_SOURCE +#include #include #include #include @@ -1276,6 +1277,317 @@ static void test_intake_logmismatch_rejected_no_false_convergence(void) printf("test_intake_logmismatch_rejected_no_false_convergence: PASS\n"); } +/* ================================================================== */ +/* Task 3.3 — drive raft_on_append_entries_reply + raft_advance_commit */ +/* on the replication ACK path */ +/* ================================================================== */ + +/* These tests drive the ACK seam ha_raft_on_repl_ack(c, abs_peer_idx, acked_lsn, + * reply_term): on a replication ACK the controller routes the reply through + * raft_on_append_entries_reply (updating match_index[]/next_index[]) then + * raft_advance_commit, which recomputes the current-term-gated commit and MAY + * fire on_commit_advanced (the task-2.4 callback that feeds the barrier). + * + * Peer-index contract (REMEDIATION round 1): ha_raft_on_repl_ack now takes the + * ABSOLUTE library peer index directly (the match_index[]/next_index[] index + * space, j != self_idx). The CALLER resolves it per endpoint type via + * ha_repl_ack_abs_peer_idx BEFORE this seam — promoted-vote eps carry a relative + * "votes" index (remapped), heartbeat standby eps carry an already-absolute + * index (passed through). The earlier blind remap inside the seam corrupted the + * heartbeat path (wrong-peer false quorum / OOB match_index[] write); the + * endpoint-type round-trip is pinned by the two remediation tests below. + * + * Fixture: make_leader(c, term, n_peers) brings the node up as the raft LEADER + * with self_idx == my_priority == 0 (make_cluster calloc's my_priority=0), so + * the only standby's absolute peer index is 1. + * + * Against the pre-3.3 ACK path (repl_on_ack durability only, no library reply), + * ha_raft_on_repl_ack does not exist (compile RED); once wired, a current-term + * quorum ACK MUST advance the library commit_index and fire on_commit_advanced + * (R4.1/R4.5), a prior-term agreed entry MUST hold, and a stale-term reply MUST + * be dropped. */ + +/* Seed `count` entries at the given term into the leader's local WAL via the + * wired on_persist_entry, growing the library log_len in lockstep. */ +static void leader_seed_entries(cluster_t* c, uint64_t term, int count) +{ + uint8_t blob[64]; + for (int i = 1; i <= count; i++) { + size_t n = pack_entry(blob, WAL_JOB_UPDATE, "x", 1); + TEST_ASSERT_EQUAL_INT(0, + ha_cb_persist_entry_for_test(&c->ha, (uint64_t)i, term, blob, n)); + c->ha.raft.log_len = (uint64_t)i; + } + TEST_ASSERT_EQUAL_INT(0, persist_flush(&c->persist)); +} + +/* (a) A current-term quorum ACK drives raft_on_append_entries_reply → + * raft_advance_commit → on_commit_advanced fires with the agreed commit_index, + * which raises the barrier ceiling. 3-peer cluster (self + 2 standbys, quorum=2): + * self holds LSN 1,2 at the current term; one standby ACKs LSN 2 → {self, peer1} + * is a quorum for LSN 2, the agreed entry is current-term, so commit advances to + * 2 and the library ceiling (commit_set_raft_commit_index effect) caps committed + * at the library point under a durable majority. */ +static void test_repl_ack_currentterm_quorum_advances_commit(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_ts_ack_adv_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + const uint64_t T = 6; + + cluster_t* c = make_cluster(tmp); + TEST_ASSERT_NOT_NULL(c); + make_leader(c, T, /*n_peers*/3); + + /* Init the cluster's commit engine over the same 3-peer cluster so a + * majority is self + 1 standby, and register one standby whose durable ACK + * supplies the durable-majority floor. */ + TEST_ASSERT_EQUAL_INT(0, + commit_engine_init(&c->commit, &c->persist, /*n_peers*/3, /*n_vcpus*/8)); + + /* Seed LSN 1,2 at the current term T → durable floor reaches LSN 2. */ + leader_seed_entries(c, T, /*count*/2); + + TEST_ASSERT_EQUAL_INT(0, + repl_add_standby(&c->persist.repl, &g_ae_dummy_ep, "sb0", 7000)); + /* Durable majority: the standby's durable position covers LSN 2 so the + * barrier ceiling is not held below the library commit point. */ + atomic_store(&c->persist.repl.standbys[0].acked_lsn, 2); + commit_on_ack(&c->commit, /*peer_idx*/0, /*acked_lsn*/2); + + /* Nothing committed yet: the library has not advanced commit_index (no ACK + * routed through the library) so the commit source is still inert at 0. */ + TEST_ASSERT_EQUAL_UINT64(0, c->ha.raft.commit_index); + + /* The standby ACKs LSN 2 at the leader's current term. The caller has + * already resolved the absolute peer index (self_idx 0 → the only standby is + * absolute peer 1); the seam takes that absolute index directly. */ + int rc = ha_raft_on_repl_ack(c, /*abs_peer_idx*/1, /*acked_lsn*/2, /*reply_term*/T); + TEST_ASSERT_EQUAL_INT(0, rc); + + /* (1) The library recorded the peer's match_index at the absolute index. */ + TEST_ASSERT_EQUAL_UINT64(2, c->ha.raft.match_index[1]); + TEST_ASSERT_EQUAL_UINT64(3, c->ha.raft.next_index[1]); + + /* (2) raft_advance_commit advanced the current-term-gated commit_index to 2 + * ({self, peer1} quorum, entry at LSN 2 is term T). */ + TEST_ASSERT_EQUAL_UINT64(2, c->ha.raft.commit_index); + + /* (3) on_commit_advanced fired → the barrier ceiling rose and committed_lsn + * advanced to the library point under the durable majority (R4.1). */ + TEST_ASSERT_EQUAL_UINT64(2, atomic_load(&c->persist.committed_lsn)); + + commit_engine_fini(&c->commit); + teardown_cluster(c); + printf("test_repl_ack_currentterm_quorum_advances_commit: PASS\n"); +} + +/* (b) A quorum that agrees only on a PRIOR-term entry does NOT advance commit + * (the Figure-8 current-term rule, R4.1/R4.5). self holds LSN 1@term3 (a + * prior-term entry) and is now leader at term 6 with no current-term entry yet. + * A standby ACKs LSN 1 → {self, peer1} is a quorum for LSN 1, but the entry at + * LSN 1 is term 3 != current term 6, so raft_advance_commit holds commit_index + * at 0 and on_commit_advanced never fires. */ +static void test_repl_ack_priorterm_quorum_holds_commit(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_ts_ack_prior_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + const uint64_t PRIOR = 3; + const uint64_t CUR = 6; + + cluster_t* c = make_cluster(tmp); + TEST_ASSERT_NOT_NULL(c); + make_leader(c, CUR, /*n_peers*/3); + + /* Seed a single PRIOR-term entry at LSN 1 (replicated from an old leader). */ + leader_seed_entries(c, PRIOR, /*count*/1); + + /* A standby ACKs LSN 1 at the leader's current term (the wire reply term is + * the leader's term; the ENTRY's term is what gates the commit). */ + int rc = ha_raft_on_repl_ack(c, /*abs_peer_idx*/1, /*acked_lsn*/1, /*reply_term*/CUR); + TEST_ASSERT_EQUAL_INT(0, rc); + + /* match_index updated (the reply itself is current-term, not stale). */ + TEST_ASSERT_EQUAL_UINT64(1, c->ha.raft.match_index[1]); + + /* But commit is HELD: the agreed entry (LSN1@term3) is not current-term → + * Figure-8 rule blocks the advance, commit_index stays 0. */ + TEST_ASSERT_EQUAL_UINT64(0, c->ha.raft.commit_index); + + teardown_cluster(c); + printf("test_repl_ack_priorterm_quorum_holds_commit: PASS\n"); +} + +/* (c) A stale-term reply is dropped: raft_on_append_entries_reply discards a + * reply whose term != s->term, so match_index[] is NOT updated and no commit + * advance occurs (R4.5 / raft.tla 415). self holds LSN 1,2 at the current term; + * a stale reply (reply_term < current term) for LSN 2 must leave match_index and + * commit_index untouched. */ +static void test_repl_ack_staleterm_reply_dropped(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_ts_ack_stale_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + const uint64_t T = 8; + const uint64_t STALE = 5; + + cluster_t* c = make_cluster(tmp); + TEST_ASSERT_NOT_NULL(c); + make_leader(c, T, /*n_peers*/3); + + leader_seed_entries(c, T, /*count*/2); + + /* Stale-term ACK for LSN 2 → dropped by the library before any state change. */ + int rc = ha_raft_on_repl_ack(c, /*abs_peer_idx*/1, /*acked_lsn*/2, /*reply_term*/STALE); + TEST_ASSERT_EQUAL_INT(0, rc); + + /* No match_index update (the peer's slot stays at the post-init default 0). */ + TEST_ASSERT_EQUAL_UINT64(0, c->ha.raft.match_index[1]); + /* No commit advance. */ + TEST_ASSERT_EQUAL_UINT64(0, c->ha.raft.commit_index); + + teardown_cluster(c); + printf("test_repl_ack_staleterm_reply_dropped: PASS\n"); +} + +/* ================================================================== */ +/* Task 3.3 REMEDIATION round 1 — endpoint-type-correct peer mapping */ +/* ================================================================== */ + +/* (d) The bug: the ACK walAck branch fires for BOTH promoted-vote eps (relative + * votes index) AND heartbeat standby eps (already-absolute index). The earlier + * seam unconditionally remapped, so a heartbeat ep's absolute index was mapped a + * SECOND time — crediting the WRONG peer (false quorum) or writing OUT OF BOUNDS. + * ha_repl_ack_abs_peer_idx must remap ONLY the vote ep and pass the standby ep + * through unchanged. We pin both endpoint types AND both my_priority values so + * the remap-vs-passthrough distinction actually round-trips (the pre-fix tests + * only used my_priority==0, which masked the bug). + * + * Round-trip oracle: for a vote ep, peer absolute index i is sent as votes_idx + * (i < self ? i : i-1); ha_repl_ack_abs_peer_idx must recover exactly i. For a + * standby ep, the stored peer_idx IS i and must pass through unchanged. */ +static void test_repl_ack_abs_peer_idx_endpoint_type_roundtrip(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_ts_ack_map_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + cluster_t* c = make_cluster(tmp); + TEST_ASSERT_NOT_NULL(c); + + /* --- self_idx (my_priority) == 0: every peer is after self --- */ + c->ha.my_priority = 0; + /* Heartbeat standby ep (ctx==NULL → ep_is_vote=0): absolute index passes + * through UNCHANGED. Absolute peer 1 must stay 1, absolute peer 2 stay 2. + * (The pre-fix blind remap would have turned 1→2 and 2→3 — the bug.) */ + TEST_ASSERT_EQUAL_INT(1, ha_repl_ack_abs_peer_idx(&c->ha, /*vote*/0, /*peer*/1)); + TEST_ASSERT_EQUAL_INT(2, ha_repl_ack_abs_peer_idx(&c->ha, /*vote*/0, /*peer*/2)); + /* Promoted vote ep (ep_is_vote=1): votes_idx 0 → abs 1, votes_idx 1 → abs 2. */ + TEST_ASSERT_EQUAL_INT(1, ha_repl_ack_abs_peer_idx(&c->ha, /*vote*/1, /*votes*/0)); + TEST_ASSERT_EQUAL_INT(2, ha_repl_ack_abs_peer_idx(&c->ha, /*vote*/1, /*votes*/1)); + + /* --- self_idx (my_priority) == 1: a peer below self is NOT shifted --- */ + c->ha.my_priority = 1; + /* Standby ep: absolute indices 0 and 2 (peer 1 is self) pass through. */ + TEST_ASSERT_EQUAL_INT(0, ha_repl_ack_abs_peer_idx(&c->ha, /*vote*/0, /*peer*/0)); + TEST_ASSERT_EQUAL_INT(2, ha_repl_ack_abs_peer_idx(&c->ha, /*vote*/0, /*peer*/2)); + /* Vote ep with self_idx==1: votes_idx 0 (peer absolute 0, < self) → 0; + * votes_idx 1 (peer absolute 2, >= self) → 2. This is where remap and + * passthrough DIVERGE — the case the my_priority==0 tests could not reach. */ + TEST_ASSERT_EQUAL_INT(0, ha_repl_ack_abs_peer_idx(&c->ha, /*vote*/1, /*votes*/0)); + TEST_ASSERT_EQUAL_INT(2, ha_repl_ack_abs_peer_idx(&c->ha, /*vote*/1, /*votes*/1)); + + teardown_cluster(c); + printf("test_repl_ack_abs_peer_idx_endpoint_type_roundtrip: PASS\n"); +} + +/* (e) End-to-end heartbeat-standby ACK: a standby ep carries an ALREADY-ABSOLUTE + * peer index. Resolving it via ha_repl_ack_abs_peer_idx (ep_is_vote=0) and + * driving ha_raft_on_repl_ack must land match_index on the CORRECT absolute slot + * (not the +1 wrong slot the blind remap produced). 3-peer cluster, self=0; the + * standby's absolute index is 1. A current-term quorum ACK for LSN 2 advances + * the library commit_index to 2 — but ONLY because match_index[1] (not [2]) is + * credited. Under the broken blind remap the credit landed on match_index[2] + * (a phantom peer): no quorum at LSN 2, commit_index would stay 0. */ +static void test_repl_ack_heartbeat_standby_absolute_slot(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_ts_ack_hb_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + const uint64_t T = 6; + + cluster_t* c = make_cluster(tmp); + TEST_ASSERT_NOT_NULL(c); + make_leader(c, T, /*n_peers*/3); + TEST_ASSERT_EQUAL_INT(0, c->ha.my_priority); /* self_idx 0 */ + + TEST_ASSERT_EQUAL_INT(0, + commit_engine_init(&c->commit, &c->persist, /*n_peers*/3, /*n_vcpus*/8)); + + leader_seed_entries(c, T, /*count*/2); + + TEST_ASSERT_EQUAL_INT(0, + repl_add_standby(&c->persist.repl, &g_ae_dummy_ep, "sb0", 7000)); + atomic_store(&c->persist.repl.standbys[0].acked_lsn, 2); + commit_on_ack(&c->commit, /*peer_idx*/0, /*acked_lsn*/2); + + /* The standby ep stores the ABSOLUTE index 1 (standby_ep_ctx_t.peer_idx). + * Resolve it as a heartbeat ep (ep_is_vote=0) — it must pass through as 1. */ + int abs_idx = ha_repl_ack_abs_peer_idx(&c->ha, /*ep_is_vote*/0, /*peer_idx*/1); + TEST_ASSERT_EQUAL_INT(1, abs_idx); /* NOT 2 — the blind-remap bug slot */ + + int rc = ha_raft_on_repl_ack(c, abs_idx, /*acked_lsn*/2, /*reply_term*/T); + TEST_ASSERT_EQUAL_INT(0, rc); + + /* The CORRECT absolute slot was credited; the phantom slot 2 stayed empty. */ + TEST_ASSERT_EQUAL_UINT64(2, c->ha.raft.match_index[1]); + TEST_ASSERT_EQUAL_UINT64(0, c->ha.raft.match_index[2]); + + /* {self, peer1} is a real quorum for LSN 2 → current-term commit advances. */ + TEST_ASSERT_EQUAL_UINT64(2, c->ha.raft.commit_index); + TEST_ASSERT_EQUAL_UINT64(2, atomic_load(&c->persist.committed_lsn)); + + commit_engine_fini(&c->commit); + teardown_cluster(c); + printf("test_repl_ack_heartbeat_standby_absolute_slot: PASS\n"); +} + +/* (f) Defense-in-depth: an out-of-range / self absolute index is rejected loudly + * (returns -EINVAL) and performs NO library accounting — fencing the unchecked + * match_index[] write at the controller boundary rather than corrupting memory. */ +static void test_repl_ack_out_of_range_peer_rejected(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_ts_ack_oob_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + const uint64_t T = 6; + + cluster_t* c = make_cluster(tmp); + TEST_ASSERT_NOT_NULL(c); + make_leader(c, T, /*n_peers*/3); /* valid peers are absolute 1,2 */ + leader_seed_entries(c, T, /*count*/2); + + /* abs index == n_peers (3) is out of range → rejected, no state change. */ + TEST_ASSERT_EQUAL_INT(-EINVAL, + ha_raft_on_repl_ack(c, /*abs_peer_idx*/3, /*acked_lsn*/2, /*reply_term*/T)); + /* self (0) is never a valid peer → rejected. */ + TEST_ASSERT_EQUAL_INT(-EINVAL, + ha_raft_on_repl_ack(c, /*abs_peer_idx*/0, /*acked_lsn*/2, /*reply_term*/T)); + + /* Nothing was credited and commit did not advance. */ + TEST_ASSERT_EQUAL_UINT64(0, c->ha.raft.match_index[1]); + TEST_ASSERT_EQUAL_UINT64(0, c->ha.raft.commit_index); + + teardown_cluster(c); + printf("test_repl_ack_out_of_range_peer_rejected: PASS\n"); +} + /* ------------------------------------------------------------------ */ /* Suite entry point */ /* ------------------------------------------------------------------ */ @@ -1308,4 +1620,10 @@ void test_term_safety_adapter(void) RUN_TEST(test_intake_higher_term_heartbeat_adopts_term); RUN_TEST(test_intake_divergent_tail_truncated_and_reconverged); RUN_TEST(test_intake_logmismatch_rejected_no_false_convergence); + RUN_TEST(test_repl_ack_currentterm_quorum_advances_commit); + RUN_TEST(test_repl_ack_priorterm_quorum_holds_commit); + RUN_TEST(test_repl_ack_staleterm_reply_dropped); + RUN_TEST(test_repl_ack_abs_peer_idx_endpoint_type_roundtrip); + RUN_TEST(test_repl_ack_heartbeat_standby_absolute_slot); + RUN_TEST(test_repl_ack_out_of_range_peer_rejected); }