myxce/.kiro/specs/ha-commit-engine/research.md
Rongsong Shen 20e59d0b42 checkpoint: bug-851 root cause + raft TLA+ conformance spec family
Investigation & fixes (failover committed-job loss):
- root-cause bug-851 on an owned 3-node cluster: chaotic failover causes
  silent committed-prefix divergence (WAL hdr.term always 0; two leaders
  per term; no (term,lsn) reconcile). Evidence in repro-851/ + INVESTIGATION.
- live-validated fixes 848/849/850 (recovery-on-promotion, advertise real
  applied_lsn, persist-layer contiguity guard); add waldump WAL inspector.

Specs (strict raft.tla conformance — library owns the log):
- TLA-gap-analysis.md: impl vs raft/raft.tla, gaps G1-G6.
- raft-log-replication, raft-commit-safety, raft-sim-conformance: extend
  src/lib/raft to own log/commit/verification via callbacks (specs generated,
  cross-spec review passed).
- ha-term-safety: controller-side adapter + config marker (R1-R7); design done.
- config-upgrade-snapshot: R8 descoped here (deps entity-bearing-snapshots, bug-857).
- roadmap + buglog (851/854/857) + .wolf knowledge updated.
2026-06-21 14:02:15 +00:00

13 KiB
Raw Blame History

Research Log: ha-commit-engine

Date: 2026-06-11. Discovery status: complete via umbrella — this sub-spec inherits the ha-consistency-invariants discovery (.kiro/specs/ha-consistency-invariants/research.md: gap analysis, Option C selection, deep-dives §A–§C, locked decisions). No decisions are re-derived here. This log records only the slice-scoped code verification performed for this spec's design.

Inherited Locked Decisions (umbrella — do not re-litigate)

  • Commit engine as single authority; WAIT/RETAIN completion modes (research synthesis)
  • Admission cap relative to n_vcpus (never absolute); COMMIT_WAIT_RESERVED default 2 (research item 3(d–e))
  • Blocking condvar on vtask threads accepted; async continuation deferred; no liveness risk (ACKs on iomgr) (research item 3(b))
  • Catch-up standbys excluded from quorum accounting and commit counting until repl_complete_catchup (umbrella §Quorum Accounting + commit_on_membership_change contract)
  • Commit state volatile (no persisted commitIndex); convergence/rejoin owned by ha-convergence-recovery (research §C)
  • Own-term gate via WAL_NOOP at promotion — API (commit_set_writable_lsn) ships in this spec, consumers in ha-single-leader-role (research item 4)

Slice Verification (2026-06-11, this spec)

Fact Evidence Implication
standby_conn_t.acked_lsn atomic exists, updated by repl_on_ack src/include/repl.h:24; repl.c:176, store at repl.c:187 matchIndex bookkeeping ready; engine consumes read-only
Dishonest counting: acked_lsn > 0 repl.c:419 replaced by per-LSN engine evaluation
n_active == 0 → success short-circuit repl.c:432-435 removed for n_peers>1 via engine (engine knows n_peers; repl does not)
Quorum denominator is n_active, not n_peers repl.c:437 engine uses configured n_peers per umbrella commit rule
ha_has_write_quorum counts standby_endpoints->count (includes catch-up) ha.c:1507-1526; gate call at ha.c:1537 fix via new repl_active_standby_count (ep && !in_catch_up); in_catch_up at repl.h:26
Sync barrier: persist_wal_sync_append persist.c:1003-1049; committed_lsn store at 1035; has_standbys → -ENODEV degraded-success at ~1027-1031; truncation store at persist.c:1438 adapter passes written_lsn; 1035 store moves into engine; -ENODEV case replaced by engine decision; 1438 store stays
ACKs processed on iomgr peer_ctrl_msg_handler → repl_ack_decode_and_dispatch (ha.c:2632) → repl_on_ack call at ha.c:2748 commit_on_ack feed point; must never block
Membership mutation points repl.h:50-51 (repl_add_standby/repl_remove_standby), repl.h:76/79 (repl_start_catchup/repl_complete_catchup); removal call sites ha.c:3176, 3866 commit_on_membership_change call sites
vtask pool default 8, --vcpus override cluster.c:1076-1083 n_vcpus input to commit_engine_init; init point after vtaskmgr sizing
replicator_t embedded in persist_t (p->repl); cluster reaches via c->persist.repl persist.c:1032; ha.c:538, 2748 engine reaches standby registry via its persist_t*; commit_engine_init(ce, p, ...) signature suffices
ha_state_t has n_peers and raft_c, but no repl pointer ha.h:180-185 add explicit replicator_t *repl back-pointer (raft_c only set by election path — not reliable for the gate)
repl.h/persist.h live in src/include (lib-visible); controller-local headers in src/controller/include find results; cluster.h, worker.h commit.h goes to src/controller/include; repl.h uses forward-declared struct commit_engine * to avoid dependency inversion
repl_wait_majority_ack callers: persist.c:1032 (walAck), config_repl.c:916 (configAck) grep signature gains entry_lsn; configAck branch untouched (out of boundary)
Existing tests encode old semantics testsuite/test_repl_majority.c (walAck cases at :85-:177); meson controller_unit_test_src list at meson.build:74-110 walAck cases must be updated; new test file registered in meson
XCE_ERR_NOT_READY defined ha.c:35 (= E_XCE_NOT_READY) reuse for admission-cap rejection

Design Synthesis Outcomes (this slice)

  • Adapter placement (brief-mandated): repl_wait_majority_ack stays as the shell; gains entry_lsn param because the replicator has no LSN knowledge; engine-less fallback retained ONLY for unit-test contexts and explicitly retired by ha-commit-barrier-rollout. Production walAck always goes through the engine.
  • Engine reachability: forward-declared struct commit_engine *ce on replicator_t (keeps src/include/repl.h free of controller headers); engine instance embedded in cluster_t; init after vtask sizing + peer count known.
  • Standalone behavior change (intentional): with the engine wired, n_peers <= 1 sync appends now advance committed_lsn (previously the -ENODEV path left it unmoved). Raft-correct (self-quorum), satisfies spec req 6.3; flagged in design.
  • Membership-shrink semantics: re-evaluation = wake + re-check; no forced early-fail (umbrella 4.3 requires fail-not-confirm, satisfied by timeout; reconnection within the deadline may still legitimately commit).
  • Review gate: design verified against the umbrella contract — interface verbatim, no contradiction with locked decisions, all 20 slice requirement IDs traced.

Gap Re-Audit (2026-06-18)

Type: Post-implementation gap audit (not a pre-impl gap analysis). Roadmap declares ha-commit-engine IMPLEMENTED 12/12 (2026-06-12, validate-impl GO, 467 tests green). tasks.md shows 6 [ ] checkboxes; all 6 are section/group headers ("1.", "2.", "3.", "4.", "5.", "6."), not leaf tasks — every leaf sub-task (1.1, 1.2, 2.1, 2.2, 3.1, 3.2, 4.1, 4.2, 5.1, 5.2, 5.3, 6.1) is [x]. The "12/12" count is the leaf count. The unchecked group headers are an artifact of the checklist format, documented as such in §Implementation Notes — they are not deferred work and not core gaps.

Verdict

STATUS: CLOSED. All 7 requirements (20 EARS acceptance criteria) are implemented and wired in production. Verified against current on-disk source (codegraph verbatim reads + grep, 2026-06-18). The engine has since been extended by ha-commit-durability (durable self-cap) — that extension strengthens, and does not regress, the ha-commit-engine contract.

Requirement-to-Asset Map (verified, file:line)

Req Criterion Asset (verified) Status
1.1 confirm only after per-entry majority commit_submit(WAIT) predicate wait_done_locked commit.c:184-189; sync path persist.c:1897-1900 → repl_wait_majority_ack walAck delegation repl.c:447 → commit_submit MET
1.2 count ACK toward N only if acked_lsn >= N majority_candidate_locked reads s->acked_lsn, sorts desc, candidate = scratch[n_peers/2-1] commit.c:107-134 MET
1.3 connectivity never substitutes for ACKs ACK-time eval only; watermarks read from registry at commit_on_ack commit.c:366-381; gate (ha_has_write_quorum) is fast-reject only, engine re-verifies MET
2.1 no commit advance without majority commit_advance_locked stores only majority_candidate_locked result commit.c:145-168 MET
2.2 monotonic committed_lsn if (cand <= cur) return; + atomic_store(max) commit.c:150-165; XCE_TESTING monotonic assert MET
2.3 contiguous, bounded by appended log candidate clamped to cap (durable, ≤ highest_appended_lsn) commit.c:136-137, 52-55; cumulative watermarks → contiguous MET (strengthened: cap is now durable_lsn per ha-commit-durability)
3.1/3.2/3.3 admission quorum counts only members able to receive; catch-up + disconnected excluded repl_active_standby_count (ep != NULL && !in_catch_up) repl.c:668-686; consumed by ha_has_write_quorum ha.c:2049-2058 MET
4.1 post-acceptance shrink → fail not confirm WAIT times out (-ETIMEDOUT) when majority never reached; no false-success path; commit_on_membership_change re-evals monotonically (shrink never regresses) commit.c:397-409 MET
4.2 membership change re-evaluates pending waits commit_on_membership_change wired at 4 mutators: repl.c:130 (add), :162 (remove), :598 (start_catchup), :621 (complete_catchup) MET
4.3/4.4 zero peer ACKs never majority (n_peers>1); no zero-active short-circuit engine derives from n_peers not n_active; k = n_peers/2-1, absent peers zero-filled commit.c:117-134; legacy n_active==0→success short-circuit removed (sync path persist.c:1897 engine branch; configAck has explicit "no n_active shortcut" repl.c:506-510) MET
5.1 lag never blocks majority majority is the only threshold; far-lagging standby sorts low, does not affect scratch[n_peers/2-1] commit.c:129-134 MET
6.1 reject without quorum ha_has_write_quorum false → ha_dispatch_gate -XCE_ERR_NOT_READY ha.c:2031-2059 MET
6.2 auto-resume on quorum restore gate is stateless per request (recomputes repl_active_standby_count each call); no reset needed MET
6.3 standalone self-quorum n_peers <= 1 → commit_advance_locked immediate, returns cap commit.c:100-102, 330-332; sync path -ENODEV special case removed on engine branch MET
7.1/7.2 n_vcpus-relative admission cap, NOT_READY beyond wait_cap = max(1, n_vcpus - COMMIT_WAIT_RESERVED) commit.c:175-180; inflight_waits >= cap → -XCE_ERR_NOT_READY commit.c:313-320 MET
7.3 ACK processing unaffected at cap commit_on_ack takes only ce->lock, never the inflight gate; advances + broadcasts regardless of cap commit.c:366-381 MET

The 6 Open [ ] Tasks — Classified

All 6 are group-header checkboxes whose every child is [x]. None is deferred work; none is a core gap. They are a checklist-rendering artifact (parent headers left unchecked while leaves complete).

Header Children Classification Effort/Risk
1. Commit engine core 1.1 [x], 1.2 [x] DONE (artifact) none
2. WAIT/self-quorum/zero-peer/cap/gate 2.1 [x], 2.2 [x] DONE (artifact) none
3. Membership re-evaluation 3.1 [x], 3.2 [x] DONE (artifact; jointly completed — RED unachievable, §Notes) none
4. Quorum-accounting fix 4.1 [x], 4.2 [x] DONE (artifact) none
5. Integration (wire engine behind job submit) 5.1 [x], 5.2 [x], 5.3 [x] DONE (artifact) none
6. Validation / dishonest-path audit 6.1 [x] DONE (artifact; 6.1 audit record in §Notes) none

Bottom line: 0 deferred follow-ups remain as core gaps. The "deferred follow-ups in §Implementation Notes" referenced by the roadmap are advisory/out-of-scope items, not open tasks — see the genuine residuals below; all are explicitly out-of-spec-boundary and owned elsewhere.

Genuine Residuals (advisory, all out-of-boundary or already resolved downstream)

These are the §Implementation Notes "deferred follow-ups". None blocks CLOSED status; each is owned by a later spec or is cosmetic.

  1. LSN-0 first-entry legacy edge (Notes 5.1, 6.1 audit). The first-ever WAL entry (LSN 0) routes to the legacy branch (persist.c:1903-1929) because commit_submit rejects lsn==0. — RESOLVED downstream: ha-single-leader-role's promotion no-op entry makes LSN 0 never a client write in practice; repl.c:438-440 now confirms LSN 0 immediately. Once-per-cluster-lifetime, no consistency loss. Effort: 0 (closed). Risk: none.
  2. ce->n_peers not re-derived on dynamic membership change (Notes 6.1(b)). If ha->n_peers is reassigned post-init (e.g. ha.c RDHUP re-election), the engine's n_peers is stale. — Out of boundary (dynamic membership = future work); current cluster topology is fixed at init. Option A: assert n_peers stability at re-election callback; Option B: re-derive ce->n_peers in a commit_set_n_peers setter called from the election path; Option C: leave as documented latent gap. Effort: A ~0.5d, B ~1d (+revalidate barrier-rollout/durability). Risk: LOW today (no dynamic membership in production); MEDIUM if dynamic reconfiguration is ever added without addressing it.
  3. Pre-existing ASAN leaks in 6 test groups (Notes 6.1 LEAK FOLLOW-UP: worker_join 33KB, rdhup_integration, failover, start_election_async, liveness, connection_reuse). — Out of scope (pre-existing, production-side allocations; suite runs with detect_leaks=0). Option: dedicated leak-cleanup pass. Effort: ~1-2d. Risk: LOW (test-harness hygiene, not a correctness gap).
  4. Stale comment persist.c:1151-1154 / pre-engine contract text (Notes 6.1). One-line doc follow-up. Effort: trivial. Risk: none.
  5. Untracked src/lib/testsuite/test_wal_wire.c referenced by committed meson.build (Notes 5.1). Repo hygiene. Effort: trivial (git add). Risk: none.

Drift Note (informational)

The engine source has been extended by ha-commit-durability (implemented 2026-06-18) AFTER ha-commit-engine: majority_candidate_locked now caps the self-contribution at persist_durable_lsn (commit.c:93) rather than the appended tip, and a commit_on_durable_advance flusher wake was added (commit.c:218). This is additive — it tightens Req 2.3 (committed never beyond durable, not merely never beyond appended) and closes bug-661. The ha-commit-engine contract is unchanged and remains fully satisfied. Original research-table anchors (e.g. repl.c:432, persist.c:1035, ha.c:1507) have drifted; verified-current anchors are in the Requirement-to-Asset Map above.