fix(ha-term-safety): remove task-2.4 commit min-ceiling (Option A) + fix bug-991 catch-up off-by-one

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.
This commit is contained in:
Rongsong Shen 2026-06-23 00:21:06 +00:00
parent df4bc4f86a
commit 83fbed2370
5 changed files with 396 additions and 172 deletions

View file

@ -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);