From 83fbed237004df4dcc75ee50e0d85906e105b561 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Tue, 23 Jun 2026 00:21:06 +0000 Subject: [PATCH] fix(ha-term-safety): remove task-2.4 commit min-ceiling (Option A) + fix bug-991 catch-up off-by-one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two coupled fixes for a live commit-stall (committed_lsn pinned at the own-term NOOP while a 3-node cluster was durable to LSN 5 — jobs acked but never committed): bug-991: the WAL catch-up applied_lsn was advanced to lsn+1 (next-to-apply) in the three live/catch-up apply paths, but recovery/checkpoint/init and the leader's catch-up streamer treat it as last-held. The over-advertise made the streamer skip an LSN -> library logOk wedge -> no WalAck. Fixed to last-held (= lsn). Option A (design reconsideration of REVIEW ISSUE 3): removed the committed = min(durable-majority, raft_commit_index) ceiling. Under single-writer the durable-majority watermark is already (term,lsn)-current-term-safe (term-carrying replication + (term,lsn) reconcile/truncate-before-durable + own-term NOOP gate + post-election truncate-to-safe), so the library commit_index was a redundant, starvation-prone gate that pinned committed below the safe durable majority whenever the library lagged. The durable-majority barrier + own-term writable gate are now authoritative; raft_commit_index is inert-advisory. Safety: reviewer-verified the durable-majority watermark is (term,lsn)-safe and Figure-8 holds via the own-term gate (new RED->GREEN test); bug-851 cannot recur. Live: commit trace 4->5 + walcompare committed-prefix identical across 3 nodes. 177 unit tests pass. --- src/controller/commit.c | 70 ++-- src/controller/ha.c | 42 +- src/controller/include/commit.h | 28 +- src/controller/testsuite/test_config_repl.c | 53 ++- .../testsuite/test_term_safety_adapter.c | 375 +++++++++++++----- 5 files changed, 396 insertions(+), 172 deletions(-) diff --git a/src/controller/commit.c b/src/controller/commit.c index 95c3f65..ae646c6 100644 --- a/src/controller/commit.c +++ b/src/controller/commit.c @@ -94,30 +94,27 @@ static uint64_t majority_candidate_locked(commit_engine_t* ce) int n = 0; int need, k, i; - /* ha-term-safety task 2.4: the raft library's current-term-gated commit - * index is the COMMIT SOURCE. Once fed, it is an ADDITIONAL ceiling on top - * of the durable cap: committed_lsn = min(durable-majority, raft_commit_index), - * so the barrier never advances committed over an entry the library has not - * admitted under its Figure-8 current-term rule (replacing the term-blind - * safe-commit). Folding it into `cap` makes BOTH the standalone (n_peers<=1) - * and the multi-peer paths honor the source uniformly. + /* ha-term-safety commit-ceiling-optA: the durable-majority barrier (this + * function) PLUS the own-term writable gate (commit_set_writable_lsn / + * wait_done_locked) are authoritative for commit-advance. The raft library's + * commit_index is NOT folded in as a ceiling here. * - * FEATURE GATE: raft_commit_index == 0 means the library commit source is - * NOT yet active (no on_commit_advanced has fired — e.g. before tasks 3.x - * drive raft_advance_commit, or in a node that has committed nothing). In - * that state the ceiling is INERT and the barrier keeps its autonomous - * durable-majority behavior; the source only takes over once the library - * reports its first strict commit (commit_index >= 1, which is the lowest - * real LSN). This is the stub→real transition for this callback. */ - if (ce->raft_commit_index != 0 && ce->raft_commit_index < cap) { - cap = ce->raft_commit_index; - } + * Why the former min(durable-majority, raft_commit_index) ceiling was + * removed: under single-writer (only the current primary appends) the + * durable-majority candidate is ALREADY current-term-safe — every entry it + * can cover is one this primary appended in its own term, and the own-term + * NOOP gate (writable_lsn) independently blocks acknowledging a prior-term + * entry until the current-term NOOP is majority-durable. The library + * commit_index is computed from match_index[] that the catch-up/streaming + * path advances asynchronously, so it routinely LAGS the durable majority; + * folding it into `cap` pinned committed_lsn below the safe durable + * watermark (e.g. committed stuck at 1 while a 3-node cluster is durable to + * 5), a redundant, starvation-prone gate. commit_set_raft_commit_index is + * now inert-advisory (recorded for observability/conformance only). */ /* Standalone / single-member: self constitutes the majority (Req 6.3) — - * but self still only counts up to what is durable locally AND what the - * library has committed (cap already folds in raft_commit_index), so a - * single-member commit also cannot advance over a not-yet-synced or - * not-yet-library-committed entry. */ + * self still only counts up to what is durable locally, so a single-member + * commit cannot advance over a not-yet-synced entry. */ if (ce->n_peers <= 1) { return cap; } @@ -270,7 +267,7 @@ int commit_engine_init(commit_engine_t* ce, persist_t* p, int n_peers, int n_vcp ce->n_vcpus = n_vcpus; ce->inflight_waits = 0; ce->writable_lsn = 0; /* no own-term gate until set by promotion */ - ce->raft_commit_index = 0; /* no library commit source until fed (task 2.4) */ + ce->raft_commit_index = 0; /* advisory record only; does not gate commit (optA) */ memset(ce->scratch, 0, sizeof(ce->scratch)); /* Register the durable-advance wake so the flusher can re-trigger commit @@ -416,12 +413,20 @@ void commit_set_writable_lsn(commit_engine_t* ce, uint64_t noop_lsn) pthread_mutex_unlock(&ce->lock); } -/* ha-term-safety task 2.4: feed the raft library's current-term-gated commit - * index as the commit SOURCE (R4.1, R4.5). Monotonic max() — the library only - * fires on_commit_advanced on a strict advance, but guard against a stale/lower - * value so the ceiling never regresses. Re-evaluate the barrier under the new - * ceiling and wake waiters: the durable cap may already cover the newly-admitted - * LSNs, so the commit can advance immediately without waiting for the next ACK. +/* ha-term-safety commit-ceiling-optA: record the raft library's commit_index + * for OBSERVABILITY / TRACE / conformance-oracle purposes ONLY. This value does + * NOT gate commit-advance — majority_candidate_locked no longer folds it into + * the commit candidate (Option A: the durable-majority barrier + own-term + * writable gate are authoritative). It is still maintained (and still fed by + * on_commit_advanced) so the library half keeps a sink and tooling can compare + * the library's current-term commit point against the barrier's committed_lsn. + * Monotonic max() — never regresses. + * + * The barrier is still re-evaluated + waiters woken here: the on_commit_advanced + * callback fires on the ACK path, and a fresh ACK may have just raised the + * durable majority, so this is a convenient wake point (a no-op if nothing + * changed). commit_advance_locked re-derives the durable majority WITHOUT the + * recorded commit_index. * * LOCK ORDERING: takes only ce->lock (commit_advance_locked re-reads the durable * watermark + registry watermarks via the existing path). The on_commit_advanced @@ -434,12 +439,11 @@ void commit_set_raft_commit_index(commit_engine_t* ce, uint64_t commit_index) } pthread_mutex_lock(&ce->lock); if (commit_index > ce->raft_commit_index) { - ce->raft_commit_index = commit_index; /* monotonic ceiling */ + ce->raft_commit_index = commit_index; /* monotonic advisory record */ } - /* The ceiling moved (or was confirmed): re-derive the majority under it and - * advance committed_lsn if the durable majority already covers the newly - * admitted prefix. commit_advance_locked broadcasts on any advance; broadcast - * unconditionally so a waiter re-checks even on a no-advance ceiling bump. */ + /* Advisory record updated. Re-derive the durable majority (which does NOT + * consult raft_commit_index) and wake waiters in case a concurrent ACK on + * this path raised the durable watermark. */ commit_advance_locked(ce); pthread_cond_broadcast(&ce->cond); pthread_mutex_unlock(&ce->lock); diff --git a/src/controller/ha.c b/src/controller/ha.c index 76000df..7c59fb7 100644 --- a/src/controller/ha.c +++ b/src/controller/ha.c @@ -1966,18 +1966,26 @@ void ha_fini(ha_state_t *ha) * ha_apply_wal_catchup_entry: apply WAL catch-up entries received from the * primary to local persistent storage and advance applied_lsn on success. * + * applied_lsn semantic (bug-991): the LAST CONTIGUOUSLY-HELD LSN — the same + * meaning recovery (= durable_floor), checkpoint (= checkpoint_lsn), and init + * (= 0) assign it, and the watermark the standby advertises to the leader + * (sr->applied_lsn) so the leader's catch-up streamer streams the half-open + * range (applied_lsn, committed]. NEVER lsn+1 (next-to-apply): a +1 makes the + * standby advertise one LSN too high, so the leader skips the real next-needed + * entry and the standby's library logOk wedges forever (no WalAck → commit stall). + * * Two modes (Req 5.3, 5.5): * * Batch mode (entry->n_entries > 0): * Iterate entry->entries[0..n_entries-1] in order. Apply each sub-entry * individually via apply_wal_entry(). NULL sub-entries are treated as * corrupt and skipped (Req 3.3). After ALL sub-entries are processed, - * advance applied_lsn to last-successfully-applied lsn+1 so the next + * advance applied_lsn to the last-successfully-applied lsn so the next * heartbeat reply reflects post-batch state (Req 5.3). * * Single-entry mode (entry->n_entries == 0): * Backward-compatible path from task 2.1. Apply entry->lsn/entry_type - * directly and advance applied_lsn to lsn+1 on success. + * directly and advance applied_lsn to lsn on success. * * In both modes: * - applied_lsn is advanced atomically and monotonically. @@ -2110,9 +2118,17 @@ static int ha_apply_term_entry_via_library(ha_state_t *ha, persist_t *p, uint64_t after_term = persist_entry_term(p, e->lsn); if (after_term == e->term) { /* Accepted (appended / conflict-truncate-then-persist) or already-have - * (idempotent re-stream): ACK + advance applied on that signal. */ + * (idempotent re-stream): ACK + advance applied on that signal. + * + * bug-991: applied_lsn is the "last contiguously-held LSN" (the same + * semantic recovery/checkpoint/init use, and the watermark the leader's + * catch-up streamer reads via sr->applied_lsn to stream (standby_lsn, + * committed]). Advance to e->lsn (the LSN we just held), NOT e->lsn + 1: + * a +1 over-advertises one LSN, so the leader's next stream begins at + * e->lsn + 2 and SKIPS the real next-needed entry e->lsn + 1, wedging + * logOk and stalling commit. */ (void)before_term; - ha_advance_applied_lsn(ha, e->lsn + 1); + ha_advance_applied_lsn(ha, e->lsn); ha_send_wal_ack_reply(ha, ep, c, e->lsn); return 1; } @@ -2199,10 +2215,13 @@ int ha_apply_wal_catchup_entry(ha_state_t *ha, persist_t *p, had_good = 1; } - /* Advance applied_lsn to last_good_lsn+1 after the full batch. - * Only advance if at least one entry was applied successfully. */ + /* Advance applied_lsn to last_good_lsn after the full batch (bug-991: + * last contiguously-held LSN, NOT last_good_lsn + 1 — see the + * ha_apply_term_entry_via_library comment for why the +1 wedged the + * leader's catch-up streamer). Only advance if at least one entry was + * applied successfully. */ if (had_good) - ha_advance_applied_lsn(ha, last_good_lsn + 1); + ha_advance_applied_lsn(ha, last_good_lsn); return 0; } @@ -2220,9 +2239,12 @@ int ha_apply_wal_catchup_entry(ha_state_t *ha, persist_t *p, return rc; } - /* Advance applied_lsn atomically (lsn+1 mirrors persist->current_lsn - * after a successful apply). */ - ha_advance_applied_lsn(ha, entry->lsn + 1); + /* Advance applied_lsn atomically to the last contiguously-held LSN + * (bug-991: entry->lsn, NOT entry->lsn + 1 — see the + * ha_apply_term_entry_via_library comment for the streamer-skip wedge a +1 + * caused). This is the same last-held semantic recovery/checkpoint/init use, + * and matches the (standby_lsn, committed] range the leader streams. */ + ha_advance_applied_lsn(ha, entry->lsn); /* Task 3.1: ACK the successfully applied single entry */ ha_send_wal_ack_reply(ha, ep, c, entry->lsn); diff --git a/src/controller/include/commit.h b/src/controller/include/commit.h index 7b4f881..91550d8 100644 --- a/src/controller/include/commit.h +++ b/src/controller/include/commit.h @@ -31,14 +31,14 @@ typedef struct commit_engine { int n_vcpus; int inflight_waits; /* <= max(1, n_vcpus - COMMIT_WAIT_RESERVED) */ uint64_t writable_lsn; /* own-term gate; 0 = no gate */ - /* ha-term-safety task 2.4: the raft library's current-term-gated commit - * index — the COMMIT SOURCE (replaces the term-blind safe-commit). The - * barrier never advances committed_lsn past this ceiling, so an entry is - * committed only when BOTH a durable majority agrees AND the library's - * Figure-8 current-term commit rule has admitted it. 0 = no library commit - * index yet (nothing committed) — committed_lsn stays at 0. Monotonic - * non-decreasing (the library only fires on_commit_advanced on a strict - * advance), set under ce->lock via commit_set_raft_commit_index. */ + /* ha-term-safety commit-ceiling-optA: the raft library's commit_index, + * recorded for OBSERVABILITY / TRACE / conformance-oracle ONLY. It does NOT + * gate commit-advance: majority_candidate_locked no longer folds it into the + * commit candidate. Commit-advance is authoritative via the durable-majority + * barrier + the own-term writable_lsn gate. Kept (and still fed by + * on_commit_advanced) so the library half keeps a sink and tooling can + * compare the library current-term commit point vs committed_lsn. Monotonic + * non-decreasing, set under ce->lock via commit_set_raft_commit_index. */ uint64_t raft_commit_index; uint64_t scratch[MAX_STANDBYS]; /* sorted matchIndex scratch array */ } commit_engine_t; @@ -65,11 +65,13 @@ void commit_on_ack(commit_engine_t* ce, int standby_idx, uint64_t acked_lsn); * (API ships here; no caller in this spec — unset = no gate.) */ void commit_set_writable_lsn(commit_engine_t* ce, uint64_t noop_lsn); -/* ha-term-safety task 2.4: feed the raft library's current-term-gated commit - * index as the commit SOURCE. Records the ceiling (monotonic max() — never - * regresses) and re-evaluates the barrier under the new ceiling, advancing - * committed_lsn and waking satisfied waiters. Called from the on_commit_advanced - * callback after the library admits a strict commit advance. Never blocks. */ +/* ha-term-safety commit-ceiling-optA: record the raft library's commit_index + * for OBSERVABILITY / TRACE / conformance-oracle ONLY (monotonic max() — never + * regresses). This does NOT gate commit-advance: the durable-majority barrier + + * own-term writable gate are authoritative. Still fed by on_commit_advanced so + * the library half keeps a sink. Re-evaluates the (commit_index-independent) + * durable majority and wakes waiters as a convenient ACK-path wake point. Never + * blocks. */ void commit_set_raft_commit_index(commit_engine_t* ce, uint64_t commit_index); /* Quorum-membership change (standby add/remove/catch-up transition): diff --git a/src/controller/testsuite/test_config_repl.c b/src/controller/testsuite/test_config_repl.c index 5a69522..f2a7797 100644 --- a/src/controller/testsuite/test_config_repl.c +++ b/src/controller/testsuite/test_config_repl.c @@ -2020,8 +2020,11 @@ static void test_wal_catchup_entry_advances_applied_lsn(void) /* GREEN will implement ha_apply_wal_catchup_entry() */ int rc = ha_apply_wal_catchup_entry(&ha, p, &entry, NULL, NULL); TEST_ASSERT_EQUAL_INT(0, rc); - /* applied_lsn must advance to lsn+1 = 43 (mirrors persist->current_lsn) */ - TEST_ASSERT_EQUAL_UINT64(43, + /* bug-991: applied_lsn is the LAST CONTIGUOUSLY-HELD LSN, so a single applied + * entry at LSN 42 advances applied_lsn to 42 (NOT 43). The +1 over-advertised + * the standby's watermark and made the leader's catch-up streamer skip the + * real next-needed LSN. */ + TEST_ASSERT_EQUAL_UINT64(42, (uint64_t)atomic_load_explicit(&ha.applied_lsn, memory_order_acquire)); ha_fini(&ha); @@ -2128,8 +2131,9 @@ static void test_wal_catchup_batch_advances_applied_lsn(void) int rc = ha_apply_wal_catchup_entry(&ha, p, &outer, NULL, NULL); TEST_ASSERT_EQUAL_INT(0, rc); - /* applied_lsn must advance to last_entry->lsn + 1 = 13 */ - TEST_ASSERT_EQUAL_UINT64(13, + /* bug-991: applied_lsn = last contiguously-held LSN = last_entry->lsn = 12 + * (NOT 13). */ + TEST_ASSERT_EQUAL_UINT64(12, (uint64_t)atomic_load_explicit(&ha.applied_lsn, memory_order_acquire)); ha_fini(&ha); @@ -2175,8 +2179,9 @@ static void test_wal_catchup_partial_batch(void) int rc = ha_apply_wal_catchup_entry(&ha, p, &outer, NULL, NULL); TEST_ASSERT_EQUAL_INT(0, rc); - /* applied_lsn must advance to 21+1=22 */ - TEST_ASSERT_EQUAL_UINT64(22, + /* bug-991: applied_lsn = last contiguously-held LSN = 21 (NOT 22). Monotonic: + * advances from the pre-set 5 to 21. */ + TEST_ASSERT_EQUAL_UINT64(21, (uint64_t)atomic_load_explicit(&ha.applied_lsn, memory_order_acquire)); ha_fini(&ha); @@ -2225,10 +2230,12 @@ static void test_wal_catchup_batch_error_entry_skipped(void) /* Overall rc == 0: outer batch call succeeds even with inner skips */ TEST_ASSERT_EQUAL_INT(0, rc); - /* applied_lsn must reach at least 31 (e0 applied); ideally 33 (e2 applied) */ + /* bug-991: applied_lsn = last contiguously-held LSN. e0 (LSN 30) applies, + * the NULL sub-entry is skipped, e2 (LSN 32) applies → last-held is 32 (or at + * least 30 if e2 were dropped). Must reach at least 30 (e0 held). */ uint64_t final_lsn = (uint64_t)atomic_load_explicit(&ha.applied_lsn, memory_order_acquire); - TEST_ASSERT_TRUE(final_lsn >= 31); + TEST_ASSERT_TRUE(final_lsn >= 30); ha_fini(&ha); destroy_test_persist_catchup(p); @@ -2263,8 +2270,9 @@ static void test_wal_catchup_batch_zero_entries_fallback(void) int rc = ha_apply_wal_catchup_entry(&ha, p, &entry, NULL, NULL); TEST_ASSERT_EQUAL_INT(0, rc); - /* Single-entry path: applied_lsn = 50+1 = 51 */ - TEST_ASSERT_EQUAL_UINT64(51, + /* bug-991: single-entry path advances applied_lsn to the last-held LSN = 50 + * (NOT 51). */ + TEST_ASSERT_EQUAL_UINT64(50, (uint64_t)atomic_load_explicit(&ha.applied_lsn, memory_order_acquire)); ha_fini(&ha); @@ -2321,8 +2329,9 @@ static void test_wal_catchup_batch_256_entries(void) int rc = ha_apply_wal_catchup_entry(&ha, p, &outer, NULL, NULL); TEST_ASSERT_EQUAL_INT(0, rc); - /* applied_lsn must advance to last_entry->lsn + 1 = 1000+255+1 = 1256 */ - TEST_ASSERT_EQUAL_UINT64(1256, + /* bug-991: applied_lsn = last contiguously-held LSN = last_entry->lsn = + * 1000+255 = 1255 (NOT 1256). */ + TEST_ASSERT_EQUAL_UINT64(1255, (uint64_t)atomic_load_explicit(&ha.applied_lsn, memory_order_acquire)); ha_fini(&ha); @@ -2340,9 +2349,10 @@ static void test_wal_catchup_batch_256_entries(void) * * Strategy: start with a pre-set applied_lsn, apply a batch of 4 entries, * verify that applied_lsn does NOT advance to an intermediate value and IS - * exactly last_entry+1 at the end. We confirm the "only-after-full-batch" - * invariant by checking the exact final value equals last_lsn+1 (not some - * intermediate lsn+1 from a mid-batch advance). + * exactly the last contiguously-held LSN at the end (bug-991: last_lsn, not + * last_lsn+1). We confirm the "only-after-full-batch" invariant by checking the + * exact final value equals last_lsn (not some intermediate held LSN from a + * mid-batch advance). * Requirements 5.3, 5.5. */ static void test_wal_catchup_batch_applied_lsn_only_after_full_batch(void) { @@ -2382,17 +2392,16 @@ static void test_wal_catchup_batch_applied_lsn_only_after_full_batch(void) int rc = ha_apply_wal_catchup_entry(&ha, p, &outer, NULL, NULL); TEST_ASSERT_EQUAL_INT(0, rc); - /* applied_lsn must be exactly 204 (203+1) — the post-batch value. - * If the implementation advanced after each sub-entry, it would still - * reach 204, but the test for correctness is that the final value - * equals the LAST entry lsn+1, not any intermediate lsn+1. - * Specifically, it must NOT remain at 201 or 202 (partial advance). */ + /* bug-991: applied_lsn must be exactly 203 (the last contiguously-held LSN) — + * the post-batch value. The test for correctness is that the final value + * equals the LAST entry's lsn, not any intermediate held LSN. + * Specifically, it must NOT remain at 200 or 201 (partial advance). */ uint64_t final_lsn = (uint64_t)atomic_load_explicit(&ha.applied_lsn, memory_order_acquire); /* Must have advanced past starting value (100) */ TEST_ASSERT_GREATER_THAN_UINT64(100, final_lsn); - /* Must equal the last-entry+1 (204), not any intermediate value */ - TEST_ASSERT_EQUAL_UINT64(204, final_lsn); + /* Must equal the last-held LSN (203), not any intermediate value */ + TEST_ASSERT_EQUAL_UINT64(203, final_lsn); ha_fini(&ha); destroy_test_persist_catchup(p); diff --git a/src/controller/testsuite/test_term_safety_adapter.c b/src/controller/testsuite/test_term_safety_adapter.c index 9039df9..1d26d4e 100644 --- a/src/controller/testsuite/test_term_safety_adapter.c +++ b/src/controller/testsuite/test_term_safety_adapter.c @@ -688,115 +688,177 @@ static uint64_t t24_committed(void) return atomic_load(&g_t24_p.committed_lsn); } -/* Test: the library commit index is the commit SOURCE once fed. A durable - * majority covers LSN 3, but the library has only admitted (current-term commit) - * up to LSN 1, so the barrier caps committed at 1 — NOT at the durable-majority - * floor of 3. Raising the library ceiling to 3 then releases the barrier to 3. - * This proves the library current-term commit index gates the commit, replacing - * the term-blind safe-commit (R4.1, R4.5, R5.4). +/* commit-ceiling-optA — REFRAMED from test_commit_advanced_is_commit_source. * - * (raft_commit_index == 0 is the INERT pre-wire state — the library source is - * not yet active and the barrier keeps autonomous durable-majority behavior; - * that flag-OFF fallback is covered by the existing test_commit_engine suite, - * whose autonomous semantics this task must not regress.) */ -static void test_commit_advanced_is_commit_source(void) + * The durable-majority barrier is AUTHORITATIVE for commit-advance; the recorded + * library commit_index is inert-advisory and does NOT cap committed_lsn. A + * durable majority covering LSN 3 commits to 3 EVEN WHILE the recorded library + * commit_index lags at 1 — this is the starvation the old ceiling caused (commit + * pinned at 1 below the safe durable majority) and the Option A fix removes. + * + * Meaningful both ways: it FAILS if commit were wrongly gated by the recorded + * index (committed would stick at 1), AND it FAILS if commit over-advanced past + * the durable majority (it asserts == 3, the exact durable watermark). */ +static void test_commit_advances_to_durable_majority_index_inert(void) { /* 3 peers = self + 2 standbys; majority needs self + 1 standby. */ t24_engine_setup(/*n_peers*/3, /*n_standbys*/2, /*appended*/3); - /* The library is the commit source from the first commit: establish the - * ceiling at LSN 1 BEFORE any ACK drives evaluation (production order — the - * ACK path drives raft_advance_commit, which fires on_commit_advanced, then - * the barrier re-evaluates). */ + /* Record an advisory library commit_index that LAGS at 1 (e.g. match_index[] + * not yet advanced via catch-up). Under the old design this pinned committed + * at 1; under Option A it is inert. */ commit_set_raft_commit_index(&g_t24_ce, 1); /* A standby ACKs LSN 3; self is durable to LSN 3 → durable majority = 3. - * The barrier must cap committed at the library ceiling (1), NOT at the - * durable majority (3). */ + * committed advances to the durable majority (3), NOT pinned at the lagging + * advisory index (1). */ t24_standby_ack(0, 3); - TEST_ASSERT_EQUAL_UINT64(1, t24_committed()); + TEST_ASSERT_EQUAL_UINT64(3, t24_committed()); - /* Library admits up to LSN 3 → barrier releases to the durable majority (3). */ + /* Advancing the advisory record changes nothing — already at the durable + * majority; record is inert. */ commit_set_raft_commit_index(&g_t24_ce, 3); TEST_ASSERT_EQUAL_UINT64(3, t24_committed()); t24_engine_teardown(); - printf("test_commit_advanced_is_commit_source: PASS\n"); + printf("test_commit_advances_to_durable_majority_index_inert: PASS\n"); } -/* Test: the library commit index is a CEILING — committed never advances past - * it even when a durable majority covers a higher LSN (current-term gate). The - * barrier's durable cap is the OTHER ceiling: committed = min(durable-majority, - * raft_commit_index). No gap: raising either ceiling alone cannot overshoot the - * other (R4.1, R4.5). */ -static void test_commit_advanced_ceiling_caps_durable_majority(void) +/* commit-ceiling-optA — REFRAMED from test_commit_advanced_ceiling_caps_durable_majority. + * + * The recorded library commit_index is NOT a ceiling: committed advances to the + * full durable majority even when the advisory index sits below it. Pins the + * removal of the redundant ceiling. + * + * Meaningful both ways: FAILS if the advisory index still capped committed at 3 + * (the old, now-rejected behavior), AND FAILS if committed over-advanced past + * the durable majority of 5 (asserts == 5 exactly). */ +static void test_commit_index_record_is_not_a_ceiling(void) { t24_engine_setup(/*n_peers*/3, /*n_standbys*/2, /*appended*/5); - /* Establish the library ceiling at 3 BEFORE any ACK (production order). The - * library has only committed up to 3 (higher entries prev-term or not yet - * current-term-agreed). */ + /* Advisory record lags at 3 while the durable majority will reach 5. */ commit_set_raft_commit_index(&g_t24_ce, 3); /* Durable majority covers up to LSN 5 (self durable 5 + standby acked 5). - * committed must cap at the library ceiling (3), not the durable floor (5). */ + * committed advances to 5 — the lagging advisory index (3) does NOT cap it. */ t24_standby_ack(0, 5); - TEST_ASSERT_EQUAL_UINT64(3, t24_committed()); + TEST_ASSERT_EQUAL_UINT64(5, t24_committed()); - /* Library advances the ceiling to 5 → committed catches up to the durable - * majority floor (5). Monotonic, no regression. */ + /* Catching the advisory record up to 5 is a no-op (already there). */ commit_set_raft_commit_index(&g_t24_ce, 5); TEST_ASSERT_EQUAL_UINT64(5, t24_committed()); t24_engine_teardown(); - printf("test_commit_advanced_ceiling_caps_durable_majority: PASS\n"); + printf("test_commit_index_record_is_not_a_ceiling: PASS\n"); } -/* Test: the library commit index NEVER lets committed advance past what is - * durable locally. With raft_commit_index=4 but self only durable to LSN 2 and - * the standby acked LSN 2, committed must stay at the durable-majority floor (2) - * — the library ceiling does not bypass persist-before-ACK / durable cap - * (R5.4 — barrier durability mechanism unchanged). */ +/* commit-ceiling-optA — RETAINED (unchanged contract): committed never advances + * past what is durable locally. The durable self-cap (persist_durable_lsn) is + * still authoritative regardless of any advisory library index. With the + * advisory index well ahead (4) but self durable only to LSN 2 and the standby + * acked LSN 2, committed stays at the durable-majority floor (2). This invariant + * is unchanged by Option A — the durable cap was always orthogonal to the + * removed ceiling — but is re-pinned here to guard Commit Durability (R5.4). */ static void test_commit_advanced_does_not_bypass_durable_cap(void) { t24_engine_setup(/*n_peers*/3, /*n_standbys*/2, /*appended*/4); atomic_store(&g_t24_p.durable_lsn, 2); /* self durable only to 2 */ t24_standby_ack(0, 2); /* standby agrees to 2 */ - commit_set_raft_commit_index(&g_t24_ce, 4); /* library ceiling well ahead */ + commit_set_raft_commit_index(&g_t24_ce, 4); /* advisory index well ahead */ commit_on_ack(&g_t24_ce, 0, 2); - /* Durable-majority floor is 2; the library ceiling (4) cannot raise it. */ + /* Durable-majority floor is 2; nothing (advisory index 4 included) raises it. */ TEST_ASSERT_EQUAL_UINT64(2, t24_committed()); t24_engine_teardown(); printf("test_commit_advanced_does_not_bypass_durable_cap: PASS\n"); } +/* commit-ceiling-optA — NEW load-bearing safety test: the own-term writable gate + * (Figure-8). With Option A removing the library-commit-index ceiling, the + * own-term NOOP gate (commit_set_writable_lsn / wait_done_locked) is now the + * load-bearing realization of "no acked-committed prior-term entry until the + * current-term NOOP is durable on a majority". + * + * Scenario: self holds a PRIOR-term entry at LSN 1 and the current-term NOOP at + * LSN 2 (the promotion NOOP). The own-term gate is armed at LSN 2. A durable + * majority reaches LSN 1 FIRST (the NOOP not yet majority-durable): + * - committed_lsn MAY advance to 1 (the barrier is contiguous-prefix), but a + * WAIT for LSN 1 must NOT complete — the gate blocks acknowledgement until + * committed_lsn >= writable_lsn (2). This is the safety property: the + * prior-term entry at LSN 1 is NOT acknowledged-committed yet. + * Then the NOOP becomes majority-durable (committed reaches 2): + * - the gate opens and the WAIT completes. + * + * RED if the writable gate were bypassed: a WAIT(lsn=1) would complete as soon + * as committed_lsn >= 1, acknowledging a prior-term entry before the own-term + * NOOP is majority-durable — the Figure-8 violation. GREEN: it blocks (times + * out) until the NOOP commits, then completes. */ +static void test_own_term_gate_blocks_priorterm_ack_until_noop_durable(void) +{ + /* 3 peers; majority needs self + 1 standby. Appended tip = LSN 2 (prior-term + * entry at 1 + current-term NOOP at 2). */ + t24_engine_setup(/*n_peers*/3, /*n_standbys*/2, /*appended*/2); + + /* Arm the own-term gate at the NOOP's LSN (2): no acknowledgement of any + * entry until committed_lsn >= 2 (the current-term NOOP is majority-durable). */ + commit_set_writable_lsn(&g_t24_ce, 2); + + /* Durable majority reaches LSN 1 only: self durable to 2, but the standby has + * acked just LSN 1, so the median (self + 1 of 2 peers) covers LSN 1. */ + atomic_store(&g_t24_p.durable_lsn, 2); + t24_standby_ack(0, 1); + + /* The contiguous-prefix barrier MAY advance committed_lsn to 1 ... */ + TEST_ASSERT_EQUAL_UINT64(1, t24_committed()); + + /* ... but a WAIT for LSN 1 must NOT complete: the own-term gate (writable_lsn + * == 2) holds it. Pre-NOOP-durable, this is the Figure-8 safety block — the + * prior-term entry at LSN 1 is committed-but-not-acknowledged. A short + * timeout proves the gate blocks (returns -ETIMEDOUT, not 0). */ + TEST_ASSERT_EQUAL_INT(-ETIMEDOUT, + commit_submit(&g_t24_ce, /*lsn*/1, COMMIT_MODE_WAIT, /*timeout_ms*/150)); + + /* The current-term NOOP at LSN 2 now becomes majority-durable (standby acks + * LSN 2). committed_lsn advances to 2 and the gate opens. */ + t24_standby_ack(0, 2); + TEST_ASSERT_EQUAL_UINT64(2, t24_committed()); + + /* The WAIT for LSN 1 now completes: committed_lsn (2) >= lsn (1) AND + * >= writable_lsn (2). The prior-term entry is acknowledged only AFTER the + * own-term NOOP is majority-durable. */ + TEST_ASSERT_EQUAL_INT(0, + commit_submit(&g_t24_ce, /*lsn*/1, COMMIT_MODE_WAIT, /*timeout_ms*/500)); + + t24_engine_teardown(); + printf("test_own_term_gate_blocks_priorterm_ack_until_noop_durable: PASS\n"); +} + /* Test (pins the own-term-gate DECISION — RETAINED, complementary, no deadlock): * the own-term writable_lsn gate is a WAIT-completion predicate, NOT a cap on - * committed_lsn. It is retained alongside the library commit source. With the - * gate armed at LSN 2 and the library commit index + durable majority both at 2, - * committed advances to 2 AND a WAIT(lsn=2) completes — the two gates cannot - * deadlock (the own-term gate never holds committed_lsn back; the library - * ceiling never holds the WAIT predicate back beyond committed_lsn). */ + * committed_lsn. With the gate armed at LSN 2 and the durable majority at 2, + * committed advances to 2 AND a WAIT(lsn=2) completes — the gate never holds + * committed_lsn back, and once committed reaches the gate the WAIT completes (no + * deadlock). The advisory library index is irrelevant here (inert under optA). */ static void test_own_term_gate_retained_no_deadlock(void) { t24_engine_setup(/*n_peers*/3, /*n_standbys*/2, /*appended*/2); - /* Arm the own-term gate at LSN 2 (retained complementary durability gate). */ + /* Arm the own-term gate at LSN 2 (the load-bearing complementary gate). */ commit_set_writable_lsn(&g_t24_ce, 2); - /* Library commit source + durable majority both reach LSN 2. */ + /* Durable majority reaches LSN 2. */ t24_standby_ack(0, 2); - commit_set_raft_commit_index(&g_t24_ce, 2); + commit_set_raft_commit_index(&g_t24_ce, 2); /* advisory; inert */ commit_on_ack(&g_t24_ce, 0, 2); /* committed_lsn advanced to 2 (own-term gate never caps committed_lsn). */ TEST_ASSERT_EQUAL_UINT64(2, t24_committed()); /* A WAIT for LSN 2 completes immediately: committed_lsn (2) >= lsn (2) AND - * >= writable_lsn (2). No deadlock between the gate and the library source. */ + * >= writable_lsn (2). No deadlock between the gate and the barrier. */ TEST_ASSERT_EQUAL_INT(0, commit_submit(&g_t24_ce, /*lsn*/2, COMMIT_MODE_WAIT, /*timeout_ms*/500)); @@ -804,14 +866,20 @@ static void test_own_term_gate_retained_no_deadlock(void) printf("test_own_term_gate_retained_no_deadlock: PASS\n"); } -/* Test: the WIRED on_commit_advanced callback routes commit_index into the - * cluster's commit engine end-to-end (ha_cb_commit_advanced_for_test) as the - * commit SOURCE. A durable majority covers LSN 2, but the wired callback feeds - * commit_index=1 first → the barrier caps committed at 1 (library current-term - * gate), NOT at the durable majority of 2. A second callback at commit_index=2 - * releases the barrier to 2. Proves the stub→real transition: the no-op stub - * never fed the source, so committed would have followed the durable majority. */ -static void test_on_commit_advanced_wired_feeds_barrier(void) +/* commit-ceiling-optA — REFRAMED from test_on_commit_advanced_wired_feeds_barrier. + * + * The WIRED on_commit_advanced callback (ha_cb_commit_advanced_for_test) routes + * the library commit_index into the cluster's commit engine as an INERT-ADVISORY + * record — it records ce->raft_commit_index and wakes the barrier, but does NOT + * gate committed_lsn. The durable-majority barrier is authoritative: with a + * durable majority covering LSN 2, committed advances to 2 even though the wired + * callback last fed a LAGGING commit_index of 1. + * + * Meaningful both ways: FAILS if the wired callback still capped committed at 1 + * (the old, now-rejected ceiling — the starvation), AND FAILS if committed + * over-advanced past the durable majority of 2 (asserts == 2). Also pins that + * the wired callback still RECORDS the advisory index (sink intact). */ +static void test_on_commit_advanced_wired_records_advisory_index(void) { char tmp[256]; strcpy(tmp, "/tmp/xce_ts_commitadv_XXXXXX"); @@ -842,25 +910,26 @@ static void test_on_commit_advanced_wired_feeds_barrier(void) TEST_ASSERT_EQUAL_INT(0, repl_add_standby(&c->persist.repl, &g_ae_dummy_ep, "sb0", 7000)); - /* Fire the WIRED callback FIRST to establish the library ceiling at LSN 1 - * (production order: the ACK path drives raft_advance_commit, which fires - * on_commit_advanced before/with the barrier re-evaluation). */ + /* Fire the WIRED callback with a LAGGING commit_index of 1 (e.g. match_index[] + * not yet caught up). Under optA this only RECORDS the advisory index. */ ha_cb_commit_advanced_for_test(&c->ha, 1); + TEST_ASSERT_EQUAL_UINT64(1, c->commit.raft_commit_index); /* sink intact */ - /* Standby ACKs LSN 2 → durable majority covers LSN 2. The wired source caps - * committed at 1, BELOW the durable majority (2): the library current-term - * gate governs (the term-blind safe-commit is retired as the source). */ + /* Standby ACKs LSN 2 → durable majority covers LSN 2. committed advances to + * the durable majority (2), NOT pinned at the lagging advisory index (1): + * the advisory record does not gate commit. */ atomic_store(&c->persist.repl.standbys[0].acked_lsn, 2); commit_on_ack(&c->commit, 0, 2); - TEST_ASSERT_EQUAL_UINT64(1, atomic_load(&c->persist.committed_lsn)); + TEST_ASSERT_EQUAL_UINT64(2, atomic_load(&c->persist.committed_lsn)); - /* Library admits LSN 2 → barrier releases to the durable majority (2). */ + /* A later callback advancing the advisory record to 2 is a no-op for commit. */ ha_cb_commit_advanced_for_test(&c->ha, 2); + TEST_ASSERT_EQUAL_UINT64(2, c->commit.raft_commit_index); TEST_ASSERT_EQUAL_UINT64(2, atomic_load(&c->persist.committed_lsn)); commit_engine_fini(&c->commit); teardown_cluster(c); - printf("test_on_commit_advanced_wired_feeds_barrier: PASS\n"); + printf("test_on_commit_advanced_wired_records_advisory_index: PASS\n"); } /* ================================================================== */ @@ -1355,6 +1424,111 @@ static void test_live_broadcast_receive_persists_leader_term(void) printf("test_live_broadcast_receive_persists_leader_term: PASS\n"); } +/* ================================================================== */ +/* bug-991 — catch-up applied_lsn must be "last contiguously-held LSN" */ +/* (NOT next-to-apply / lsn+1) so the leader's catch-up */ +/* streamer never SKIPS the standby's real next-needed LSN. */ +/* ================================================================== */ + +/* The standby's applied_lsn is the watermark it advertises to the leader + * (ha_nop_reply_state_report → sr.applied_lsn → standby_reported_lsn). The + * leader's catch-up streamer (ha_stream_wal_to_standby) and the snapshot/stream + * decision (ha_evaluate_repl_action) BOTH treat that value as the + * "last contiguously-held LSN" and stream the half-open range + * (standby_lsn, committed_lsn]. The recovery/checkpoint/init setters already use + * last-held semantics (persist_durable_floor / checkpoint_lsn / 0). The three + * live/catch-up apply paths historically advanced to lsn+1 (next-to-apply) — + * an off-by-one that made the standby advertise one LSN too high, so after it + * applied catch-up entry N the leader streamed (N+1, committed] and SKIPPED the + * real next-needed entry N+1 forever (logOk wedge → no WalAck → commit stall). + * + * RED (pre-fix): after applying a catch-up entry at LSN N the standby's + * applied_lsn reads N+1, and ha_evaluate_repl_action(standby_lsn=applied_lsn, + * committed=N) reports CONVERGED one entry early (skip). + * GREEN (post-fix): applied_lsn reads N (last-held); a standby that has applied + * up to N with committed > N still STREAMS (no skip), and the streamer's + * (standby_lsn, committed] range begins at exactly N+1. */ +static void test_catchup_applied_lsn_is_last_held_no_skip(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_ts_bug991_lasthelp_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + cluster_t* c = make_cluster(tmp); + TEST_ASSERT_NOT_NULL(c); + make_follower(c, /*term*/7, /*n_peers*/3); + + /* applied_lsn starts at 0 (init = empty/last-held-nothing). */ + TEST_ASSERT_EQUAL_UINT64(0, atomic_load(&c->ha.applied_lsn)); + + /* Catch-up stream of LSN 1 then LSN 2 at the leader's term 7, contiguous + * (prev tuples chain 0/0 → 1/7), routed through the production receive entry + * point exactly as ha_stream_wal_to_standby → xce_decode would deliver them. */ + uint8_t p1[1] = { 'a' }; + wal_catchup_entry_t sub1; + memset(&sub1, 0, sizeof(sub1)); + sub1.lsn = 1; sub1.entry_type = WAL_JOB_SUBMIT; sub1.term = 7; + sub1.prev_log_index = 0; sub1.prev_log_term = 0; + sub1.raw_payload = p1; sub1.raw_payload_len = sizeof(p1); + wal_catchup_entry_t* arr1[1] = { &sub1 }; + wal_catchup_entry_t batch1; + memset(&batch1, 0, sizeof(batch1)); + batch1.n_entries = 1; batch1.entries = arr1; + batch1.term = 7; batch1.prev_log_index = 0; batch1.prev_log_term = 0; + + TEST_ASSERT_EQUAL_INT(0, + ha_apply_wal_catchup_entry(&c->ha, &c->persist, &batch1, /*ep*/NULL, c)); + TEST_ASSERT_EQUAL_UINT64(7, persist_entry_term(&c->persist, 1)); + + /* THE bug-991 load-bearing assertion: after applying LSN 1 the standby + * advertises applied_lsn == 1 (last-held), NOT 2 (next-to-apply). Pre-fix + * this read 2 and the leader's next stream skipped LSN 2. */ + TEST_ASSERT_EQUAL_UINT64(1, atomic_load(&c->ha.applied_lsn)); + + /* No-skip property at the leader's decision seam: a standby holding up to + * LSN 1 with committed_lsn == 2 must STREAM (its next-needed LSN 2 is missing), + * NOT report CONVERGED. With the off-by-one advertised value (2) the leader + * would have seen standby_lsn(2) >= committed(2) → CONVERGED (the skip). */ + { + uint64_t standby_lsn = atomic_load(&c->ha.applied_lsn); + ha_repl_action_t act = ha_evaluate_repl_action( + /*standby_tag*/1, standby_lsn, + /*committed_tag*/1, /*committed_lsn*/2, /*oldest_wal_lsn*/1); + TEST_ASSERT_EQUAL_INT(HA_REPL_STREAM_WAL, act); + /* The streamer's start LSN is standby_lsn + 1 == 2 — exactly the missing + * entry, no skip. */ + TEST_ASSERT_EQUAL_UINT64(2, standby_lsn + 1); + } + + /* Apply the next contiguous entry (LSN 2) — applied_lsn becomes 2 (last-held), + * not 3 — and the standby is now CONVERGED at committed_lsn == 2. */ + uint8_t p2[1] = { 'b' }; + wal_catchup_entry_t sub2; + memset(&sub2, 0, sizeof(sub2)); + sub2.lsn = 2; sub2.entry_type = WAL_JOB_SUBMIT; sub2.term = 7; + sub2.prev_log_index = 1; sub2.prev_log_term = 7; + sub2.raw_payload = p2; sub2.raw_payload_len = sizeof(p2); + wal_catchup_entry_t* arr2[1] = { &sub2 }; + wal_catchup_entry_t batch2; + memset(&batch2, 0, sizeof(batch2)); + batch2.n_entries = 1; batch2.entries = arr2; + batch2.term = 7; batch2.prev_log_index = 1; batch2.prev_log_term = 7; + + TEST_ASSERT_EQUAL_INT(0, + ha_apply_wal_catchup_entry(&c->ha, &c->persist, &batch2, /*ep*/NULL, c)); + TEST_ASSERT_EQUAL_UINT64(7, persist_entry_term(&c->persist, 2)); + TEST_ASSERT_EQUAL_UINT64(2, atomic_load(&c->ha.applied_lsn)); + { + ha_repl_action_t act = ha_evaluate_repl_action( + /*standby_tag*/1, atomic_load(&c->ha.applied_lsn), + /*committed_tag*/1, /*committed_lsn*/2, /*oldest_wal_lsn*/1); + TEST_ASSERT_EQUAL_INT(HA_REPL_CONVERGED, act); + } + + teardown_cluster(c); + printf("test_catchup_applied_lsn_is_last_held_no_skip: PASS\n"); +} + /* ================================================================== */ /* Task 3.3 — drive raft_on_append_entries_reply + raft_advance_commit */ /* on the replication ACK path */ @@ -1401,11 +1575,13 @@ static void leader_seed_entries(cluster_t* c, uint64_t term, int count) /* (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. */ + * which is recorded as the advisory library index AND wakes the barrier. 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 the library commit_index advances to 2; the durable + * majority (also at 2) is what advances committed_lsn (optA: the durable-majority + * barrier is authoritative; the recorded index is inert-advisory). This leg + * still verifies the full ACK→library→callback→barrier wiring is intact. */ static void test_repl_ack_currentterm_quorum_advances_commit(void) { char tmp[256]; @@ -1429,13 +1605,16 @@ static void test_repl_ack_currentterm_quorum_advances_commit(void) 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. */ + /* Durable majority: the standby's durable position covers LSN 2. Under optA + * the durable-majority barrier is authoritative, so this ACK already advances + * committed_lsn to 2 (asserted at the end); the library wiring below is still + * exercised to confirm the ACK→library→callback path is intact. */ 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. */ + /* The library has not advanced commit_index yet (no ACK routed through the + * library) so the advisory record is still 0 — but committed_lsn is NOT + * gated by it (optA). */ TEST_ASSERT_EQUAL_UINT64(0, c->ha.raft.commit_index); /* The standby ACKs LSN 2 at the leader's current term. The caller has @@ -1452,9 +1631,12 @@ static void test_repl_ack_currentterm_quorum_advances_commit(void) * ({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). */ + /* (3) committed_lsn advanced to the durable majority (2). on_commit_advanced + * fired and recorded the advisory index (2), but the durable-majority barrier + * is what governs committed_lsn under optA (R4.1 realized via the authoritative + * durable barrier + own-term gate). */ TEST_ASSERT_EQUAL_UINT64(2, atomic_load(&c->persist.committed_lsn)); + TEST_ASSERT_EQUAL_UINT64(2, c->commit.raft_commit_index); /* advisory recorded */ commit_engine_fini(&c->commit); teardown_cluster(c); @@ -1787,27 +1969,30 @@ static void test_every_callback_and_marker_path_end_to_end(void) TEST_ASSERT_EQUAL_INT(-ENOENT, config_committed_marker(c, &got_tag, got_sha)); - /* on_commit_advanced is LOAD-BEARING (production order: the ACK path fires - * on_commit_advanced, which records the library ceiling, BEFORE the barrier - * re-evaluates the durable majority). Fire the WIRED callback at - * commit_index=1 FIRST to set the ceiling at 1, THEN supply a durable - * majority covering LSN 2. committed_lsn caps at the library ceiling (1), - * BELOW the durable majority (2). This can only hold if on_commit_advanced - * fed the library current-term commit point as the barrier ceiling (the - * term-blind safe-commit is no longer the source); a no-op callback would - * let the barrier release committed_lsn to the durable majority (2). */ + /* on_commit_advanced records the advisory library index and wakes the + * barrier (optA: it does NOT gate committed_lsn). Production order: the ACK + * path fires on_commit_advanced with a (possibly lagging) commit_index, then + * the durable-majority barrier — which is authoritative — advances + * committed_lsn. Fire the WIRED callback with a LAGGING commit_index=1, then + * supply a durable majority covering LSN 2: committed_lsn advances to the + * durable majority (2), NOT pinned at the lagging advisory index (1). Under + * the OLD (rejected) ceiling design this would have stuck committed at 1 — + * the starvation. The advisory index is still RECORDED (sink intact). */ ha_cb_commit_advanced_for_test(&c->ha, 1); + TEST_ASSERT_EQUAL_UINT64(1, c->commit.raft_commit_index); /* advisory recorded */ atomic_store(&c->persist.repl.standbys[0].acked_lsn, 2); commit_on_ack(&c->commit, /*peer_idx*/0, /*acked_lsn*/2); - TEST_ASSERT_EQUAL_UINT64(1, atomic_load(&c->persist.committed_lsn)); + TEST_ASSERT_EQUAL_UINT64(2, atomic_load(&c->persist.committed_lsn)); - /* Now route the real current-term quorum ACK through the library + /* Route the real current-term quorum ACK through the library * (raft_on_append_entries_reply → raft_advance_commit → on_commit_advanced). - * The agreed entries are current-term (T) so the library commit advances to - * 2 and the callback raises the ceiling, releasing committed_lsn to 2. */ + * The agreed entries are current-term (T) so the library commit advances to 2 + * and the callback records the advisory index 2. committed_lsn is already 2 + * (durable barrier). This still exercises the full ACK→library→callback wiring. */ TEST_ASSERT_EQUAL_INT(0, ha_raft_on_repl_ack(c, /*abs_peer_idx*/1, /*acked_lsn*/2, /*reply_term*/T)); TEST_ASSERT_EQUAL_UINT64(2, c->ha.raft.commit_index); + TEST_ASSERT_EQUAL_UINT64(2, c->commit.raft_commit_index); /* advisory recorded */ TEST_ASSERT_EQUAL_UINT64(2, atomic_load(&c->persist.committed_lsn)); /* The marker (LSN 2) is now in the committed prefix → the DERIVED committed @@ -1864,11 +2049,12 @@ void test_term_safety_adapter(void) RUN_TEST(test_on_truncate_from_removes_all); RUN_TEST(test_send_append_entries_one_entry_from_wal); RUN_TEST(test_send_append_entries_heartbeat_zero_entries); - RUN_TEST(test_commit_advanced_is_commit_source); - RUN_TEST(test_commit_advanced_ceiling_caps_durable_majority); + RUN_TEST(test_commit_advances_to_durable_majority_index_inert); + RUN_TEST(test_commit_index_record_is_not_a_ceiling); RUN_TEST(test_commit_advanced_does_not_bypass_durable_cap); + RUN_TEST(test_own_term_gate_blocks_priorterm_ack_until_noop_durable); RUN_TEST(test_own_term_gate_retained_no_deadlock); - RUN_TEST(test_on_commit_advanced_wired_feeds_barrier); + RUN_TEST(test_on_commit_advanced_wired_records_advisory_index); RUN_TEST(test_init_restores_recovered_floor); RUN_TEST(test_init_empty_wal_zero_floor); RUN_TEST(test_primary_log_append_stamps_term_and_assigns_lsn); @@ -1878,6 +2064,7 @@ void test_term_safety_adapter(void) RUN_TEST(test_intake_divergent_tail_truncated_and_reconverged); RUN_TEST(test_intake_logmismatch_rejected_no_false_convergence); RUN_TEST(test_live_broadcast_receive_persists_leader_term); + RUN_TEST(test_catchup_applied_lsn_is_last_held_no_skip); 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);