From 54d3293a9eea765d07eecc9dca068f59bd065ca8 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Sat, 20 Jun 2026 10:20:07 +0000 Subject: [PATCH] spec: open heartbeat-committed-propagation brief + log convergence live finding - ha-heartbeat-committed-propagation/brief.md (NEW): the live-fresh off-primary committed follow-up controller-status Req 10 routed out of boundary. Primary stamps committed_lsn/config_tag into each heartbeat (HeartbeatMsg fields exist but are unconsumed); standby monotonically refreshes last_known_committed_*. - ha-convergence-recovery/research.md: live 'cluster won't hold a primary' finding (2026-06-20) as a candidate manifestation for the deferred Task 8 E2E, with a differential (demotion self-restart churn vs prevote thrash vs fsync env) and a diagnosis-first recommendation. Code is complete; this is runtime evidence. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../specs/ha-convergence-recovery/research.md | 84 +++++++++++++++++++ .../brief.md | 50 +++++++++++ 2 files changed, 134 insertions(+) create mode 100644 .kiro/specs/ha-heartbeat-committed-propagation/brief.md diff --git a/.kiro/specs/ha-convergence-recovery/research.md b/.kiro/specs/ha-convergence-recovery/research.md index added24..3038e7a 100644 --- a/.kiro/specs/ha-convergence-recovery/research.md +++ b/.kiro/specs/ha-convergence-recovery/research.md @@ -79,3 +79,87 @@ All file:line anchors from the umbrella audit re-verified by grep before being w - **Build vs adopt**: zero new libraries; all five workstreams reuse existing primitives (truncation, boot recovery, streaming, dispatch-gate allowlist, hook seam pattern). Net-new machinery is limited to saved-argv re-exec, the watermark predicate, the continuation scheduler, and the transition-logging helper. - **Simplification (locked)**: no in-place memory teardown (restart instead), no persisted commit index, no per-entry undo, no filtering of read paths. - Design review gate: all 26 requirement IDs traced; no orphan components; boundary ↔ file-structure alignment verified. + +--- + +## Gap Re-Audit (2026-06-18) + +**Framing**: POST-IMPLEMENTATION GAP AUDIT. Tasks 1-7 (18 sub-tasks) are complete and code-verified; one task remains open (`[ ]` Task 8 — live 3-node E2E). This pass re-verifies every EARS requirement against the on-disk code (codegraph + grep, 2026-06-18 worktree) and characterizes the single open task. + +**Verdict**: PARTIAL — all 9 requirements (26 acceptance criteria) are IMPLEMENTED and unit/integration-tested in code. The only residual gap is the environment-dependent end-to-end verification of the cross-node behaviors (umbrella 2.4/8.1/8.2/9.1/9.2 over a real cluster), which is Task 8. + +### Requirement-to-Asset Map (re-verified 2026-06-18) + +| Req (criteria) | Asset (file:line) | Status | Notes | +|---|---|---|---| +| **1.1** hooks disabled on demotion before anything else | `raft_cb_become_follower` ha.c:1202-1232 (`prior_role == CTRL_PRIMARY && c != NULL` branch reads role before store, disables hooks inline) | MET | guard at ha.c:1205; demotion scheduled at ha.c:1232 | +| **1.2** truncate above committed_lsn on demotion | `ha_demotion_convergence_fn` ha_convergence.c:144-181 → `persist_truncate_wal_above(&c->persist, committed)` ha_convergence.c:163 | MET | committed read via atomic acquire ha_convergence.c:153 | +| **1.3 / 1.7** self-restart rebuilds via boot recovery | `ha_self_restart` ha_convergence.c:82-120 (`execv(argv[0], argv)` :111); boot path `persist_recover`/`replay_wal` reused | MET | truncation precedes restart (1.2 before :180) | +| **1.4** exec failure → FATAL | ha_convergence.c:115-117 `ERROR + abort()`; no-saved-argv → abort ha_convergence.c:101-105 | MET | controller never self-terminates on operational condition — this is crash-equivalent fatal, per locked constraint | +| **1.5** WARN log with truncation range | `ha_demotion_convergence_fn` WARN with committed_lsn + prior_tip, ha_convergence.c:170-173 | MET | prior_tip read under persist lock :157-161 | +| **1.6** XCE_TESTING restart seam | `ha_restart_hook` / `ha_fatal_hook` ha_convergence.c:84-96 | MET | tests drive via these seams | +| **2.1** rejoin tip>boundary → truncate+restart once | `ha_rejoin_reconcile` ha_convergence.c:271+ → `ha_rejoin_reconcile_fn` :240-269; `_Atomic rejoin_reconciled` once-guard | MET | test_rejoin_clamp_…_restarts_once, test_rejoin_second_call_no_second_restart | +| **2.2** never truncate own committed prefix (clamp) | cutoff = `max(primary_committed, local committed_lsn)` (test asserts cutoff=6 when boundary=4, local committed=6) | MET | test_demotion_convergence.c:599-635 | +| **2.3** tip<=boundary → no action | `ha_rejoin_reconcile` early-return path, returns 0 no side effects | MET | test_rejoin_tip_at_or_below_boundary_no_action :642-665 | +| **2.4** single outcome, never retroactive success | demotion + rejoin convergence (in-process halves tested: append committed prefix + uncommitted suffix → demote seam → replay → committed-only). Cross-node end-to-end = Task 8 | PARTIAL (in-process MET; live E2E = Task 8) | the only criterion split across unit + E2E | +| **3.1 / 3.2** uncommitted submit excluded from scheduler export / included once committed | `job_submit_committed` egress.c:47-56 (`submit_lsn==0 \|\| submit_lsn<=committed_lsn`); `collect_jobs_committed` egress.c:76-109 | MET | sched.c:218 (full push) + sched.c:38 (JOB_NEW push) consume it | +| **3.3** submit_lsn tagging live + replay | live: `persist_job_hook_fun` persist.c:2202-2220 (`j->submit_lsn = submit_lsn`); replay: `restore_job(entry_lsn)` from `apply_wal_entry` (hdr->lsn) | MET | tag set on local-write success incl. -ETIMEDOUT/-NOT_READY | +| **3.4** status/API reads unfiltered | apiserver.c:83 + opget.c:173 use plain `collect_jobs` | MET | grep-confirmed: no read path calls the committed-only wrapper | +| **4.1** full fan-out to all reachable standbys | `repl_broadcast` regression pin in test_convergence_verification.c (existing behavior) | MET | verification-only, no new production code | +| **5.1** continuation on walAck, no heartbeat wait | walAck branch → `ha_schedule_catchup_continuation` config_repl.c:1784; body `ha_catchup_continuation_fn` :1739 streams `[acked+1, target]` on delaymgr | MET | re-derives target each round (extends to entries appended meanwhile) | +| **5.2** single-flight per standby | `_Atomic catchup_inflight[]` CAS expected=0→1 config_repl.c:1839; cleared on every exit path :1772/:1791/:1811 | MET | duplicate ACK while in flight schedules nothing | +| **5.3** heartbeat loop stays fallback + snapshot initiator | `ha_dispatch_repl_action_for_standby` byte-unchanged (heartbeat path) | MET | snapshot decision ownership preserved | +| **6.1** snapshot fallback for far-behind standby | `repl_needs_snapshot` repl.c:630 + HA_REPL_SEND_SNAPSHOT decision pin | MET | verification-only | +| **7.1** re-inclusion at parity | `repl_complete_catchup` repl.c:602-628 (clears in_catch_up, calls commit_on_membership_change) — production caller now at parity branch config_repl.c:1831 | MET | closed the "no production caller" gap from §1 | +| **8.1 / 8.2 / 8.3** quorum loss/restore logged once per transition with counts | `ha_log_quorum_transition` ha_convergence.c:412-452 (CAS edge-detect on `quorum_lost`, WARN once); 5 call sites ha.c:1085, 2076, 3969, 5056, gate backstop | MET | mirrors `ha_has_write_quorum` body (connected=repl_active_standby_count, required=n_peers/2) | +| **9.1 / 9.2** status + replication served while quorum lost | `ha_dispatch_gate` degraded-mode allowlist regression pin (admits OPCode_get/heartbeat/repl/fact opcodes; rejects request-shaped) | MET | verification-only | + +**Boundary audits re-confirmed (per Task 7 completion notes)**: `commit.c`/`commit.h` UNTOUCHED by the spec; status/API read paths UNFILTERED; non-PRIMARY `raft_cb_become_follower` behavior byte-equivalent (guarded by `prior_role==CTRL_PRIMARY && c!=NULL`). New TUs `egress.c` and `ha_convergence.c` registered in both `controller_src` and `controller_unit_test_src` (meson.build:6-8, 137-138); three new test files registered (meson.build:130-132). + +### The One Open Task — Task 8 (3-node E2E convergence test) + +**Nature**: ENVIRONMENT-DEPENDENT INTEGRATION TEST — not code work. No production code or unit-test gap remains; every in-process half of Task 8's assertions is already covered by tasks 1-6 (single-outcome demotion+replay in test_demotion_convergence; rejoin clamp/once-guard; commit-gated egress in test_egress_filter; quorum-loss/restore WARN-once in test_convergence_verification). Task 8 exercises the cross-node, cross-process behaviors that a single-process unit binary structurally cannot: +1. Kill primary → promoted standby owns all confirmed jobs. +2. Force a submit to time out at the barrier (withhold standby ACKs) → entry committed on all members OR absent on all members; requester's failure answer never retroactively changed; scheduler never dispatched the unconfirmed job (2.4 end-to-end). +3. Restart the killed ex-primary → exactly one self-restart/reconciliation cycle, converged rejoin, no phantom jobs (1.3 + 2.1 over a real re-exec). +4. Isolate primary → submits fail, joblist/status still served, quorum-loss WARN; heal → quorum-restored WARN, submits succeed (8.1/8.2/9.1/9.2 live). + +**Prerequisites**: +- A running 3-node cluster via `/xce-deploy-ha` (e.g. `localhost:9991 9992 9993`). +- **ha-single-leader-role landed** (direct promotion, CTRL_PRIMARY_COMMITTING removed) — confirmed satisfied in this worktree: `raft_cb_become_leader` ha.c:1026-1036 enters CTRL_PRIMARY directly ("ha-single-leader-role (Req 1.1): enter CTRL_PRIMARY directly"). So the dependency is already met; only the live infra is missing. +- Operator-observed assertions (log inspection for WARN transitions; joblist/status probes during isolation). + +**Effort**: LOW-MEDIUM. The scenario script (deploy → kill/isolate/heal → assert) is mechanical; the cost is standing up and observing a live cluster and authoring the convergence assertions. Estimated a few hours of hands-on cluster time, no design or code uncertainty. Shares the exact blocked-manual status as ha-wal-replication task 5.4. + +**Risk**: LOW for the spec's correctness claims (all logic is unit-verified). Residual risk is integration-surface only — re-exec inheriting an unexpected fd, scheduler/worker reconnection after the restart (explicitly out of scope here; worker-rejoin-resync roadmap backstop), or a timing artifact in the live barrier-timeout path. These are discoverable ONLY on real infra, which is precisely why Task 8 exists. + +### Options for closing Task 8 + +- **Option A — Run the live E2E now** (recommended when infra is available): execute `/xce-deploy-ha` 3-node, run the four scenarios, record converged-outcome + WARN-log assertions in completion notes. Closes 2.4/8.1/8.2/9.1/9.2 end-to-end and marks the spec fully complete. Effort LOW-MEDIUM, Risk LOW. Gated only on cluster availability + ha-single-leader-role (already landed). +- **Option B — Accept as blocked-manual and ship the spec on unit coverage** (current de-facto state): document Task 8 as deferred-to-infra, identical to ha-wal-replication 5.4. All production code is complete and regression-pinned; the umbrella's single-outcome guarantee is established in-process. Effort ZERO, Risk LOW-MEDIUM (integration-surface bugs would surface later, in a deploy/soak rather than in CI). +- **Option C — Invest in automated multi-process E2E harness** (out of current scope): build a scripted kill/isolate/heal driver around `/xce-deploy-ha` so this and ha-wal-replication 5.4 run in CI. Effort HIGH (new test infrastructure), Risk LOW. Pays off across the HA spec family but is a separate workstream, not part of this spec's task list. + +**Recommendation**: Option A if a cluster can be stood up this session; otherwise Option B with an explicit deferral note (consistent with how the sibling ha-wal-replication 5.4 is tracked). Option C is the right long-term move for the HA family but belongs in a dedicated test-infra effort. + +--- + +## Live finding (2026-06-20): cluster "won't hold a primary" — candidate manifestation for the deferred Task 8 E2E + +Surfaced during controller-status `ctrlstatus` live validation on a 3-node `/xce-deploy-ha` cluster. Logged here (not as a new spec) because the convergence-recovery **production code is already complete** (per the Options above) — so the symptom is a *runtime* instability to diagnose against this spec's domain, not missing implementation. + +### Observation +- `wait-primary` named `ctrl-2` as primary in 42 ms, but seconds later `ctrlstatus` answered by `ctrl-3` (standby) reported `Primary: unknown`, `term: 3`, then client failover **exhausted** ("cluster not ready after 21 attempts", `E_XCE_NOT_READY`). The cluster elected a primary then lost it / could not keep one serving. +- Matches the **original /kiro-discovery logs** (the motivating logs for the controller-status spec): repeated PreVote rounds, term bumps, "PreVote round timed out before a decision — treating as LOSS", "no primary served within 30000 ms" — recurring election churn, not a single clean failover. +- The controller unit suite (this session) shows the convergence machinery active and looping in-process: repeated `HA quorum lost: connected=0 required=1`, `PRIMARY demoted (term=7) — scheduling demotion convergence (truncate above committed_lsn + self-restart)`, and `HA rejoin reconcile: local tip exceeded committed boundary; truncated ... restarting once`. The demotion path's **self-restart** is exactly the action that, if re-triggered in a loop, produces primary churn. + +### Differential (root cause NOT yet confirmed — needs live diagnosis) +1. **Demotion self-restart churn (this spec's domain):** quorum-loss → demote → truncate-above-committed → self-restart → rejoin → quorum-loss → … If a transient quorum flap re-enters demotion convergence repeatedly, the cluster never stabilizes a primary. Owned here. +2. **PreVote / election thrash (ha-prevote / ha-single-leader-role):** PreVote rounds timing out as LOSS and re-arming, term bumping without a stable leader. Owned upstream, not here. +3. **Environment (fsync):** on an overlayfs sandbox, `fdatasync errno=5` blocks the durability/commit path (bug-795), so a primary may never advance committed and never stabilize. May be a **sandbox artifact** — the user's real-fs `/tmp/xce-ha-deploy` could behave differently. MUST be ruled out first by reproducing on a real fs. + +### Recommended next step +Run this spec's deferred **Task 8** (Option A) on a real-fs cluster with instrumentation capturing, per node and per term: quorum enter/exit transitions, demotion-convergence entries, PreVote round outcomes, and committed-LSN advance. That evidence tells us whether the churn is convergence self-restart (close it here), election thrash (route to ha-prevote / ha-single-leader-role), or an fsync/env artifact (no code owner). Until that diagnosis runs, do NOT assume this is a convergence-recovery code defect — the code is present and unit-green; this is an integration/runtime observation. + +### Related +- controller-status `ctrlstatus` is the diagnostic tool here (answers in every role incl. fenced/no-quorum; term/commit/apply visible; responder timestamp for log correlation) — use it to watch the churn live. +- The failover-exhaustion error rendering crashed (`xce_strerror_r` global-buffer-overflow, bug-827, fixed in libxce `utils.c`) — rebuild before re-observing so the client prints a clean `not-ready` instead of aborting. diff --git a/.kiro/specs/ha-heartbeat-committed-propagation/brief.md b/.kiro/specs/ha-heartbeat-committed-propagation/brief.md new file mode 100644 index 0000000..01d2456 --- /dev/null +++ b/.kiro/specs/ha-heartbeat-committed-propagation/brief.md @@ -0,0 +1,50 @@ +# Brief: ha-heartbeat-committed-propagation + +> Opened 2026-06-20 from a controller-status (`ctrlstatus`) live finding. This is the **live-fresh off-primary committed** follow-up that controller-status Req 10 explicitly routed out of its own boundary. + +## Problem +A standby's knowledge of the cluster's committed position — `ha->last_known_committed_lsn` / `ha->last_known_committed_config_tag` — is refreshed **only at CtrlHello join / RDHUP-failover** (`cluster_apply_ctrlhello_reply`, `ha_convergence.c:375`) and an election safe-commit path. It is **not** refreshed on steady-state heartbeats. So between joins, a standby's view of committed lags the live primary, even as the standby keeps applying replicated entries. + +This surfaced live in `ctrlstatus` (controller-status Req 10): a standby reported `committedLsn: 0` while `appliedLsn: 10` — honest per Req 10.2 (it never learned a committed position) but not useful. Even after a join, the reported committed can sit stale between joins. controller-status Req 10 deliberately reports the honest best-known value and documented this staleness as an **out-of-boundary** caveat to be fixed here. + +The wire already anticipates this: `HeartbeatMsg` (`src/schema/controller.capnp:761`) **defines** `committedLsn @2` and `committedConfigTag @3`, but **no code populates or consumes them** — the `OP_nop` heartbeat carries no committed payload, and `ha_on_heartbeat_received` only updates liveness. + +## Current State +- `HeartbeatMsg{committedLsn, committedConfigTag}` fields exist in the schema but are dead (no producer, no consumer). +- `last_known_committed_lsn` (`ha.h:170`, `_Atomic`) + `last_known_committed_config_tag` (`ha.h:172`, plain) written only at join/RDHUP + election safe-commit. +- `OP_nop` heartbeat receive path (`cluster.c:300`) resets liveness only. +- controller-status `ctrlstatus_fill` already reports `last_known_committed_*` off-primary (controller-status task 6.2) — it will automatically reflect a fresher value once this spec lands. + +## Desired Outcome +The primary stamps its current `committed_lsn` / `committed_config_tag` into every heartbeat; each standby consumes them in the heartbeat receive path and **monotonically** advances `last_known_committed_lsn` / `last_known_committed_config_tag`. Off-primary committed then tracks the live primary within one heartbeat interval, and `ctrlstatus` on a standby shows a near-real-time committed position instead of a join-stale (or zero) value. + +## Approach +Populate the already-defined `HeartbeatMsg.committedLsn`/`committedConfigTag` on the primary's heartbeat sender; consume them on the standby's heartbeat receive path with a monotonic update of `last_known_committed_*`. No change to how committed is produced/replicated/persisted — this only propagates an already-committed marker as liveness-adjacent metadata. + +## Scope +- **In**: primary heartbeat sender populates `committedLsn`/`committedConfigTag`; standby heartbeat receive path consumes them → monotonic update of `last_known_committed_lsn`/`last_known_committed_config_tag`; unit + integration tests (standby's learned committed advances on heartbeat without a re-join); memory-ordering consistency for the non-atomic `last_known_committed_config_tag` vs the `ctrlstatus` reader. +- **Out**: how `committed_lsn` is produced/advanced (ha-commit-engine / commit durability); the `ctrlstatus` reporting itself (controller-status Req 10, already done); WAL catch-up triggering (stays ACK-driven per ha-convergence-recovery); election safe-commit derivation (unchanged — must NOT use heartbeat-borne committed for truncation, per the existing "never truncate from stale ctrlhello/heartbeat" rule). + +## Boundary Candidates +- Primary heartbeat sender (stamp committed into `HeartbeatMsg`). +- Standby heartbeat receive (consume + monotonic `last_known_*` update). +- The monotonicity + memory-ordering contract on `last_known_committed_*`. + +## Out of Boundary +- Committed-LSN production / quorum advance (commit engine). +- `ctrlstatus` rendering and field sourcing (controller-status). +- Any use of heartbeat-borne committed for WAL truncation or vote-freshness floor changes (explicitly forbidden — truncation/freshness must stay durable-backed; this value is advisory/reporting only). + +## Upstream / Downstream +- **Upstream**: ha-heartbeat-liveness (owns `HeartbeatMsg` + the heartbeat tick/receive path), ha-commit-engine (committed_lsn semantics). +- **Downstream**: controller-status Req 10 (off-primary committed becomes live-fresh; removes the documented staleness caveat); ha-vote-freshness-invariant (voter floor `max(durable_floor, last_known_committed_lsn)` benefits from a fresher `last_known_committed_lsn`, but freshness must remain correct — verify no safety regression). + +## Existing Spec Touchpoints +- **Extends**: ha-heartbeat-liveness (heartbeat content/behavior). +- **Adjacent**: ha-repl-hardening (standby replication state), controller-status (the consumer that motivated this), ha-convergence-recovery (catch-up stays ACK-driven, not heartbeat-driven). + +## Constraints +- `last_known_committed_*` updates must be **monotonic** (never regress on an out-of-order/stale heartbeat). +- Heartbeat path is hot — keep the added work to two field reads + two conditional stores. +- The heartbeat-borne committed is **advisory** (reporting/liveness): it must never feed WAL truncation or weaken the durable-backed vote-freshness floor. +- `last_known_committed_config_tag` is non-atomic (`ha.h:172`); reconcile the writer/reader memory ordering with the `ctrlstatus` best-effort read (consider promoting to `_Atomic`).