diff --git a/.kiro/specs/ha-commit-engine/tasks.md b/.kiro/specs/ha-commit-engine/tasks.md index c0baad0..7f3f92f 100644 --- a/.kiro/specs/ha-commit-engine/tasks.md +++ b/.kiro/specs/ha-commit-engine/tasks.md @@ -59,7 +59,7 @@ - Observable completion: new/updated cases FAIL (red) against unwired production code - _Requirements: 1.1, 1.2, 1.3, 2.1, 4.2, 4.4, 6.3_ -- [ ] 5.2 Implement the repl-side wiring: ACK feed, membership hooks, adapter +- [x] 5.2 Implement the repl-side wiring: ACK feed, membership hooks, adapter - repl.h: forward-declare `struct commit_engine`, add `ce` pointer to `replicator_t`, declare `repl_set_commit_engine()`; add `uint64_t entry_lsn` parameter to `repl_wait_majority_ack` (update the config_repl.c:916 caller to pass 0 — configAck branch behavior untouched) - repl.c: `repl_on_ack` calls `commit_on_ack(r->ce, slot_idx, acked_lsn)` after the `acked_lsn` store when wired; `repl_add_standby`/`repl_remove_standby`/`repl_start_catchup`/`repl_complete_catchup` call `commit_on_membership_change` when wired; walAck branch of `repl_wait_majority_ack` delegates to `commit_submit(ce, entry_lsn, COMMIT_MODE_WAIT, timeout_ms)` when wired, with the legacy counting kept only for `ce == NULL` test contexts and marked deprecated (retired by ha-commit-barrier-rollout) - Observable completion: ACK-feed and membership-hook tests from 5.1 PASS; configAck tests unaffected @@ -92,3 +92,7 @@ - (5.1) persist.c currently passes entry_lsn=0 to the adapter — if 5.2 lands walAck delegation before 5.3 passes written_lsn, the wired sync path returns -EINVAL. Land 5.2's delegation gated on ce!=NULL AND entry_lsn!=0 falling back to legacy, OR land 5.2+5.3 wiring in lockstep. - (5.1) Baseline failure root causes identified: test_committed_lsn_does_not_advance_on_timeout (test_repl_unit.c:294) expects the OLD contract (rc==0 + sync_rc out-param) but persist_wal_sync_append now rejects-on-timeout — STALE TEST EXPECTATION, fix the test at 6.1, not the code. test_p2_integration.c:test_single_node_committed_lsn_unchanged pins the -ENODEV degraded path that 5.3 replaces — update it at 5.3/6.1. - (5.1) Repo hygiene (pre-existing, out of scope): untracked src/lib/testsuite/test_wal_wire.c is referenced by committed src/lib/meson.build:176 — should be added to git eventually. +- (5.2) commit.c linked into production controller_src ONE TASK EARLY (meson.build:3) — necessary (repl.c references engine symbols) and reviewer-verified inert (zero production callers of commit_engine_init/repl_set_commit_engine until 5.3; repl_init memsets ce=NULL → all new paths dormant). +- (5.2) repl_broadcast's over-limit eviction (in_catch_up=1, repl.c:268-277) deliberately does NOT feed commit_on_membership_change — reviewer-proven no-op for waiter correctness (shrink can never raise the majority candidate; monotonic store). Design lists exactly four mutators; eviction recovery fires the hook at repl_complete_catchup. +- (5.2) r->ce is read unlocked under a set-once-before-traffic contract (repl.h:47-52). 5.3 must wire it at cluster init BEFORE replication traffic; never NULL it concurrent with traffic (fini ordering is 5.3's responsibility). +- (5.2) Remaining RED going into 5.3: test 33 (Expected -110 Was 0 — persist passes entry_lsn=0 → legacy fallback) and test 34 (Expected 0 Was -19 — -ENODEV special case). These plus test 32's honest-delegation green are 5.3's completion evidence. diff --git a/.wolf/cerebrum.md b/.wolf/cerebrum.md index 11b7f32..3f52cd8 100644 --- a/.wolf/cerebrum.md +++ b/.wolf/cerebrum.md @@ -9,6 +9,7 @@ ## Key Learnings +- 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. - Use `clang-format` for C/C++ formatting; shared config lives at `xce/.clang-format`. - Logging macros in utils.h: TRACE (= tlog_debug3, verbose), INFO, WARN. There is NO DEBUG3 macro — use TRACE for debug-level logging in repl.c and other controller files. (2026-06-10) diff --git a/.wolf/memory.md b/.wolf/memory.md index be1124c..e84bb27 100644 --- a/.wolf/memory.md +++ b/.wolf/memory.md @@ -927,3 +927,16 @@ | 5.1-RED-done | RED captured: 9 new failures (4 commit_engine #30/31/33/34, 5 repl_majority walAck) + 3 baselines exact; build green; test 32 pins positive contract (green in RED by design) | census /tmp/census_final.log | ok | ~90k | | 16:08 | Edited .kiro/specs/ha-commit-engine/tasks.md | inline fix | ~22 | | 16:08 | Edited .kiro/specs/ha-commit-engine/tasks.md | modified hygiene() | ~517 | +| 16:14 | Edited src/include/repl.h | modified struct() | ~202 | +| 16:14 | Edited src/controller/repl.c | 3→4 lines | ~52 | +| 16:15 | Edited src/controller/repl.c | added 1 condition(s) | ~298 | +| 16:15 | Edited src/controller/repl.c | added 1 condition(s) | ~115 | +| 16:15 | Edited src/controller/repl.c | added 1 condition(s) | ~211 | +| 16:15 | Edited src/controller/repl.c | added 1 condition(s) | ~208 | +| 16:16 | Edited src/controller/repl.c | added 1 condition(s) | ~241 | +| 16:16 | Edited src/controller/repl.c | added 1 condition(s) | ~368 | +| 16:16 | Edited src/controller/repl.c | added 1 condition(s) | ~136 | +| 16:16 | Edited src/controller/meson.build | 3→4 lines | ~18 | +| 23:35 | ha-commit-engine 5.2: wired ce into replicator_t, ACK feed (repl_on_ack→commit_on_ack), 4 membership hooks, walAck delegation to commit_submit (gated ce!=NULL && entry_lsn!=0); added commit.c to controller_src for prod link | src/include/repl.h, src/controller/repl.c, src/controller/meson.build | tests 30-32 + 5 repl_majority walAck GREEN; 33/34 stay RED until 5.3; 3 baselines unchanged | ~45k | +| 16:32 | Edited .kiro/specs/ha-commit-engine/tasks.md | inline fix | ~21 | +| 16:32 | Edited .kiro/specs/ha-commit-engine/tasks.md | added error handling | ~345 | diff --git a/src/controller/meson.build b/src/controller/meson.build index 3778b31..6dc2b69 100644 --- a/src/controller/meson.build +++ b/src/controller/meson.build @@ -1,5 +1,6 @@ controller_src = [ 'cluster.c', + 'commit.c', 'config_repl.c', 'dataex.c', 'ha.c', diff --git a/src/controller/repl.c b/src/controller/repl.c index 89b3bc9..b72ca86 100644 --- a/src/controller/repl.c +++ b/src/controller/repl.c @@ -10,6 +10,7 @@ #include "controller.capnp.h" /* OPCode_walAck, OPCode_upgradeConfigAck */ #include "repl.h" +#include "commit.h" /* commit engine feed + adapter (ha-commit-engine 5.2) */ #include "mylock.h" #include "utils.h" #include "request.h" /* xce_wal_batch_req_raw */ @@ -120,11 +121,21 @@ int repl_add_standby(replicator_t *r, repl_ep_t ep, const char *host, uint32_t p r->nstandbys++; mylock_unlock(&r->lock); + + /* Quorum membership changed: re-evaluate pending commits. Called AFTER + * releasing r->lock — engine lock order is ce->lock then r->lock + * (ha-commit-engine task 5.2, Req 4.2). The idempotent early-return + * above mutates nothing, so it deliberately skips this hook. */ + if (r->ce != NULL) { + commit_on_membership_change(r->ce); + } return idx; } void repl_remove_standby(replicator_t *r, repl_ep_t ep) { + int removed = 0; + if (r == NULL || ep == NULL) { return; } @@ -138,11 +149,18 @@ void repl_remove_standby(replicator_t *r, repl_ep_t ep) s->last_disconnect_time = time(NULL); s->ep = NULL; r->nstandbys--; + removed = 1; break; } } mylock_unlock(&r->lock); + + /* Membership shrink: wake pending waiters to re-check their predicates + * (after r->lock release — engine lock order; ha-commit-engine Req 4.2). */ + if (removed && r->ce != NULL) { + commit_on_membership_change(r->ce); + } } void repl_expire_disconnect_slots(replicator_t *r, time_t now_s, uint32_t snap_interval_s) @@ -175,6 +193,8 @@ void repl_expire_disconnect_slots(replicator_t *r, time_t now_s, uint32_t snap_i void repl_on_ack(replicator_t *r, repl_ep_t ep, uint64_t acked_lsn) { + int slot_idx = -1; + if (r == NULL || ep == NULL) { return; } @@ -187,12 +207,21 @@ void repl_on_ack(replicator_t *r, repl_ep_t ep, uint64_t acked_lsn) atomic_store(&s->acked_lsn, acked_lsn); if (s->pending_ack_count > 0) s->pending_ack_count--; + slot_idx = i; break; } } mylock_unlock(&r->lock); + /* Feed the commit engine AFTER the acked_lsn store and AFTER releasing + * r->lock — commit_on_ack takes ce->lock then r->lock (majority scan), + * so calling it under r->lock would invert the engine's lock order. + * Never blocks (iomgr-safe; ha-commit-engine task 5.2, design §ACK Feed). */ + if (r->ce != NULL && slot_idx >= 0) { + commit_on_ack(r->ce, slot_idx, acked_lsn); + } + /* Signal any waiter (repl_wait_ack / repl_wait_majority_ack) */ pthread_mutex_lock(&r->ack_lock); pthread_cond_signal(&r->ack_cond); @@ -386,17 +415,31 @@ void repl_on_config_ack(replicator_t *r, repl_ep_t ep, uint64_t acked_config_lsn int repl_wait_majority_ack(replicator_t *r, uint64_t entry_lsn, uint32_t timeout_ms, uint8_t ack_op) { - /* entry_lsn is accepted but unused by this legacy counting body - * (ha-commit-engine task 5.1 signature change): task 5.2 delegates the - * walAck branch to commit_submit(ce, entry_lsn, COMMIT_MODE_WAIT, ...) - * when the engine is wired. Behavior here stays byte-identical. */ - (void)entry_lsn; - if (r == NULL) return -EINVAL; int is_wal_ack = (ack_op == (uint8_t)OPCode_walAck); + /* 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) { + 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. */ + struct timespec deadline; clock_gettime(CLOCK_REALTIME, &deadline); deadline.tv_sec += (long)(timeout_ms / 1000); @@ -513,30 +556,50 @@ uint64_t repl_trim_lsn(replicator_t *r, uint64_t checkpoint_lsn, void repl_start_catchup(replicator_t *r, repl_ep_t ep, uint64_t join_applied_lsn) { + int changed = 0; + mylock_lock(&r->lock); for (int i = 0; i < MAX_STANDBYS; i++) { if (r->standbys[i].ep == ep) { r->standbys[i].join_applied_lsn = join_applied_lsn; atomic_store_explicit(&r->standbys[i].in_catch_up, 1, memory_order_release); + changed = 1; break; } } mylock_unlock(&r->lock); + + /* Catch-up entry excludes the standby from the evaluation count: + * re-evaluate pending commits (after r->lock release — engine lock + * order; ha-commit-engine task 5.2, Req 4.2). */ + if (changed && r->ce != NULL) { + commit_on_membership_change(r->ce); + } } void repl_complete_catchup(replicator_t *r, repl_ep_t ep) { + int changed = 0; + mylock_lock(&r->lock); for (int i = 0; i < MAX_STANDBYS; i++) { if (r->standbys[i].ep == ep) { atomic_store_explicit(&r->standbys[i].in_catch_up, 0, memory_order_release); + changed = 1; break; } } mylock_unlock(&r->lock); + /* Standby rejoins the evaluation count: its watermark may newly + * satisfy a pending majority — re-evaluate (after r->lock release — + * engine lock order; ha-commit-engine task 5.2, Req 4.2). */ + if (changed && r->ce != NULL) { + commit_on_membership_change(r->ce); + } + /* Broadcast so any blocked repl_wait_ack() re-checks the ACK condition */ pthread_mutex_lock(&r->ack_lock); pthread_cond_broadcast(&r->ack_cond); @@ -563,16 +626,17 @@ int repl_needs_snapshot(replicator_t *r, uint64_t standby_lsn, * (ha.c) for write admission. Takes ONLY r->lock — never the commit-engine * mutex (lock-ordering rule: gate path holds no locks when calling in). * (ha-commit-engine Req 3.1, 3.2, 3.3) */ -/* repl_set_commit_engine — INERT stub (ha-commit-engine task 5.1). - * Stores nothing, so the engine is never wired and the task-5.1 RED tests - * fail on behavior (ACK and membership events never reach the engine), - * not on linking. Task 5.2 adds the replicator_t.ce field, stores the - * pointer here, and feeds commit_on_ack / commit_on_membership_change - * from repl_on_ack and the membership mutators. */ +/* repl_set_commit_engine — wire the commit engine into the replicator + * (ha-commit-engine task 5.2). Once set, repl_on_ack feeds commit_on_ack, + * the membership mutators feed commit_on_membership_change, and the walAck + * branch of repl_wait_majority_ack delegates to commit_submit. Called once + * at cluster init before replication traffic (task 5.3); NULL un-wires. */ void repl_set_commit_engine(replicator_t *r, struct commit_engine *ce) { - (void)r; - (void)ce; + if (r == NULL) { + return; + } + r->ce = ce; } int repl_active_standby_count(replicator_t *r) diff --git a/src/include/repl.h b/src/include/repl.h index 1cf10ca..4c6abeb 100644 --- a/src/include/repl.h +++ b/src/include/repl.h @@ -44,6 +44,12 @@ typedef struct replicator_s { mylock_t lock; /* Protects standbys[], nstandbys */ pthread_cond_t ack_cond; /* Signaled when any ACK arrives */ pthread_mutex_t ack_lock; /* Guards ack_cond */ + /* Commit engine feed (ha-commit-engine task 5.2). NULL = not wired + * (legacy counting contexts). Set once via repl_set_commit_engine at + * cluster init, before replication traffic — read without r->lock. + * Lock ordering: engine paths take ce->lock then r->lock, so repl.c + * must NEVER call into the engine while holding r->lock. */ + struct commit_engine *ce; } replicator_t; /* Lifecycle */