commit_on_ack: active-standby (non-catch-up, connected) watermark scan, descending sort, candidate arr[n_peers/2-1], clamped to appended log, monotonic single-site committed_lsn store, condvar broadcast, never blocks beyond engine mutex. RETAIN returns immediately. Self-quorum for n_peers<=1; zero-fill makes zero-standby commit impossible. 7/7 task-1.1 tests green unmodified; full suite at known 3-failure baseline.
12 KiB
Implementation Plan: ha-commit-engine
TDD ordering per project steering (
testing.md): every implementation sub-task is preceded by a testsuite sub-task that lands failing (RED) tests first. All new tests live intestsuite/test_commit_engine.c(brief-scoped single test file) plus targeted updates totest_repl_majority.c— this shared-file reality is why no(P)markers are applied: the test file andrepl.care touched by multiple groups, so the plan is sequential. Verify exact line numbers with grep before editing (research.md table has the verified anchors; files drift).
-
1. Commit engine core: per-LSN evaluation and commit-index integrity
-
1.1 Write RED tests for per-LSN majority evaluation and commit-index integrity
- Create
testsuite/test_commit_engine.cagainst a stubpersist_t+replicator_t(XCE_TESTING pattern, AAA structure) and register it incontroller_unit_test_src(meson.build); createsrc/controller/include/commit.hwith the umbrella-verbatim interface (types + prototypes only) so tests compile - Tests: with n_peers=3, a standby ACK for LSN M does not commit entry N>M (kills
acked_lsn > 0counting); ACK watermark >= N from one standby plus self commits N; cumulative/out-of-order watermarks advancecommitted_lsnto the majority median, never past the highest appended LSN, never backwards; one far-lagging standby does not block majority commit; RETAIN mode returns 0 immediately without blocking - Observable completion: build succeeds with the new header and test file; all new test cases FAIL (red) against a stub
commit.cor unlinked implementation - Requirements: 1.2, 2.1, 2.2, 2.3, 5.1
- Create
-
1.2 Implement the commit engine core to green
- Create
src/controller/commit.c(add to meson controller sources):commit_engine_init/commit_engine_fini(validate args, one mutex + one condvar),commit_on_ack(bounded scan of active standbys'acked_lsninto the scratch array, sort descending, candidate =arr[n_peers/2 - 1]clamped to appended log, monotonicmax()store intopersist->committed_lsn, condvar broadcast — never blocks),commit_submit(RETAIN)immediate return - Catch-up standbys (
in_catch_up) excluded from the evaluation count (umbrella-locked conservative rule) - XCE_TESTING asserts for monotonicity
- Observable completion: all task 1.1 tests PASS; engine is the only
committed_lsnwriter incommit.cpaths - Requirements: 1.2, 2.1, 2.2, 2.3, 5.1
- Create
-
2. WAIT mode, self-quorum, zero-peer rule, admission cap, own-term gate storage
-
2.1 Write RED tests for blocking-wait semantics and admission control
- Tests in
test_commit_engine.c: WAIT blocks until majority ACK then returns 0; WAIT returns-ETIMEDOUTat deadline withcommitted_lsnunmoved past the entry; n_peers=1 → WAIT returns 0 immediately andcommitted_lsnadvances to the entry; n_peers=3 with zero standby ACKs →-ETIMEDOUT, never success (short-circuit regression); with n_vcpus=8 and reserved=2, a 7th concurrent WAIT gets-XCE_ERR_NOT_READYwhile the 6 admitted waits proceed; cap floormax(1, n_vcpus - reserved)holds for tiny n_vcpus; while the cap is saturated,commit_on_ackstill advancescommitted_lsnand completes admitted waiters;writable_lsnunset (0) → no gating;commit_set_writable_lsn(G)with G > entry → waiter completes only oncecommitted_lsn >= G - Observable completion: new cases FAIL (red) against the task-1 engine
- Requirements: 1.1, 4.3, 4.4, 6.3, 7.1, 7.2, 7.3
- Tests in
-
2.2 Implement WAIT mode, admission counter, and writable_lsn gate to green
commit_submit(WAIT): admission check (inflight_waits >= max(1, n_vcpus - COMMIT_WAIT_RESERVED)→-XCE_ERR_NOT_READY, distinct log message),pthread_cond_timedwaitloop with per-LSN predicate (committed_lsn >= lsn && (writable_lsn == 0 || committed_lsn >= writable_lsn)),-ETIMEDOUTon deadline;n_peers <= 1immediate self-quorum success advancingcommitted_lsn;commit_set_writable_lsnstores the gate (no caller in this spec)- XCE_TESTING assert: inflight counter within cap and never negative
- Observable completion: all task 2.1 tests PASS; cap is expressed only relative to the
n_vcpusinit argument (no absolute constant other than the reserved headroom) - Requirements: 1.1, 4.3, 4.4, 6.3, 7.1, 7.2, 7.3
-
3. Membership-change re-evaluation
-
3.1 Write RED tests for membership re-evaluation
- Tests in
test_commit_engine.c: a catch-up standby's ACK watermark does not count toward a pending entry; after clearingin_catch_upand callingcommit_on_membership_change, the pending waiter completes if the standby's watermark covers it; on standby removal that makes majority unreachable, a pending WAIT never returns success and fails with-ETIMEDOUTat its deadline;commit_on_membership_changereturns without blocking - Observable completion: new cases FAIL (red)
- Requirements: 4.1, 4.2
- Tests in
-
3.2 Implement commit_on_membership_change to green
- Re-run the commit evaluation under the engine mutex against current registry state and broadcast the condvar so all pending waiters re-check their predicates; no forced early-fail (reconnection within the deadline may still commit — design decision)
- Observable completion: all task 3.1 tests PASS
- Requirements: 4.1, 4.2
-
4. Quorum-accounting fix: catch-up exclusion at the dispatch gate
-
4.1 Write RED tests for write-admission quorum counting
- Tests:
repl_active_standby_countcounts only slots withep != NULL && !in_catch_up;ha_has_write_quorumwith n_peers=3 returns false when the only connected standby is in catch-up, true with one active standby, false with zero connected; after simulated reconnect (registry restored) the gate accepts again with no reset call (stateless re-evaluation); n_peers=1 always true - Declare
repl_active_standby_countin src/include/repl.h so tests compile - Observable completion: new cases FAIL (red) — current
ha_has_write_quorumcountsstandby_endpoints->countincluding catch-up (grep ha.c for the current anchor near ha.c:1507) - Requirements: 3.1, 3.2, 3.3, 6.1, 6.2
- Tests:
-
4.2 Implement the quorum-accounting fix to green
- Add
repl_active_standby_count(replicator_t *r)to repl.c (underr->lock); addreplicator_t *replback-pointer toha_state_t(ha.h);ha_has_write_quorumuses the helper when the back-pointer is wired, falls back to the existingstandby_endpointscount when NULL (legacy test contexts); quorum condition itself unchanged (active >= n_peers / 2,n_peers <= 1 → true) - Observable completion: all task 4.1 tests PASS; existing dispatch-gate tests still green
- Requirements: 3.1, 3.2, 3.3, 6.1, 6.2
- Add
-
5. Integration: wire the engine behind the job-submit sync path
-
5.1 Write RED integration tests for the ACK feed and the sync-append path
- Tests in
test_commit_engine.c:repl_on_ackwith a wired engine updatesacked_lsnAND triggers commit evaluation (majority →committed_lsnadvances, waiter wakes);repl_start_catchup/repl_complete_catchup/repl_remove_standby/repl_add_standbytrigger re-evaluation;persist_wal_sync_appendwith wired engine returns 0 only on per-entry majority, returns-ETIMEDOUTwhen standbys'acked_lsncovers only an EARLIER entry (stale-ACK regression — connectivity/old ACKs never substitute), leavescommitted_lsnunmoved on timeout, and for n_peers=1 returns 0 withcommitted_lsnadvanced (replaces the old-ENODEVdegraded path) - Update
testsuite/test_repl_majority.cwalAck cases to wire an engine and assert per-LSN semantics; configAck cases byte-identical - 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
- Tests in
-
5.2 Implement the repl-side wiring: ACK feed, membership hooks, adapter
- repl.h: forward-declare
struct commit_engine, addcepointer toreplicator_t, declarerepl_set_commit_engine(); adduint64_t entry_lsnparameter torepl_wait_majority_ack(update the config_repl.c:916 caller to pass 0 — configAck branch behavior untouched) - repl.c:
repl_on_ackcallscommit_on_ack(r->ce, slot_idx, acked_lsn)after theacked_lsnstore when wired;repl_add_standby/repl_remove_standby/repl_start_catchup/repl_complete_catchupcallcommit_on_membership_changewhen wired; walAck branch ofrepl_wait_majority_ackdelegates tocommit_submit(ce, entry_lsn, COMMIT_MODE_WAIT, timeout_ms)when wired, with the legacy counting kept only force == NULLtest 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
- Depends: 2.2, 3.2
- Requirements: 1.1, 4.2
- repl.h: forward-declare
-
5.3 Implement engine instantiation and the sync-append path switch
- cluster.h/cluster.c: embed
commit_engine_t commitincluster_t; callcommit_engine_init(&c->commit, &c->persist, n_peers, n_vcpus)+repl_set_commit_engineat the init point where both the vtask pool size (cluster.c:1082 anchor) andha->n_peersare final; set theha_state_t.replback-pointer here;commit_engine_finion teardown - persist.c
persist_wal_sync_append: passwritten_lsnto the adapter; on the engine-wired path remove thehas_standbys → -ENODEVspecial case and thecommitted_lsnstore (persist.c:1035 anchor — the engine now owns advancement; truncation store at persist.c:1438 stays); return contract to callers: 0 /-ETIMEDOUT/-XCE_ERR_NOT_READYpropagated through the existing job-submit error path - Observable completion: all task 5.1 tests PASS, including stale-ACK regression and single-node
committed_lsnadvancement; job submit on a wired cluster confirms only on per-entry majority - Requirements: 1.1, 1.2, 1.3, 2.1, 4.4, 6.3
- cluster.h/cluster.c: embed
-
6. Validation: full regression and dishonest-path audit
-
6.1 Run the full controller regression and audit the legacy paths
- Build and run the complete controller testsuite with ASAN (
/xce-build controller); fix any regression introduced by the short-circuit removal or thecommitted_lsnownership move (existing persist/repl/ha suites) - Audit: grep confirms no production (engine-wired) walAck path reaches the
acked_lsn > 0counting or then_active == 0 → successbranch; both survive only under the deprecatedce == NULLtest fallback - Observable completion: full testsuite green under ASAN; audit results recorded in the task completion notes
- Requirements: 1.1, 1.2, 1.3, 2.1, 2.2, 2.3, 3.1, 3.2, 3.3, 4.1, 4.2, 4.3, 4.4, 5.1, 6.1, 6.2, 6.3, 7.1, 7.2, 7.3
- Build and run the complete controller testsuite with ASAN (
Implementation Notes
- (1.1) THREE PRE-EXISTING suite failures, masked until now (bug-411): test_committed_lsn_does_not_advance_on_timeout (-110), test_repl_ack_after_promote_gate_allows_jobsub (-10013), test_rdhup_primary_dispatch_gate_opens (-10013). Masking cause: each test group runs its own UNITY_BEGIN/END so the process exit code reflects only the LAST group. They live in repl/persist sync-append and quorum-gate paths — tasks 1.2/2.x/5.x touch those areas and 6.1 requires full green, so they must be fixed (or proven fixed by the engine) along the way.
- (1.1) Full-suite output under default ASAN dies at exit via LeakSanitizer (pre-existing leaks in worker_join/rdhup_integration/start_election_async/liveness groups) without flushing stdout — use ASAN_OPTIONS=detect_leaks=0 for full-run logs or run groups standalone with -t.
- (1.1) repl_add_standby returns the SLOT INDEX, not 0 (bug-410 note for fixture writers).
- (1.1) commit.c is registered ONLY in controller_unit_test_src for now; production target linkage lands with task 5.3.