diff --git a/.kiro/specs/ha-commit-barrier-rollout/tasks.md b/.kiro/specs/ha-commit-barrier-rollout/tasks.md index f4e4460..2de14f9 100644 --- a/.kiro/specs/ha-commit-barrier-rollout/tasks.md +++ b/.kiro/specs/ha-commit-barrier-rollout/tasks.md @@ -25,7 +25,7 @@ - Observable completion: new cases FAIL (red) — the helper does not exist yet - _Requirements: 1.1, 1.2, 1.3_ -- [ ] 2.2 Implement the barrier helper and retire the legacy walAck counting +- [x] 2.2 Implement the barrier helper and retire the legacy walAck counting - Implement `persist_commit_barrier` in persist.c: snapshot `current_lsn` under `p->lock` (mirror persist.c:1019-1021), short-circuit when covered, else `commit_submit(ce, lsn, COMMIT_MODE_WAIT, timeout)`; return engine code unmodified - repl.c `repl_wait_majority_ack` (repl.c:386-462 anchor): remove the legacy walAck counting branch (upstream marked it "retired by ha-commit-barrier-rollout") — the walAck path delegates to the engine unconditionally; the function's remaining body is the configAck track (hardened in task 5) - Observable completion: all task 2.1 tests PASS; grep shows no remaining `acked_lsn > 0` counting and no `n_active == 0 → success` short-circuit reachable from any walAck call path @@ -112,4 +112,5 @@ - (1.1) wal_entry_type has 13 values (wal_types.h:44-58), no WAL_NOOP yet (lands in ha-single-leader-role, which must add a classification row + sweep entry — the totality sweep will catch it). - (1.1) Suite baseline is ZERO failures since ha-commit-engine 6.1. detect_leaks=0 remains the harness norm (6 inventoried pre-existing leak groups). - (1.2) CENSUS METHOD: only ~9 of 48 groups emit Unity summary blocks; the rest report per-test PASS lines only — count distinct Unity results (~465) and grep FAIL lines, never sum summary blocks. commit_submit(RETAIN) early-returns BEFORE taking ce->lock (commit.c:233-239) — registration is lock-free, safe with entity write locks held. wal_hook_append_classified is the single classified-append seam; appended_lsn read = current_lsn - 1 under p->lock (benign race, commented). +- (2.2) Legacy walAck counting RETIRED: walAck track = entry_lsn==0 → 0 (LSN-0 interim) else unconditional commit_submit (ce==NULL → -EINVAL, fail-loud). configAck track byte-equivalent (task 5's surface intact, incl. its n_active==0 shortcut and acked_config_lsn>0 counting — both die at task 5). persist_commit_barrier: ce==NULL → 0 (vacuous, documented). test_repl_unit's two lsn=0 walAck callers are now vacuous for quorum counting (INFO for task 5/7.2). CENSUS = 472 (465 at 1.2 + 7 from 2.1). - (2.1) BARRIER WATERMARK RESOLVED (reviewer-judged): design's literal "Snapshot lsn = p->current_lsn" is loose phrasing contradicted by its own anchors — the coherent contract is watermark = current_lsn - 1 (last appended; persist.h:71 invariant committed_lsn <= current_lsn - 1 makes the literal reading unsatisfiable). Tests pin current_lsn - 1 with off-by-one killing power in BOTH directions; task 2.2 must implement that. Resolved contract recorded in persist.h doc comment. diff --git a/.wolf/cerebrum.md b/.wolf/cerebrum.md index 5ad1299..7be787b 100644 --- a/.wolf/cerebrum.md +++ b/.wolf/cerebrum.md @@ -9,6 +9,7 @@ ## Key Learnings +- repl_wait_majority_ack walAck track is engine-only since barrier-rollout 2.2: entry_lsn==0 returns 0 immediately (documented once-per-lifetime LSN-0 first entry), everything else delegates to commit_submit unconditionally — ce==NULL surfaces as -EINVAL from commit_submit (unreachable in production; cluster_init_phase2 always wires the engine). The counting loop that remains in the function is configAck-ONLY (no self-count, n_active==0 shortcut is Req 8.5 config behavior). persist_wal_sync_append's residual non-engine branch can only store committed_lsn for the LSN-0 no-op case. (2026-06-12, task 2.2) - `commit_submit(COMMIT_MODE_RETAIN)` is fully lock-free: it returns 0 BEFORE `pthread_mutex_lock(&ce->lock)` (commit.c:233-239), so it is safe to call from hook functions even while entity write locks are held — only WAIT mode takes ce->lock. (barrier-rollout task 1.2, 2026-06-12) - ha-commit-engine: once repl.c references commit_* symbols (task 5.2), commit.c MUST be in controller_src (meson.build) or the production xce_controller link fails — the "linkage lands with 5.3" note in tasks.md only held while repl.c had no engine references. Lock order ce->lock → r->lock: repl.c calls engine hooks only AFTER releasing r->lock. diff --git a/.wolf/memory.md b/.wolf/memory.md index 6ad5e90..13888ab 100644 --- a/.wolf/memory.md +++ b/.wolf/memory.md @@ -994,3 +994,12 @@ | 05:05 | ha-commit-barrier-rollout task 2.1: RED tests for persist_commit_barrier (7 cases) + decl + inert -ENOSYS stub | test_commit_barrier_rollout.c, persist.h, persist.c | 7 FAIL (RED) on -38 sentinel, prior 8 PASS, suite census clean | ~60k | | 22:08 | Edited .kiro/specs/ha-commit-barrier-rollout/tasks.md | inline fix | ~17 | | 22:08 | Edited .kiro/specs/ha-commit-barrier-rollout/tasks.md | modified RESOLVED() | ~256 | +| 22:14 | Edited src/controller/persist.c | added 4 condition(s) | ~640 | +| 22:15 | Edited src/controller/repl.c | added 1 condition(s) | ~657 | +| 22:15 | Edited src/controller/repl.c | modified if() | ~180 | +| 22:16 | Edited src/controller/persist.c | modified if() | ~342 | +| 22:20 | Edited src/controller/repl.c | 5→5 lines | ~96 | +| 05:25 | barrier-rollout 2.2: persist_commit_barrier implemented (watermark=current_lsn-1, covered short-circuit, ce==NULL→0, timeout fallback) + legacy walAck counting removed from repl_wait_majority_ack (LSN-0→0, unconditional commit_submit; configAck track untouched) | src/controller/persist.c, src/controller/repl.c | 15/15 group, 472 Unity / 0 FAIL | ~35k | +| 05:45 | Adversarial review ha-commit-barrier-rollout task 2.2 (barrier helper + walAck retirement): boundary clean (persist.c+repl.c only), 15/15 group, full census 472 Unity PASS / 0 FAIL (= 465 prior + 7 new), configAck byte-equivalent, LSN-0 absorption commented, ce==NULL asymmetry judged defensible — APPROVED | src/controller/persist.c, src/controller/repl.c | APPROVED | ~30k | +| 22:36 | Edited .kiro/specs/ha-commit-barrier-rollout/tasks.md | inline fix | ~21 | +| 22:36 | Edited .kiro/specs/ha-commit-barrier-rollout/tasks.md | modified RESOLVED() | ~148 | diff --git a/src/controller/persist.c b/src/controller/persist.c index 035e25d..b80c981 100644 --- a/src/controller/persist.c +++ b/src/controller/persist.c @@ -1042,10 +1042,13 @@ int persist_wal_sync_append(persist_t* p, uint32_t type, if (sync_err != 0) CTRL_METRIC_INC(ctrl_repl_degraded_count); } else { - /* DEPRECATED legacy branch: engine-less (ce == NULL) unit-test - * contexts plus the LSN-0 first entry. Connectivity-based counting; - * committed_lsn is stored here only because no engine owns it. - * Retired by ha-commit-barrier-rollout. */ + /* Residual non-engine branch: engine-less (ce == NULL) unit-test + * contexts plus the LSN-0 first entry. The legacy connectivity- + * based counting was retired by ha-commit-barrier-rollout task 2.2: + * repl_wait_majority_ack now confirms LSN 0 immediately (documented + * once-per-lifetime edge) and surfaces engine-less walAck waits as + * -EINVAL via commit_submit, so the committed_lsn store below only + * fires for the LSN-0 case (a no-op store of 0). */ mylock_lock(&p->repl.lock); int has_standbys = (p->repl.nstandbys > 0); mylock_unlock(&p->repl.lock); @@ -1171,19 +1174,58 @@ wal_commit_mode_t wal_classify(uint32_t wal_type, int detail) } } -/* Commit barrier helper — INERT RED stub (ha-commit-barrier-rollout task - * 2.1; implemented in task 2.2). Returns a wrong sentinel so the task-2.1 - * tests fail on assertions, not linking. Task-2.2 contract (design.md - * §Commit Barrier Helper): snapshot the appended watermark (current_lsn - 1 - * under p->lock), return 0 when committed_lsn already covers it (or nothing - * appended), else commit_submit(WAIT, timeout_ms ? timeout_ms : - * p->config.sync_timeout_ms) with the engine code passed through - * unmodified. */ +/* Commit barrier helper (ha-commit-barrier-rollout task 2.2, design.md + * §Commit Barrier Helper; Req 1.1, 1.2, 1.3). + * + * Handler-callable WAIT completion on the appended watermark: snapshot the + * last-appended LSN, return 0 when the commit index already covers it, + * else block in the commit engine until coverage or deadline. The engine + * result is returned UNMODIFIED (0 / -ETIMEDOUT / -XCE_ERR_NOT_READY / + * -EINVAL) so callers map it to their own error replies. + * + * Lock discipline: p->lock is held only for the current_lsn snapshot and + * is released BEFORE commit_submit(WAIT) — the engine blocks on ce->lock + * + condvar, and p->lock must never be held across it (engine + * lock-ordering rule). */ int persist_commit_barrier(persist_t* p, uint32_t timeout_ms) { - (void)p; - (void)timeout_ms; - return -ENOSYS; + uint64_t watermark; + + if (p == NULL) + return -EINVAL; + + /* Snapshot the last-appended watermark under p->lock. current_lsn is + * the NEXT LSN to assign (persist.h invariant: committed_lsn <= + * current_lsn - 1), so the highest appended entry is current_lsn - 1 + * (resolved contract, spec Implementation Notes 2.1). current_lsn == + * 0 means nothing was ever appended — vacuously satisfied. */ + pthread_mutex_lock(&p->lock); + if (p->current_lsn == 0) { + pthread_mutex_unlock(&p->lock); + return 0; + } + watermark = p->current_lsn - 1; + pthread_mutex_unlock(&p->lock); + + /* Already covered — committed_lsn is monotone, so coverage observed + * here can never regress. This check also absorbs the LSN-0 first + * entry (watermark 0 vs committed_lsn 0): that once-per-lifetime entry + * has no quorum contract (ha-commit-engine Implementation Notes) and + * commit_submit would reject lsn == 0 anyway. */ + if (atomic_load_explicit(&p->committed_lsn, memory_order_acquire) >= + watermark) + return 0; + + /* Engine not wired: legacy/bootstrap contexts have no engine commit + * contract — there is nothing to wait on, so the barrier is a no-op. + * Production always wires the engine (cluster_init_phase2). */ + if (p->repl.ce == NULL) + return 0; + + /* timeout_ms == 0 borrows the configured sync deadline (Req 1.2). */ + return commit_submit(p->repl.ce, watermark, COMMIT_MODE_WAIT, + timeout_ms != 0 ? timeout_ms + : p->config.sync_timeout_ms); } /* Classified append at the hook seam (barrier-rollout task 1.2). diff --git a/src/controller/repl.c b/src/controller/repl.c index b72ca86..898e2ef 100644 --- a/src/controller/repl.c +++ b/src/controller/repl.c @@ -399,17 +399,19 @@ void repl_on_config_ack(replicator_t *r, repl_ep_t ep, uint64_t acked_config_lsn } /* ============================================================ - * Majority-quorum ACK wait (task 7.3) + * Majority-quorum ACK wait (task 7.3; walAck track rebased onto the + * commit engine by ha-commit-barrier-rollout task 2.2) * - * Quorum formula: quorum = (n_active + 2) / 2 [ceiling of (n_active+1)/2] - * n_active = count of active (non-catch-up) standbys. - * - * walAck: acked_total = 1 (self) + standbys with acked_lsn > 0. - * Primary counts itself — it persists WAL before broadcasting. - * configAck: acked_total = standbys with acked_config_lsn > 0 (no self-count). - * Config must be confirmed by standbys; primary does not self-count. - * - * Returns 0 on quorum, -ETIMEDOUT on timeout. + * walAck: per-LSN commit evaluation, delegated to the commit engine + * unconditionally (the legacy connectivity-based "ACKed + * anything ever" counting with self-count and the zero-active + * success short-circuit is retired). Returns the engine code + * unmodified: 0 / -ETIMEDOUT / -XCE_ERR_NOT_READY / -EINVAL. + * configAck: legacy standby counting (hardened by barrier-rollout task 5): + * quorum = (n_active + 2) / 2 over active (non-catch-up) + * standbys; acked_total = standbys with acked_config_lsn > 0, + * NO self-count (config must be confirmed by standbys). + * Returns 0 on quorum, -ETIMEDOUT on timeout. * ============================================================ */ int repl_wait_majority_ack(replicator_t *r, uint64_t entry_lsn, @@ -418,27 +420,28 @@ int repl_wait_majority_ack(replicator_t *r, uint64_t entry_lsn, if (r == NULL) return -EINVAL; - int is_wal_ack = (ack_op == (uint8_t)OPCode_walAck); + if (ack_op == (uint8_t)OPCode_walAck) { + /* LSN-0 edge: the first-ever WAL entry of a fresh cluster is LSN 0 + * (current_lsn starts at 0; written_lsn = current_lsn - 1) and is + * structurally un-commitable — commit_submit rejects lsn == 0. + * Documented once-per-cluster-lifetime interim behavior + * (ha-commit-engine Implementation Notes): confirm it immediately; + * the promotion no-op entry (ha-single-leader-role) eliminates the + * case in practice. configAck callers also pass entry_lsn = 0 but + * are dispatched by ack_op above, never through this guard. */ + if (entry_lsn == 0) + return 0; - /* Engine adapter (ha-commit-engine task 5.2, design §Job-Submit Adapter): - * the walAck barrier is per-LSN commit evaluation owned by the commit - * engine when wired. Caller holds no repl locks here, so the engine's - * ce->lock -> r->lock order is preserved. - * - * entry_lsn == 0 falls through to legacy counting: persist.c's - * persist_wal_sync_append still passes 0 until task 5.3 forwards - * written_lsn (and commit_submit rejects lsn==0 with -EINVAL). Note the - * first-ever WAL entry IS LSN 0 — structurally un-commitable under the - * current numbering; task 5.3 documents the resolution, and the - * promotion no-op entry (ha-single-leader-role) eliminates it in - * practice. configAck callers pass entry_lsn=0 and never delegate. */ - if (is_wal_ack && r->ce != NULL && entry_lsn != 0) { + /* Unconditional engine delegation (ha-commit-engine task 5.2, + * design §Job-Submit Adapter). Caller holds no repl locks here, so + * the engine's ce->lock -> r->lock order is preserved. ce == NULL + * (engine never wired) is unreachable in production — the engine + * is wired at cluster_init_phase2 — and commit_submit surfaces it + * as -EINVAL rather than this adapter fabricating a quorum result. */ return commit_submit(r->ce, entry_lsn, COMMIT_MODE_WAIT, timeout_ms); } - /* DEPRECATED legacy counting below (connectivity-based, not per-LSN): - * survives only for engine-less (ce == NULL) unit-test contexts and the - * configAck branch; retired by ha-commit-barrier-rollout. */ + /* ── configAck track (legacy standby counting; task 5 hardens it) ── */ struct timespec deadline; clock_gettime(CLOCK_REALTIME, &deadline); @@ -465,27 +468,23 @@ int repl_wait_majority_ack(replicator_t *r, uint64_t entry_lsn, if (atomic_load_explicit(&s->in_catch_up, memory_order_acquire)) continue; n_active++; - if (is_wal_ack) { - if (atomic_load_explicit(&s->acked_lsn, memory_order_acquire) > 0) - acked_count++; - } else { - if (atomic_load_explicit(&s->acked_config_lsn, - memory_order_acquire) > 0) - acked_count++; - } + if (atomic_load_explicit(&s->acked_config_lsn, + memory_order_acquire) > 0) + acked_count++; } mylock_unlock(&r->lock); - /* Single-node shortcircuit: no active standbys → local write is quorum. - * Applies to both walAck (self=1, quorum=1) and configAck (Req 8.5). */ + /* Single-node shortcircuit: no active standbys → local write is + * quorum (configAck only, Req 8.5 — walAck never reaches this + * loop; its single-member case is the engine's self-quorum). */ if (n_active == 0) { rc = 0; break; } int quorum = (n_active + 2) / 2; - int acked_total = is_wal_ack ? (1 + acked_count) : acked_count; + int acked_total = acked_count; /* no self-count for configAck */ if (acked_total >= quorum) { rc = 0;