From 227f5922507ed1627719267fc2e6c876491ba470 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Sat, 27 Jun 2026 04:37:07 +0000 Subject: [PATCH] docs(raft-ha-core-safety): finalize spec (requirements, design, research, tasks) Reframed as residual-defect + convergence/liveness + live-verification spec (machinery present at baseline 21e26a1). Root cause: term-blind catch-up bypass of the single reconcile seam. 8 requirements, 7-phase task plan, all review gates passed. --- .kiro/specs/raft-ha-core-safety/design.md | 348 ++++++++++++++++++ .../specs/raft-ha-core-safety/requirements.md | 122 ++++-- .kiro/specs/raft-ha-core-safety/research.md | 130 +++++++ .kiro/specs/raft-ha-core-safety/spec.json | 18 +- .kiro/specs/raft-ha-core-safety/tasks.md | 132 +++++++ 5 files changed, 710 insertions(+), 40 deletions(-) create mode 100644 .kiro/specs/raft-ha-core-safety/design.md create mode 100644 .kiro/specs/raft-ha-core-safety/research.md create mode 100644 .kiro/specs/raft-ha-core-safety/tasks.md diff --git a/.kiro/specs/raft-ha-core-safety/design.md b/.kiro/specs/raft-ha-core-safety/design.md new file mode 100644 index 00000000..1b904567 --- /dev/null +++ b/.kiro/specs/raft-ha-core-safety/design.md @@ -0,0 +1,348 @@ +# Design Document — raft-ha-core-safety + +## Overview + +This feature makes repeated and chaotic primary failover on an XCE controller HA cluster provably safe and live. It is a **correction-and-verification** spec, not a build-from-scratch one: the `(term, lsn)` commit-identity, conflict-reconcile, Figure-8 commit, and vote-freshness machinery already exist and are validated (`raft-log-replication`, `raft-commit-safety`, `ha-term-safety`), yet the live chaotic soak still produces committed-prefix divergence (LSN 9), standby-rejoin failure, and intermittent election stalls — on this branch and on baseline `21e26a1`. + +Discovery root-caused the residual divergence to a **split catch-up path**: term-carrying entries route through the single Raft reconcile seam (`raft_on_append_entries_request` → logOk → `on_truncate_from` → `persist_set_term`), but a **term-blind branch** (`entry->term == 0` → `ha_apply_single_entry` → `persist_wal_write_replicated` direct) bypasses logOk entirely, stamps the standby's own term instead of the leader's, and the persistence layer's **LSN-only idempotency guard silently no-ops** when the correct term-carrying entry later arrives for an already-written LSN — so a divergent committed entry can never be repaired. + +The design therefore: (1) collapses every entry-ingest path into the one reconcile seam and makes persistence `(term, lsn)`-conflict-aware instead of LSN-only-idempotent; (2) closes the snapshot-boundary and standby-reconvergence gaps; (3) guarantees election liveness under churn; and (4) delivers a deterministic chaotic harness that forces each defect RED before its fix and a green 6+ round real-fs soak as the acceptance gate. + +### Goals +- Committed log prefix is identical across all members under repeated/chaotic failover (zero `walcompare` divergence). +- Every entry entering a member's log passes the single `(term, lsn)` reconcile seam; no path stamps a non-leader term or silently skips conflict repair. +- A restarted node deterministically reconverges to 3-of-3 (committed + applied), and elections always complete under churn. +- Each defect is pinned by a deterministic RED→GREEN test; the spec closes only on a green 6+ round chaotic real-fs release soak with zero divergence and zero committed-job loss. + +### Non-Goals +- Rebuilding or re-deriving the Raft library consensus core (log replication, commit advance, vote freshness) — adopted as-is. +- Persist-before-ACK durability and the sync barrier — owned by `ha-commit-durability`. +- `raft-drive-async` async-drive/offload behavior (SlowLane, peer-control dispatch, snapshot/config-push offload, endpoint-quiescence) — out of scope and must not change. +- Client-side primary discovery (libxce bug-854) and the overlayfs `fdatasync` environment defect (bug-924) — excluded from the acceptance verdict, not fixed here. +- Cluster membership resize, multi-region, non-Raft replication. + +## Boundary Commitments + +### This Spec Owns +- The **single-seam ingest invariant**: routing every replicated/catch-up entry through `raft_on_append_entries_request` so logOk + `(term,lsn)` conflict-truncate + leader-term stamping always run. +- Making the WAL persistence idempotency/contiguity guard **`(term,lsn)`-conflict-aware** (detect same-LSN/different-term as a conflict to repair, not a no-op to skip). +- **Snapshot-boundary `(term,lsn)` consistency** so post-install AE intake reconciles correctly and no committed entry below the checkpoint can diverge. +- **Standby reconvergence completeness**: post-recovery `log_len`/applied sync and catch-up apply completeness before a node reports healthy/3-of-3. +- **Election liveness under churn**: a member that loses its primary always proceeds to a completed election; no permanent stall. +- The **deterministic chaotic harness** (unit/integration + `xce-test-ha` soak extension + `walcompare` use) and the **acceptance soak gate**. + +### Out of Boundary +- Raft library internals (`src/lib/raft/*`) beyond consuming the existing callbacks/seam — no new consensus primitives. +- Persist-before-ACK durability mechanism and the commit sync barrier (`ha-commit-durability`). +- `raft-drive-async` dispatch behavior. +- libxce client discovery (bug-854); overlayfs env (bug-924). +- Sequential-failover rejoin reconcile already delivered by `ha-convergence-recovery` (reused, not re-implemented). + +### Allowed Dependencies +- Raft library reconcile seam + five callbacks (`raft_on_append_entries_request`, `on_persist_entry`, `on_get_entry_term`, `on_truncate_from`, `on_commit_advanced`). +- `persist_set_term` / per-LSN term index / `WAL_LEGACY_TERM` mapping (`ha-term-safety`). +- `ha_rejoin_reconcile` intake hook and `ha_self_restart` convergence primitive (`ha-convergence-recovery`). +- `walcompare` `(term, lsn, pcrc)` detector; `xce-test-ha` soak driver; `run-ha-soak.sh` BIN/XT override + non-ASAN release build. +- The controller TLA+ spec `raft/raft.tla` and the invariant checklist in `src/controller/CLAUDE.md` as the correctness oracle. + +### Revalidation Triggers +- Any change to the Raft callback signatures or the reconcile-seam contract → revalidate `ha-term-safety`, `raft-log-replication`, `raft-commit-safety`. +- Any change to WAL header `(term, lsn)` layout or the term index → revalidate `walcompare` and `ha-commit-durability`. +- Any change to the snapshot install boundary contract → revalidate `ha-convergence-recovery` rejoin path. +- Closing this gate (green soak) is itself the revalidation trigger that **unblocks the `raft-drive-async` production flag-flip**. + +## Architecture + +### Existing Architecture Analysis +- **Single reconcile seam (preserve, do not duplicate):** all conflict detection/repair lives inside `raft_on_append_entries_request` (raft.c:646–765). The defect is that not every ingest path enters it. +- **Layering (must hold):** Raft library (pure, callback-driven) ← controller HA adapter (`ha.c`, `ha_convergence.c`, `config_repl.c`) ← persistence (`persist.c`, WAL). The adapter calls into the library and implements callbacks; the library never calls the adapter except through callbacks. The fix routes adapter ingest *into* the library, never around it. +- **Two distinct truncate contracts (must not be conflated):** + - **Promotion-truncate** (a node becoming leader): clamps at `max(safe_committed_lsn, durable_tip)` (ha.c:999–1000, bug-1060) — a fresh leader must not discard its own durable tip. + - **Follower-AE-conflict-truncate** (a follower/standby receiving an entry that conflicts at `(term, lsn)`): obeys Raft Log Matching — truncates the conflicting suffix **regardless of local `durable_tip`**. The bug-1060 promotion clamp does **not** apply here. Rationale: an entry that conflicts with the leader's `(term, lsn)` was, by Raft's Leader Completeness, never majority-committed, so it is safe — and required — to remove even if the local node had marked it durable/committed. Applying the promotion clamp on this path would block legitimate repair and convert repairable divergence into a permanent fence. + - The only entries the follower-conflict path must never cut are those that match the leader (no conflict) — guaranteed by logOk before any `on_truncate_from`. +- **Technical debt addressed:** the term-blind ingest branch and the LSN-only idempotency guard are the debt this spec removes. + +### Architecture Pattern & Boundary Map + +```mermaid +graph TB + subgraph Library [Raft Library pure] + AE[raft_on_append_entries_request] + LOGOK[logOk term lsn check] + TRUNC[on_truncate_from] + PERSISTCB[on_persist_entry] + AE --> LOGOK --> TRUNC + LOGOK --> PERSISTCB + end + subgraph Adapter [Controller HA Adapter] + INGEST[Unified Entry Ingest] + SNAP[Snapshot Boundary Reconcile] + CONV[Standby Reconvergence] + ELECT[Election Liveness] + end + subgraph Persist [Persistence WAL] + GUARD[term lsn conflict aware guard] + SETTERM[persist_set_term leader term] + TERMIDX[per lsn term index] + end + subgraph Verify [Verification] + HARNESS[Deterministic Chaotic Harness] + WALCMP[walcompare term lsn pcrc] + SOAK[xce-test-ha chaotic soak] + end + INGEST --> AE + SNAP --> AE + CONV --> INGEST + PERSISTCB --> SETTERM --> GUARD --> TERMIDX + TRUNC --> GUARD + ELECT -.guarantees.-> AE + HARNESS --> WALCMP + HARNESS --> INGEST + SOAK --> WALCMP +``` + +**Architecture Integration** +- **Selected pattern:** funnel-into-single-seam — every ingest path (live replication, catch-up batch, snapshot post-install, rejoin) terminates in `raft_on_append_entries_request`; persistence enforces `(term,lsn)` conflict semantics underneath. +- **Boundaries:** ingest-routing (Adapter), persistence-conflict (Persist), liveness (Adapter election), verification (test-only) are four separable seams under one acceptance gate. +- **Patterns preserved:** single reconcile seam, callback-driven library, truncate-clamp, persist-before-ACK. +- **New components rationale:** only the harness is genuinely new code; everything else removes a bypass or hardens an existing guard. +- **Steering compliance:** async-I/O (no blocking in vtask handlers; file I/O + restart on delaymgr, never callback thread), fail-loud over silent skip, controller-never-self-terminates-on-operational-condition. + +### Technology Stack + +| Layer | Choice / Version | Role in Feature | Notes | +|-------|------------------|-----------------|-------| +| Consensus core | `src/lib/raft` (in-repo) | reconcile seam + callbacks (consumed) | no library changes beyond callback use | +| Controller HA adapter | C (`ha.c`, `ha_convergence.c`, `config_repl.c`) | ingest routing, snapshot/rejoin, election | primary change surface | +| Persistence | C (`persist.c`, WAL segments) | `(term,lsn)`-aware guard, term index | harden guard, no format change | +| Verification | C unit (`controller_unit_test`), `walcompare`, `xce-test-ha` bash | RED→GREEN + soak gate | new `test_ha_core_safety.c`; chaotic soak mode | +| Runtime | real filesystem (ext4) release build | acceptance soak | overlayfs excluded (bug-924) | + +## File Structure Plan + +### Modified Files +- `src/controller/ha.c` — Collapse the term-blind ingest branch: `ha_apply_wal_catchup_entry` (ha.c:2233) and `ha_apply_single_entry` (ha.c:2097) route **all** entries through `ha_apply_term_entry_via_library`/`ha_consume_append_entries` (the seam), never `persist_wal_write_replicated` directly. Snapshot post-install (`ha_handle_snapshot_chunk` ha.c:2492) establishes the `(term,lsn)` boundary. Standby reconvergence completion (applied/log_len sync, healthy gating). Election-liveness guarantee on the failover/vote path. +- `src/controller/persist.c` — Make the idempotency/contiguity guard (`persist_wal_write_replicated` persist.c:802, guard at :822/:839) **`(term,lsn)`-conflict-aware**: same LSN + different term ⇒ surface a conflict to the reconcile seam (truncate-then-repersist) rather than `return 0`. Ensure no ingest writes a non-leader term. +- `src/controller/config_repl.c` — Leader-side streamer (`ha_stream_wal_to_standby` config_repl.c:2032) always emits term-carrying entries (no `entry->term == 0` on the wire from a live leader); walAck continuation feeds only the seam path. +- `src/controller/ha_convergence.c` — Reuse the `ha_rejoin_reconcile` intake hook for chaotic-rejoin reconciliation through the seam; retain the `ha_self_restart` convergence primitive and once-guard. +- `src/controller/testsuite/meson.build` — Register `test_ha_core_safety.c`. +- `src/controller/testsuite/walcompare.c` — Extend to emit a machine-checkable `(lsn → term, pcrc, present)` per-member report consumable by the harness verdict (committed-vs-uncommitted classification). +- `.claude/skills/xce-test-ha/run-ha-soak.sh` (and `SKILL.md`) — Add a **chaotic mode** (overlapping kills inside election windows) and a verdict that isolates bug-854/bug-924 from the divergence/job-loss result. + +### New Files +- `src/controller/testsuite/test_ha_core_safety.c` — Deterministic RED→GREEN harness: forces the term-blind divergence, snapshot-boundary divergence, standby reconvergence-failure, and election-stall paths using existing gate/probe seams (pattern: `test_ha_dispatch_integration.c`); no sleeps. + +> Dependency direction (enforced): `persist.c` (term/lsn storage) ← `ha.c`/`config_repl.c`/`ha_convergence.c` (adapter ingest/election) → Raft library seam. Tests depend on all; production never depends on tests. + +## System Flows + +### Divergence repair via unified seam (the fix) + +```mermaid +sequenceDiagram + participant L as Leader + participant S as Standby ingest + participant Seam as raft_on_append_entries_request + participant P as persist term lsn guard + L->>S: replicate or catch-up entry at LSN N + S->>Seam: route ALL entries no term-blind branch + Seam->>Seam: UpdateTerm then logOk prev term lsn + alt conflict same LSN different term + Seam->>P: on_truncate_from N + Seam->>P: on_persist_entry N leader_term + P->>P: guard sees term conflict repersist not no-op + else match + Seam->>P: on_persist_entry N leader_term + end + P-->>S: WAL holds leader term at N + Note over S,P: walcompare term pcrc identical across members +``` + +Key decisions: the term-blind branch is removed (single path); the persist guard distinguishes "same LSN same term" (idempotent no-op, safe) from "same LSN different term" (conflict → truncate+repersist). Truncation never cuts below `max(safe_committed_lsn, durable_tip)`. + +### Standby reconvergence gating + +```mermaid +stateDiagram-v2 + [*] --> Rejoining + Rejoining --> CatchingUp: route entries through seam + CatchingUp --> AppliedSync: log tip reached at match level + AppliedSync --> Healthy: applied_lsn and log_len equal leader committed + AppliedSync --> CatchingUp: apply gap remains + Healthy --> [*]: report 3 of 3 +``` + +A node is reported healthy/3-of-3 only when committed **and** applied state match the leader — not at match-index alone (closes the bug-1042 reconvergence gap). + +## Requirements Traceability + +| Requirement | Summary | Components | Key Contracts | Flows | +|-------------|---------|------------|---------------|-------| +| 1.1–1.4 | Committed prefix identical; no divergent entry at a position; divergent treated non-authoritative; TLA+ State-Machine/Commit safety | Unified Entry Ingest; Persist Conflict Guard | seam routing; `(term,lsn)` guard | Divergence repair | +| 2.1–2.4 | Entry stamped leader term; never term-0 committed; identity distinguishes conflicts; identity survives restart | Persist Conflict Guard; Unified Entry Ingest | `persist_set_term`; term index | Divergence repair | +| 3.1–3.4 | Detect conflict; repair to majority prefix; surface unrecoverable; never cut majority-committed | Unified Entry Ingest; Snapshot Boundary | `on_truncate_from`; follower-conflict truncate contract (logOk-gated) | Divergence repair | +| 4.1–4.3 | At most one committing primary/term; step-down on higher term; Election-Safety/Leader-Completeness | Election Liveness (step-down) | vote-once; pre-vote; vote freshness | — | +| 5.1–5.4 | Rejoin → 3-of-3 in bound; applied+log_len sync; catch-up completeness; not converged until applied matches | Standby Reconvergence | healthy-gating contract | Reconvergence gating | +| 6.1–6.3 | Elect higher-term primary in bound; keep electing under churn; retry on failed round | Election Liveness | election-restart guarantee | — | +| 7.1–7.5 | Force divergence/reconverge/stall RED on baseline; GREEN after; extend soak + unit/integration | Deterministic Chaotic Harness | RED→GREEN gate | all | +| 8.1–8.4 | 6+ round chaotic real-fs soak: zero divergence, zero job loss, reconverge+elect each round; gate for flag-flip | Acceptance Soak Gate | soak verdict (bug-854/924 isolated) | all | + +## Components and Interfaces + +| Component | Layer | Intent | Req Coverage | Key Dependencies | Contracts | +|-----------|-------|--------|--------------|------------------|-----------| +| Unified Entry Ingest | Adapter | Route every entry through the one reconcile seam | 1,2,3 | Raft seam (P0); Persist guard (P0) | Service, State | +| Persist Conflict Guard | Persist | `(term,lsn)`-conflict-aware write; never non-leader term | 1,2,3 | term index (P0); `persist_set_term` (P0) | State | +| Snapshot Boundary Reconcile | Adapter | Establish `(term,lsn)` at checkpoint; post-install reconcile | 3,5 | snapshot install (P0); seam (P1) | State | +| Standby Reconvergence | Adapter | Gate healthy/3-of-3 on committed+applied match | 5 | catch-up apply (P0); `ha_rejoin_reconcile` (P1) | Service, State | +| Election Liveness | Adapter | Always complete an election under churn; step-down on higher term | 4,6 | vote/election path (P0) | Service | +| Deterministic Chaotic Harness | Verify | Force each defect RED→GREEN; no sleeps | 7 | gate/probe seams (P0); `walcompare` (P0) | Batch | +| Acceptance Soak Gate | Verify | 6+ round chaotic real-fs soak verdict | 8 | `xce-test-ha`; release build | Batch | + +### Adapter + +#### Unified Entry Ingest +| Field | Detail | +|-------|--------| +| Intent | Every replicated/catch-up entry passes `raft_on_append_entries_request` (logOk + `(term,lsn)` conflict + leader-term stamp). No term-blind branch. | +| Requirements | 1.1, 1.2, 2.1, 2.3, 3.1, 3.2, 3.4 | + +**Responsibilities & Constraints** +- Remove the `entry->term == 0` direct-persist branch; route batch/single/catch-up ingest through the seam. +- Conflict repair on this (follower/standby) path uses the **follower-AE-conflict-truncate** contract (truncate the conflicting suffix per Raft Log Matching) — **not** the promotion clamp. logOk gates the truncate, so only genuinely-conflicting (never-majority-committed) entries are cut. The promotion clamp applies only to the leader-promotion path, which this component does not touch. +- File I/O / truncation / restart never on a callback or iomgr thread — defer to delaymgr (existing pattern). + +**Dependencies**: Inbound: catch-up/replication/rejoin call sites (P0). Outbound: Raft seam (P0); Persist Conflict Guard (P0). + +**Contracts**: State [x] / Service [x] + +##### State Management +- State model: per-member WAL log indexed by LSN, each entry carrying `(term, payload)`; library `log_len`/`commit_index` mirror the WAL. +- Consistency: post-ingest, `on_get_entry_term(N)` equals the leader's term for N across all members; `walcompare` reports identical `(term, pcrc)` on the committed prefix. +- Concurrency: ingest may run on iomgr; deferred truncate/persist follow the existing delaymgr one-shot pattern. + +**Implementation Notes** +- Integration: reuse `ha_apply_term_entry_via_library`/`ha_consume_append_entries`; retire `ha_apply_single_entry`'s direct-persist path. +- Validation: RED unit test injects a term-blind/divergent entry at an LSN and asserts the seam repairs it and `persist_set_term` stamped the leader term. +- Risks: the safety boundary on this path is **logOk**, not a durable-tip clamp — only entries that fail the `(term, lsn)` match are truncated, and a failed match proves the entry was never majority-committed (Leader Completeness). A unit test must assert that an entry which *matches* the leader is never truncated, and that a *conflicting* entry below local `durable_tip` *is* truncated. + +#### Standby Reconvergence +| Field | Detail | +|-------|--------| +| Intent | Report a rejoining node healthy/3-of-3 only after committed **and** applied state match the leader. | +| Requirements | 5.1, 5.2, 5.3, 5.4 | + +**Responsibilities & Constraints**: post-recovery `log_len`/applied-lsn sync; complete outstanding catch-up apply before "healthy"; bounded recovery window. + +**Dependencies**: Inbound: rejoin path (P0). Outbound: Unified Entry Ingest (P0); `ha_rejoin_reconcile` hook (P1). + +**Contracts**: State [x] / Service [x] +- Postcondition: `applied_lsn == log_len == leader_committed_lsn` before reporting converged; otherwise remain `CatchingUp`. + +**Implementation Notes** +- Integration: reuse the `ha-convergence-recovery` rejoin hook; add the applied/log_len equality gate for the chaotic path. +- Validation: RED integration test — inject an apply gap and assert the node is not reported converged until the gap closes. + +#### Election Liveness +| Field | Detail | +|-------|--------| +| Intent | A member that loses its primary always proceeds to a completed election; at most one committing primary per term. | +| Requirements | 4.1, 4.2, 4.3, 6.1, 6.2, 6.3 | + +**Responsibilities & Constraints**: on primary loss with quorum reachable, elect a higher-term primary within the failover window; restart a failed election round; step down on observing a higher/equal-term leader. No permanent stall under repeated rapid rounds. + +**Dependencies**: Inbound: failover detection (P0). Outbound: Raft vote/election + `raft_cb_vote_eligible` (P0). + +**Contracts**: Service [x] +- Invariant: never two committing primaries in one term (Election Safety); a node without a primary keeps initiating elections (liveness). + +**Implementation Notes** +- Validation (liveness, R6): RED integration test forces repeated rapid kill/restart rounds and asserts a higher-term `CTRL_PRIMARY` appears each round; a scripted single-round failure asserts a follow-on election starts. +- Validation (safety, R4 — two-leaders-per-term): an explicit **Election Safety assertion** runs in the chaotic harness and soak — for every observed term, at most one member ever reaches committing-primary state. This is a *falsifiable verification*, not an assertion of correctness: it maps to `raft/raft.tla` Election Safety and must hold under overlapping kills. If it fails, the design's "library already enforces single-leader" assumption is disproven and a concrete guard (e.g. tighten the pre-vote/vote-once window at the adapter promotion gate) becomes required work rather than verification. +- Risks: over-eager step-down could itself cause a stall — step-down must be paired with election restart. The harness must distinguish a *liveness* failure (no primary) from a *safety* failure (two primaries) so a fix for one does not regress the other. + +### Persist + +#### Persist Conflict Guard +| Field | Detail | +|-------|--------| +| Intent | `(term,lsn)`-aware write: same-LSN/same-term is an idempotent no-op; same-LSN/different-term is a conflict surfaced for truncate+repersist; never stamp a non-leader term. | +| Requirements | 1.1, 1.2, 2.1, 2.2, 2.4, 3.1, 3.4 | + +**Responsibilities & Constraints**: replace the LSN-only idempotency check (persist.c:822) with `(term,lsn)` semantics; preserve persist-before-ACK; preserve the term index and `WAL_LEGACY_TERM` mapping. + +**Dependencies**: Inbound: `on_persist_entry`/`on_truncate_from` (P0). Outbound: term index `record_entry_term`/`persist_entry_term` (P0). + +**Contracts**: State [x] +- Precondition: caller has passed the entry through the seam (leader term known via `persist_set_term`; logOk already decided match vs conflict). +- Postcondition: WAL at LSN N holds exactly the leader's `(term, payload)`; no silent retention of a stale-term entry. +- Invariant: never write `term == 0` for a live-replicated entry. On the seam-driven conflict path the guard honors `on_truncate_from` (follower-AE-conflict contract) — it does **not** re-apply the promotion durable-tip clamp, which belongs to the leader-promotion path only. The guard never *initiates* a truncate on its own; it only executes the seam's decision. + +**Implementation Notes** +- Integration: the guard distinguishes "already have identical entry" from "have a different entry at this LSN"; the latter routes to the seam's truncate path instead of `return 0`. +- Validation: unit test writes a wrong-term entry at LSN N, then the correct one, and asserts the WAL ends with the leader term (no silent skip). +- Risks: mis-tagging same-term-same-payload as a conflict would cause needless truncation — the guard compares both term and payload-crc. + +### Verify + +#### Deterministic Chaotic Harness +| Field | Detail | +|-------|--------| +| Intent | Force each defect deterministically; RED on baseline, GREEN after fix; no sleeps. | +| Requirements | 7.1, 7.2, 7.3, 7.4, 7.5 | + +**Contracts**: Batch [x] +- Trigger: unit/integration run (`controller_unit_test`) + `xce-test-ha` chaotic soak. +- Input: injected divergent `(term,lsn)` entry; injected apply gap; scripted overlapping kills inside election windows. +- Output: pass/fail per path; `walcompare` `(term,lsn,pcrc)` report. +- Idempotency & recovery: deterministic gate/probe seams (pattern `test_ha_dispatch_integration.c`); teardown drains pending tasks (avoid the known teardown-race UAF class, bug-1224). + +**Hypothesis-validation discipline (mandatory):** +- The term-blind bypass is the *primary* hypothesis, not a proven sole cause. The RED gate (7.1) must **first reproduce the actual live divergence** — a `walcompare` divergence from the chaotic soak — and bisect it to a concrete seam, **before** the synthetic `(term,lsn)`-injection unit test is accepted as representative. A green synthetic test over a still-red soak is not a pass. +- Note baseline `21e26a1` already carries ha-term-safety 3.4 (retired `term==0` repl_broadcast), so a *live leader* may never emit `term==0`. If the live divergence is therefore **not** the term-blind path, the harness's `walcompare (term,lsn,pcrc)` classification (committed vs uncommitted; which member, which term) drives re-diagnosis before any fix is committed. +- **Contingency**: if the soak still diverges after the term-blind path is closed, treat it as a new finding — classify via `walcompare`, record in `research.md`, and add the next fix task; do **not** mark R1/R8 done on the strength of green unit tests alone. + +#### Acceptance Soak Gate +| Field | Detail | +|-------|--------| +| Intent | 6+ round chaotic failover on real fs (release build): zero divergence, zero job loss, reconverge+elect each round. | +| Requirements | 8.1, 8.2, 8.3, 8.4 | + +**Contracts**: Batch [x] +- Trigger: `run-ha-soak.sh` chaotic mode, real-fs (ext4) release binaries. +- Output: verdict that **isolates** bug-854 (client discovery) and bug-924 (overlayfs) from the divergence/job-loss result. +- Idempotency & recovery: verdict counts `walcompare` divergences and committed-job loss; gate passes only at zero across ≥6 rounds. This green result is the trigger that unblocks the `raft-drive-async` production flag-flip. + +## Error Handling + +### Error Strategy +- **Conflict at an LSN (same LSN, different term) on the follower/standby path:** logOk fails ⇒ seam truncates the conflicting suffix (follower-AE-conflict contract, no promotion clamp) and repersists the leader's `(term, payload)`; never a silent no-op (`fail-loud over silent skip`). This is the normal, recoverable case — a conflicting entry was never majority-committed, so cutting it loses no committed data. +- **Genuinely unrecoverable divergence:** only when a node cannot obtain the majority-agreed prefix at all (e.g. the leader's log no longer covers the needed range and no snapshot can backfill it) — surface a loud, non-promotable fence and keep retrying; the controller does **not** self-terminate on an operational condition (steering invariant). Operator-visible WARN/ERROR with both LSNs/terms. Note: a local entry conflicting below the node's own `durable_tip` is **not** in this category — it is repairable via truncate, because the conflict proves it was never committed cluster-wide. +- **Reconvergence apply gap:** remain `CatchingUp`, keep streaming via the seam; never report healthy with an open gap. +- **Election round failure:** start a further election; never park without a primary while quorum is reachable. + +### Monitoring +- `walcompare` divergence count (soak verdict), per-edge quorum transition WARN (existing), election round/term progression logs, reconvergence applied/log_len deltas. Each defect path emits an operator-observable line so the soak verdict is explainable. + +## Testing Strategy + +### Unit Tests (`test_ha_core_safety.c`, `controller_unit_test`) +- Wrong-term entry at LSN N then correct entry ⇒ WAL ends with leader term; persist guard did not silently skip (2.1, 2.2, 3.1). +- Term-blind ingest path removed: a `term==0`-shaped catch-up entry still routes through the seam and is reconciled (1.2, 3.2). +- Follower-conflict truncate contract: a *conflicting* entry below local `durable_tip` **is** truncated and repaired (the promotion clamp does not block it); a *matching* entry is **never** truncated (3.2, 3.4). +- Reconvergence gate: injected apply gap ⇒ node not reported converged until closed (5.2, 5.3, 5.4). + +### Integration Tests +- Snapshot post-install reconcile: a divergent pre-snapshot entry is reconciled against the checkpoint `(term,lsn)` (3.1, 5.1). +- Election liveness under churn: repeated rapid kill/restart ⇒ higher-term `CTRL_PRIMARY` each round; scripted failed round ⇒ follow-on election (6.1, 6.2, 6.3). +- Election Safety (two-leaders-per-term): across overlapping kills, assert at most one member ever reaches committing-primary per term — falsifiable check mapped to `raft/raft.tla` Election Safety; failure promotes a concrete adapter-gate guard to required work (4.1, 4.3). + +### E2E / Soak (`xce-test-ha` chaotic mode, real fs, release) +- **Live-repro first**: the chaotic soak must first reproduce the actual `walcompare` divergence (RED) and bisect it to a seam before synthetic unit tests are accepted as representative (7.1). +- ≥6 rounds chaotic failover: zero `walcompare` divergence (8.1); zero committed-job loss (8.2); 3-of-3 reconverge + new primary each round (8.3); verdict isolates bug-854/bug-924 (8.4). + +### RED-first ordering +- Every implementation task is preceded by its failing test on the unfixed baseline (project TDD steering). The live-divergence repro (7.1) and harness/RED tests land before the ingest/persist/election fixes so each fix is pinned to a `walcompare`/invariant violation (7.1–7.4). A green synthetic unit test over a still-red soak does not satisfy R1/R8. + +## Security Considerations +- No new external surface; all changes are intra-cluster consensus/persistence. The single-seam invariant strengthens integrity (no unvalidated entry enters a member's committed log). Fenced unrecoverable-divergence prevents serving divergent committed state. No secrets, auth, or input-boundary changes. \ No newline at end of file diff --git a/.kiro/specs/raft-ha-core-safety/requirements.md b/.kiro/specs/raft-ha-core-safety/requirements.md index f8904f2c..2911c646 100644 --- a/.kiro/specs/raft-ha-core-safety/requirements.md +++ b/.kiro/specs/raft-ha-core-safety/requirements.md @@ -1,42 +1,102 @@ # Requirements Document -## Project Description (Input) +## Introduction -**raft-ha-core-safety** — Fix the pre-existing HA-core consistency and convergence failures that the live failover soak (`xce-test-ha`) reproduces under repeated/chaotic primary failover. This work is **independent of, and a gate that must close before, the raft-drive-async production flag-flip**. +**raft-ha-core-safety** fixes the pre-existing HA-core consistency, convergence, and liveness failures that the live failover soak (`xce-test-ha`) reproduces under repeated and chaotic primary failover on an XCE controller cluster. Operators and clients rely on Raft failover to keep committed jobs safe and the cluster available; today, under primary churn, the cluster can silently diverge committed state, fail to recover a healthy quorum, or stall electing a new primary. -### Who has the problem -Operators and clients of an XCE controller HA cluster relying on Raft failover to keep committed jobs safe and the cluster available. Under repeated/chaotic primary churn the cluster can silently diverge committed state and/or fail to recover a healthy quorum. +All three symptoms reproduce on the pre-`raft-drive-async` baseline (commit `21e26a1`), confirmed by a baseline-vs-current soak comparison, so they are pre-existing HA-core defects independent of the (complete and dormant) `raft-drive-async` async-drive/offload work. This spec is the gate that must close before `raft-drive-async` flips its dispatch flags ON in production. -### Current situation (all baseline-reproduced) -All three symptoms reproduce on the **pre-raft-drive-async baseline (commit `21e26a1`)** — confirmed by a baseline-vs-current failover-soak comparison — so they are **NOT** caused by the raft-drive-async async-drive/offload work (which is complete and dormant). They are pre-existing HA-core bugs: +The defects, in protocol terms: -1. **Committed-prefix divergence (bug-851, bug-795/828 family).** Under chaotic failover, `walcompare` reports `FAIL: committed-prefix divergence at LSN N` — one member is **MISSING a committed WAL entry that the majority holds**, and keeps a divergent committed prefix silently. Contributing root causes already identified: WAL `hdr.term` left `0` (entry term never set from the writing leader's term), two-leaders-per-term windows, and no per-entry `(term,lsn)` identity or reconcile so a member cannot detect/repair a divergent committed prefix. The bug-795/828 family (committed-job/prefix loss under repeated and total-outage failover) is part of this cluster. +1. **Committed-prefix divergence** (bug-851; bug-795/828 family). Under chaotic failover, `walcompare` reports a committed-prefix divergence: one member is missing a committed log entry the majority holds, and silently retains a divergent committed prefix. Known contributors: entry term left at `0` instead of the writing leader's term; two-leaders-per-term windows; and no per-entry commit identity or reconcile path, so a member cannot detect or repair a divergent committed prefix. +2. **Standby-rejoin / reconvergence failure** (bug-1042 family). A restarted, previously-downed node intermittently fails to return the cluster to a healthy 3-of-3 within bound, even after reaching the leader's log tip at the replication-match level, due to post-recovery log-length/applied-state desync and catch-up apply gaps. +3. **Round-N election stall.** After several rapid kill/restart rounds the cluster intermittently fails to elect a new primary after a primary kill within the failover window — a liveness regression under churn. -2. **Standby-rejoin / reconvergence failure (bug-1042 family).** A restarted, previously-downed node intermittently fails to bring the cluster back to a healthy **3-of-3** within bound — even when it reaches the leader's log tip at the raft `match` level — due to post-recovery `log_len`/applied-state desync and catch-up apply gaps. +Behavior is verified against `raft/raft.tla` and the controller TLA+ safety-invariant checklist in `src/controller/CLAUDE.md` (Election Safety, Leader Completeness, State Machine Safety, Commit Ordering/Durability). -3. **Round-N election stall.** After several rapid kill/restart rounds the cluster intermittently fails to elect a new primary after a primary kill (no `CTRL_PRIMARY` at `term > prev` within the failover window) — a liveness regression under churn. +## Boundary Context -### What should change -Make repeated/chaotic failover **provably safe and live**: - -- **Safety — committed entries never diverge across members.** Enforce a real per-entry `(term, lsn)` commit identity plus a reconcile path so a member can **detect and repair** a divergent committed prefix rather than silently keeping it. Ensure the WAL entry term is always the writing leader's term (never `0`). Close two-leaders-per-term windows. -- **Convergence — a restarted node always reconverges to 3-of-3.** Deterministic post-recovery `log_len`/applied sync and catch-up apply completeness. -- **Liveness — elections always complete under churn.** No permanent election stall after repeated rapid failover. - -### Out of scope -The raft-drive-async async-drive/offload work (SlowLane, peer-control vtask dispatch, snapshot/config-push offload, endpoint-quiescence) — separate, complete, and dormant. This spec must not change its behavior. - -### Deliverables -- A **deterministic reproduction**: extend the `xce-test-ha` soak and add a unit/integration harness that forces the committed-prefix divergence and the reconverge / election-stall paths (RED before fix). -- The **`(term, lsn)` commit-identity + reconcile** mechanism. -- The **recovery/convergence** fixes (post-recovery log/applied sync, catch-up apply completeness) and the **election-liveness-under-churn** fix. -- A **green 6+round chaotic-failover soak on a real fs (release build)** with **zero committed-prefix divergence and zero job loss**. - -### Acceptance gate -This spec is the gate that must close before raft-drive-async flips its dispatch flags ON in production. - -### Verification reference -Verify all changes against `raft/raft.tla` and the controller TLA+ safety-invariant checklist in `src/controller/CLAUDE.md` (Election Safety, Leader Completeness, State Machine Safety, Commit Ordering/Durability). +- **In scope**: per-entry commit identity and divergence detect/repair; entry term always reflecting the writing leader's term; single-leader-per-term enforcement; deterministic standby-rejoin reconvergence to 3-of-3; election liveness under repeated churn; a deterministic reproduction harness (soak extension + unit/integration) that goes RED before the fix; and a green multi-round chaotic-failover soak as the acceptance gate. +- **Out of scope**: the `raft-drive-async` async-drive/offload work (SlowLane, peer-control vtask dispatch, snapshot/config-push offload, endpoint-quiescence). This spec must not change its behavior. New cluster-membership-resize, multi-region, or non-Raft replication features are also out of scope. +- **Adjacent expectations**: the cluster runs the existing 3-node Raft controller HA topology and persists committed state to a real filesystem. This spec expects `xce-test-ha` and the controller test harness to be the verification surface; it does not own the async dispatch path or the client-facing API contract beyond preserving committed-job safety. ## Requirements - + +### Requirement 1: Committed-prefix safety across members + +**Objective:** As an operator of an XCE controller HA cluster, I want every member to agree on the contents of the committed log prefix, so that no committed job is ever silently lost or diverged under failover. + +#### Acceptance Criteria +1. While the cluster is operating under repeated or chaotic primary failover, the Controller HA subsystem shall keep the committed log prefix identical across all members up to each member's commit point. +2. When a member holds a committed entry at a given log position, the Controller HA subsystem shall guarantee no other member holds a different committed entry at that same position. +3. If a member's committed prefix differs from the majority-agreed committed prefix at any position, then the Controller HA subsystem shall treat that member's committed state as divergent rather than authoritative. +4. The Controller HA subsystem shall satisfy the State Machine Safety and Commit Ordering/Durability invariants in the controller TLA+ safety checklist for every committed entry. + +### Requirement 2: Per-entry commit identity + +**Objective:** As an operator, I want each log entry to carry an unambiguous identity, so that members can distinguish entries written by different leaders and terms at the same position. + +#### Acceptance Criteria +1. When a leader writes a log entry, the Controller HA subsystem shall stamp that entry with the leader's current term. +2. The Controller HA subsystem shall never persist a committed entry whose term is `0` or otherwise does not identify the writing leader's term. +3. When two entries occupy the same log position, the Controller HA subsystem shall be able to determine from their identity whether they are the same entry or conflicting entries from different leaders. +4. While restoring persisted log entries after restart, the Controller HA subsystem shall preserve each entry's original term identity. + +### Requirement 3: Divergence detection and repair + +**Objective:** As an operator, I want a member with a divergent committed prefix to detect and repair it, so that the cluster self-heals instead of silently carrying a permanent inconsistency. + +#### Acceptance Criteria +1. When a member rejoins or replicates and its committed prefix conflicts with the majority-agreed prefix, the Controller HA subsystem shall detect the conflict. +2. When a committed-prefix conflict is detected, the Controller HA subsystem shall repair the divergent member's log to match the majority-agreed committed prefix. +3. If a member cannot repair its divergent committed prefix, then the Controller HA subsystem shall surface the unrecoverable divergence rather than continue serving divergent committed state. +4. The Controller HA subsystem shall not discard or overwrite any entry that is part of the majority-agreed committed prefix during repair. + +### Requirement 4: Single leader per term + +**Objective:** As an operator, I want at most one leader to be able to commit entries in any given term, so that conflicting writes cannot enter the committed prefix. + +#### Acceptance Criteria +1. The Controller HA subsystem shall allow at most one member to act as committing primary within any single term. +2. If a member detects another leader in the same or a higher term, then the Controller HA subsystem shall require that member to stop committing entries in its current term. +3. The Controller HA subsystem shall satisfy the Election Safety and Leader Completeness invariants in the controller TLA+ safety checklist across all failover transitions. + +### Requirement 5: Standby-rejoin reconvergence to 3-of-3 + +**Objective:** As an operator, I want a restarted node to deterministically rejoin and restore a full healthy quorum, so that the cluster regains its fault tolerance after recovery. + +#### Acceptance Criteria +1. When a previously-downed node restarts and reconnects, the Controller HA subsystem shall bring the cluster to a healthy 3-of-3 membership within the bounded recovery window. +2. When a rejoining node reaches the leader's log tip at the replication-match level, the Controller HA subsystem shall also bring that node's applied state and log length into agreement with the leader. +3. If a rejoining node has a catch-up apply gap, then the Controller HA subsystem shall complete the outstanding apply work before reporting the node as healthy. +4. While a node is rejoining, the Controller HA subsystem shall not report the cluster as fully converged until that node's committed and applied state matches the leader. + +### Requirement 6: Election liveness under churn + +**Objective:** As an operator, I want elections to always complete after a primary failure, so that the cluster never gets permanently stuck without a primary under repeated failover. + +#### Acceptance Criteria +1. When the current primary is killed and a quorum of members remains reachable, the Controller HA subsystem shall elect a new primary at a higher term within the bounded failover window. +2. While the cluster undergoes repeated rapid kill/restart rounds, the Controller HA subsystem shall continue to elect a new primary after each primary loss. +3. If an election round fails to produce a primary, then the Controller HA subsystem shall start a further election rather than remain permanently without a primary. + +### Requirement 7: Deterministic reproduction harness + +**Objective:** As a developer, I want a deterministic harness that forces the divergence, reconvergence, and election-stall paths, so that each fix is proven by a test that fails before it and passes after. + +#### Acceptance Criteria +1. The reproduction harness shall force the committed-prefix divergence path and fail (RED) on the unfixed baseline. +2. The reproduction harness shall force the standby-rejoin reconvergence-failure path and fail (RED) on the unfixed baseline. +3. The reproduction harness shall force the round-N election-stall path and fail (RED) on the unfixed baseline. +4. When run against the fixed system, the reproduction harness shall pass (GREEN) for the divergence, reconvergence, and election-liveness paths. +5. The reproduction harness shall extend the existing `xce-test-ha` soak and add unit/integration coverage for the same paths. + +### Requirement 8: Acceptance soak gate + +**Objective:** As a release owner, I want a green multi-round chaotic-failover soak on a real filesystem before this spec is accepted, so that `raft-drive-async` may flip its dispatch flags ON only after committed-job safety is proven. + +#### Acceptance Criteria +1. When a chaotic-failover soak of at least 6 rounds runs against a release build on a real filesystem, the Controller HA subsystem shall report zero committed-prefix divergence. +2. When that soak runs, the Controller HA subsystem shall report zero committed-job loss. +3. While the soak runs, the Controller HA subsystem shall reconverge to 3-of-3 and elect a new primary after every primary kill round. +4. The Controller HA subsystem shall not be accepted as the `raft-drive-async` production flag-flip gate until the soak passes all of the above. diff --git a/.kiro/specs/raft-ha-core-safety/research.md b/.kiro/specs/raft-ha-core-safety/research.md new file mode 100644 index 00000000..736362a3 --- /dev/null +++ b/.kiro/specs/raft-ha-core-safety/research.md @@ -0,0 +1,130 @@ +# Gap Analysis — raft-ha-core-safety + +_Generated by `/kiro-validate-gap`. Information over decisions: options and findings, not final choices._ + +## Headline Finding (reframes the spec) + +The `(term, lsn)` commit-identity + reconcile + commit-safety machinery the requirements describe **already exists and is wired at the baseline**. Confirmed: + +- **Library half is 100% complete and validated**: `raft-log-replication` (24/24, G1/G2/G3/G5) and `raft-commit-safety` (18/18, G4 Figure-8 + G6 vote freshness). All five callbacks present in `src/lib/raft/raft.{c,h}`. +- **Controller adapter is 16/20 wired** by `ha-term-safety`: all five callbacks implemented in `s_real_cbs` (`ha.c:1480/1539/1656/1711/1739`), primary append routed through `raft_log_append` (`ha.c:3408+`), heartbeat clean (no `on_persist_entry` on `n_entries==0`), per-LSN term index (`persist.c:414/476/487/752`), config markers, and `walcompare` (`src/controller/testsuite/walcompare.c`). +- **Baseline `21e26a1` already contains all of it.** `git`: `21e26a1` is itself a `raft-drive-async` task-2.3 commit, ancestor of HEAD, and contains `s_real_cbs` + `walcompare`. So the machinery is NOT absent at baseline. + +**Yet the live chaotic-failover soak still fails on this branch AND on baseline `21e26a1`**: `walcompare FAIL: committed-prefix divergence at LSN 9` (round 3) and standby `ctrl-2` rejoin failure (round 4), with intermittent round-N election stall. + +**Therefore raft-ha-core-safety is NOT a build-the-machinery spec. It is a residual-defect + convergence/liveness + live-verification spec.** The work is: root-cause why divergence survives the existing reconcile path under overlapping chaotic failover; close the convergence and election-liveness gaps no completed spec actually closed; and finish the live chaotic soak that `ha-term-safety` left permanently BLOCKED on env. **Do not rebuild the `(term,lsn)`/callback machinery — extend and correct it.** + +## Requirement-to-Asset Map + +| Req | Capability | Existing asset | Gap tag | +|-----|-----------|----------------|---------| +| R1 Committed-prefix safety | quorum commit + Figure-8 guard | `raft_advance_commit` raft.c:769-821; `on_commit_advanced` ha.c:1739 | **Constraint** — machinery present but live soak still diverges at LSN 9 → residual defect | +| R2 Per-entry commit identity | term stamped from leader; per-LSN term index; legacy-0→1 | raft.c:571-580; `persist_set_term` persist.c:476; `record_entry_term` persist.c:414/752; `WAL_LEGACY_TERM` persist.c:456 | **Done (verify)** — confirm no path still emits term-0 under chaos | +| R3 Divergence detect + repair | logOk + conflict-truncate-then-persist on AE intake | raft.c:701-765; `on_truncate_from` ha.c:1711; `walcompare` detector | **Constraint** — repair fires on AE-intake path only; rejoin/catch-up path may bypass it (see Research Needed #1) | +| R4 Single leader per term | term++ on election, vote-once, step-down, pre-vote, vote freshness | raft.c:105-249; `raft_cb_vote_eligible` ha.c:342 | **Constraint** — two-leaders-per-term window under churn not yet proven closed live | +| R5 Standby rejoin → 3-of-3 | catch-up apply, applied-lsn floor, snapshot backfill, contiguity guard | `ha_apply_wal_catchup_entry` ha.c:2233; `ha_advance_applied_lsn` ha.c:2078; `ha_handle_snapshot_chunk` ha.c:2492; contiguity persist.c:839 | **Missing (effective)** — bug-1042 standby-never-converges has NO dedicated owning task; `ha-convergence-recovery` claims it but soak still fails round 4 | +| R6 Election liveness under churn | election restart, candidate advertise floor | `raft_start_election` raft.c:105; `ha_candidate_advertise_lsn` ha.c:3921 | **Missing (effective)** — round-N stall not closed; no deterministic churn test | +| R7 RED→GREEN harness | `xce-test-ha` soak, `walcompare`, `controller_unit_test`, `test_ha_dispatch_integration` | `.claude/skills/xce-test-ha/`; `walcompare.c`; `src/xce.build/controller/controller_unit_test` | **Missing** — no deterministic harness that *forces* chaotic divergence/rejoin/stall as a RED unit/integration gate (current soak is probabilistic) | +| R8 Acceptance soak gate | real-fs release soak; BIN/XT overridable | `run-ha-soak.sh` (BIN/XT env override added this branch); non-ASAN release build | **Unknown** — never passed at 6+ chaotic rounds; was BLOCKED on overlayfs `fdatasync` (bug-924) | + +## Existing-Code Constraints (must respect) + +- **Reconcile is single-seam**: divergence repair happens only inside `raft_on_append_entries_request` (logOk → `on_truncate_from` → re-persist, raft.c:730-765). Any code path that brings a node's log forward *without* routing through this function (e.g. a snapshot/catch-up stream that writes WAL directly) will not detect or repair a divergent committed prefix. +- **Truncate must never cut the majority-committed prefix** (raft.c monotonic commit; promotion truncate-clamp `max(safe_committed_lsn, durable_tip)` ha.c:999-1000, bug-1060 fix). R3.4 depends on this clamp holding under chaos. +- **Persist-before-ACK durability is owned by `ha-commit-durability`, not this spec.** R1/R2 must not weaken it. `on_commit_advanced` is a barrier trigger, not the durability mechanism. +- **`raft-drive-async` is out of scope and dormant.** Do not change SlowLane / peer-control dispatch / endpoint-quiescence behavior (Boundary Context). +- **Client-side bug-854** (libxce primary discovery) can fail the soak harness at EXIT-1 even when server data safety passes. Must be isolated so it does not mask/forge R8 verdicts. + +## Implementation Approach Options + +### Option A — Extend & correct existing machinery (recommended) +Keep all library + adapter machinery. Root-cause the residual divergence, route the rejoin/catch-up path through the reconcile seam, close the convergence + liveness gaps with targeted fixes, add a deterministic chaotic harness, finish the live soak. +- ✅ No rebuild of consensus-critical code that already passed library + clean-failover validation +- ✅ Smallest blast radius; aligns with TLA+ already-proven invariants +- ❌ Requires precise root-cause of a hard-to-repro chaotic defect before fixing +- ❌ Risk of "fix the symptom not the invariant violation" if harness stays probabilistic + +### Option B — New convergence/divergence-repair subsystem +Build a standalone post-failover reconcile/repair component separate from the AE-intake seam. +- ✅ Clean place to own rejoin reconcile + unrecoverable-divergence fencing +- ❌ Duplicates logic that already lives in `raft_on_append_entries_request`; two reconcile paths is exactly the class of bug being fixed +- ❌ High risk of new two-leaders/two-reconcile divergence + +### Option C — Hybrid (Option A + new deterministic harness component) +Extend/correct in place (A), but build the **deterministic chaotic harness** (R7) as genuinely new test components (forced overlapping kills, injected divergent prefix, scripted round-N stall) rather than relying on the probabilistic soak. +- ✅ Keeps consensus code as A; isolates the new surface to test infrastructure where new code is safe +- ✅ Gives a RED gate that pins each fix to an invariant violation +- ❌ Harness design is itself non-trivial (deterministic chaos, no sleeps, gate/probe seams like `test_ha_dispatch_integration.c`) + +## Effort & Risk + +| Item | Effort | Risk | Justification | +|------|--------|------|---------------| +| Root-cause residual LSN-9 divergence | M | High | Chaotic, intermittent; survives existing reconcile — invariant violation is subtle | +| Route rejoin/catch-up through reconcile seam (R3/R5) | M | High | Touches recovery + commit prefix; must not cut majority-committed | +| Election liveness under churn (R6) | M | Medium | Election machinery exists; needs churn-stall root cause + restart guarantee | +| Deterministic chaotic harness (R7) | L | Medium | New, but test-only; pattern exists (`test_ha_dispatch_integration.c`) | +| Live 6+round real-fs soak (R8) | M | High | Was permanently blocked on overlayfs `fdatasync` (bug-924); needs real-fs env + bug-854 isolation | +| **Total** | **L–XL** | **High** | Consensus correctness under chaos + env-gated live verification | + +## Research Needed (carry to design phase) + +1. **Why does divergence survive the reconcile seam?** Trace the rejoin/catch-up path (`ha_apply_wal_catchup_entry` ha.c:2233, snapshot `ha.c:2492`): does it route through `raft_on_append_entries_request` so logOk/`on_truncate_from` fire, or does it write WAL directly and bypass conflict repair? Hypothesis: catch-up streams `[watermark..committed]` directly (per memory bug-795/828 family) → no `(term,lsn)` conflict check on rejoin. +2. **Is LSN-9 divergence in the committed or uncommitted prefix?** If committed, the violation is upstream (two-leaders-per-term or premature commit-advance), not a repair gap. Use `walcompare` `(lsn→term,pcrc)` output to classify. +3. **Two-leaders-per-term under overlapping kills** — does pre-vote + vote-once fully close the window when kills overlap election timeouts? Map against `raft/raft.tla` Election Safety. +4. **bug-1042 ownership** — `ha-convergence-recovery` claims the rejoin path but soak still fails round 4. Determine whether its tasks are actually landed on this branch or stranded, to avoid duplicate work. +5. **R8 env** — confirm real-fs (ext4) soak runs now via `run-ha-soak.sh` BIN/XT override + non-ASAN release build; quantify how bug-924 (overlayfs) and bug-854 (client discovery) are excluded from the R8 verdict. + +## Recommendation for Design Phase + +- **Preferred: Option C** (extend/correct in place + new deterministic chaotic harness). Treat the spec as residual-defect + convergence + liveness + live-verification, explicitly **not** a machinery rebuild. +- **Key design decisions to make**: (a) route rejoin/catch-up through the single reconcile seam vs. add a guarded repair at promotion; (b) classify LSN-9 as committed vs uncommitted before choosing repair vs upstream fix; (c) harness determinism mechanism (forced-kill gates/probes) for R7; (d) how R8 isolates bug-854/bug-924 so the gate measures only server-side committed-job safety. +- **Carry forward**: Research Needed #1–#5; verify every change against `raft/raft.tla` (Election Safety, Leader Completeness, State Machine Safety, Commit Ordering/Durability) per `src/controller/CLAUDE.md`. + +--- + +# Design-Phase Discovery (kiro-spec-design) + +## ROOT CAUSE (term-blind catch-up bypass) — primary hypothesis, harness-confirmable + +Deep-trace of the standby catch-up/rejoin paths found a **split** where one branch bypasses the single reconcile seam: + +| Path | Entry | Routes through `raft_on_append_entries_request`? | logOk | conflict-truncate | term stamped | +|------|-------|---|---|---|---| +| 1 Term-carrying | `entry->term != 0` → `ha_apply_term_entry_via_library` (ha.c:2173) → `ha_consume_append_entries` (ha.c:2383) → raft.c:646 | YES | YES (raft.c:701) | YES (raft.c:735) | leader term via `persist_set_term` | +| 2 **Term-blind (defect)** | `entry->term == 0` → `ha_apply_single_entry` (ha.c:2097) → `persist_wal_write_replicated` (persist.c:802) DIRECT | **NO** | **NO** | **NO** | **standby's own `p->term`** (persist.c:862) | +| 3 Snapshot | `ha_handle_snapshot_chunk` (ha.c:2492) → `persist_load_snapshot` sets applied_lsn=checkpoint | NO | N/A | N/A | N/A (canonical state) | +| 4 Stream+ACK | `ha_stream_wal_to_standby` (config_repl.c:2032) feeds Path 1 or 2 by `entry->term` | depends | depends | depends | depends | +| 5 `ha_rejoin_reconcile` | ha_convergence.c:254 — **now a no-op stub** (committed-watermark truncate retired; relies on library) | NO | N/A | retired | N/A | + +**Divergence mechanism (LSN 9):** +1. An entry reaches a standby via Path 2 (term-blind). `persist_wal_write_replicated` stamps `hdr.term = standby p->term` (wrong) and records it (`record_entry_term`, persist.c:902). No logOk, no conflict check. +2. The correct term-carrying entry for the same LSN later arrives via Path 1. The persist **idempotency guard** (`if lsn+1 <= p->current_lsn return 0`, persist.c:822) **silently no-ops** — the divergent entry is never overwritten/repaired. +3. `walcompare` (testsuite/walcompare.c:135) compares `(term, payload_crc)` per committed LSN; the member with the term-blind write shows a different `term` → `WALCMP_DIVERGENT`. `match_term` maps raw 0 → `WAL_LEGACY_TERM`, so legacy-0 vs genuine-term at the same LSN reads as divergence. + +**Why the existing `(term,lsn)` machinery does not catch it:** all conflict detection lives *inside* `raft_on_append_entries_request`; Path 2 never enters it, and the idempotency guard treats "LSN already present" as "done" regardless of term. + +## Synthesis + +- **Generalization (D1):** R1/R2/R3/R4 are one underlying invariant — *every* entry that enters a member's log must pass the single `(term,lsn)` reconcile seam. The fix is not four mechanisms but **eliminating every bypass of the one seam** + making persistence `(term,lsn)`-conflict-aware rather than LSN-only-idempotent. +- **Build vs adopt (D2):** Adopt the existing library seam (`raft_on_append_entries_request` + `on_truncate_from` + `persist_set_term`) — already validated (raft-log-replication/raft-commit-safety). Build nothing new in the consensus core; only route into it and harden the persist guard. Reuse `ha_rejoin_reconcile` intake hook (currently stub) rather than a second reconcile path. +- **Simplification (D3):** Do not add a separate "divergence repair subsystem" (gap-analysis Option B rejected) — that would create the exact second-reconcile-path class of bug being fixed. Collapse Path 2 into Path 1. +- **Harness-first (D4):** The live bug is intermittent/chaotic. Make it deterministic: a RED unit test injects a term-blind/divergent entry at an LSN and asserts the seam repairs it; a chaotic soak mode forces overlapping kills. RED before any fix (R7), pins each fix to a `walcompare`/invariant violation. + +## Open risks carried to tasks +- Confirm Path 2 is the *only* divergence source live (harness RED test will confirm/refute; if soak still diverges after Path-2 closure, classify next via `walcompare (term,lsn,pcrc)`). +- Snapshot boundary (Path 3): post-install AE intake must reconcile against the snapshot's `(term,lsn)`; verify no committed entry below the checkpoint can diverge. +- Standby reconvergence (R5) and election-liveness (R6) are separable seams from divergence-repair; phaseable but kept in one spec per the unified acceptance gate. +- bug-854 (client discovery) and bug-924 (overlayfs fdatasync) must be excluded from the R8 verdict so the gate measures server-side committed-job safety only. + +--- + +# Design Review Resolution (kiro-validate-design) + +Three critical issues raised and folded into design.md (GO): + +1. **Repair-vs-clamp conflation (correctness).** Split truncate into two contracts: *promotion-truncate* keeps the bug-1060 `max(safe_committed_lsn, durable_tip)` clamp; *follower-AE-conflict-truncate* obeys Raft Log Matching and truncates the conflicting suffix regardless of local `durable_tip` (a conflicting entry was never majority-committed → safe to cut). Applying the promotion clamp on the follower path would have converted repairable divergence into a permanent fence and failed R8. Updated: §Existing Architecture, §Unified Entry Ingest, §Persist Conflict Guard, §Error Handling, Traceability 3.x, Testing. +2. **R4 two-leaders-per-term unverified.** Added a falsifiable Election Safety assertion (≤1 committing-primary per term under overlapping kills, mapped to raft.tla); failure promotes a concrete adapter-gate guard from verification to required work. Updated: §Election Liveness, Testing Integration. +3. **Hypothesis-validation risk.** RED gate (7.1) must reproduce the *live* `walcompare` divergence and bisect to a seam before synthetic `(term,lsn)` injection counts; baseline 21e26a1 already retired live term-0 (ha-term-safety 3.4), so the term-blind path may not be the live trigger. Contingency: if soak still diverges after Path-2 closure, re-classify via `walcompare (term,lsn,pcrc)` and add a fix task — do not mark R1/R8 done on green unit tests alone. Updated: §Deterministic Chaotic Harness, Testing E2E + RED-first. + +Verdict: **GO** (unconditional after edits). Architecture unchanged; refinements were bounded contract clarifications. diff --git a/.kiro/specs/raft-ha-core-safety/spec.json b/.kiro/specs/raft-ha-core-safety/spec.json index 02e98542..709be73f 100644 --- a/.kiro/specs/raft-ha-core-safety/spec.json +++ b/.kiro/specs/raft-ha-core-safety/spec.json @@ -1,22 +1,22 @@ { "feature_name": "raft-ha-core-safety", "created_at": "2026-06-27T03:15:23Z", - "updated_at": "2026-06-27T03:15:23Z", + "updated_at": "2026-06-27T04:30:00Z", "language": "en", - "phase": "initialized", + "phase": "tasks-generated", "approvals": { "requirements": { - "generated": false, - "approved": false + "generated": true, + "approved": true }, "design": { - "generated": false, - "approved": false + "generated": true, + "approved": true }, "tasks": { - "generated": false, - "approved": false + "generated": true, + "approved": true } }, - "ready_for_implementation": false + "ready_for_implementation": true } diff --git a/.kiro/specs/raft-ha-core-safety/tasks.md b/.kiro/specs/raft-ha-core-safety/tasks.md new file mode 100644 index 00000000..09d1bfe2 --- /dev/null +++ b/.kiro/specs/raft-ha-core-safety/tasks.md @@ -0,0 +1,132 @@ +# Implementation Plan: raft-ha-core-safety + +> **TDD (project steering, mandatory):** every implementation sub-task is preceded by a testsuite sub-task that lands failing (RED) tests first, then implements to GREEN. The deterministic harness (Phase 1) is the RED infrastructure that pins every later fix to a `walcompare`/invariant violation. +> **No `(P)` markers:** Phases 2–6 all modify `ha.c` (and `persist.c`), and the RED cases share `test_ha_core_safety.c`, so the plan is sequential — concurrent edits would contend on the same files (same constraint as `ha-convergence-recovery`). Foundation Phase 1 establishes shared prerequisites and is also sequential. +> **Correctness oracle:** verify every change against `raft/raft.tla` and the invariant checklist in `src/controller/CLAUDE.md` (Election Safety, Leader Completeness, State Machine Safety, Commit Ordering/Durability). +> **Verify anchors before editing** (files drift): the design and `research.md` carry 2026-06-27 file:line anchors; re-grep before each edit. +> **Hypothesis discipline:** the term-blind bypass is the primary hypothesis, not proven sole cause. Task 1.3 must reproduce the *live* divergence first; a green synthetic unit test over a still-red soak does not satisfy 1.1/8.1. + +- [ ] 1. Foundation: deterministic chaotic harness and live-divergence reproduction +- [ ] 1.1 Extend the divergence detector to a machine-checkable per-member report + - Extend `walcompare` to emit a parseable per-member `(lsn -> term, payload-crc, present/MISSING)` report over the committed prefix, and classify each divergence as committed vs uncommitted + - Observable completion: `walcompare` prints a parseable per-member report and a committed-vs-uncommitted classification; the existing soak still detects the round-3 LSN divergence through it + - _Requirements: 7.1, 7.5_ + - _Boundary: Deterministic Chaotic Harness_ + +- [ ] 1.2 Add a chaotic soak mode with an isolated verdict + - Add an overlapping-kill mode to the soak driver (kills landing inside election windows, not only sequential kill→wait→rejoin); produce a verdict that counts `walcompare` divergences and committed-job loss while excluding client-discovery (bug-854) and overlayfs `fdatasync` (bug-924) failures from the divergence/job-loss result + - Observable completion: the soak runs chaotic mode against real-fs release binaries and emits a verdict whose divergence/job-loss counts are unaffected by bug-854/bug-924 + - _Requirements: 7.5, 8.4_ + - _Boundary: Acceptance Soak Gate, Deterministic Chaotic Harness_ + +- [ ] 1.3 Reproduce and classify the live divergence, then bisect to a seam + - Run the chaotic soak to capture the actual `walcompare` committed-prefix divergence (RED), classify it via `(term, lsn, pcrc)` (which member, which term, committed vs uncommitted), and bisect it to the concrete ingest path; confirm or refute the term-blind-bypass hypothesis + - Observable completion: a recorded RED divergence with member/term/lsn classification and the identified seam, written to `research.md`; the hypothesis is confirmed or a new trigger is named before any fix + - _Requirements: 7.1, 1.1_ + - _Boundary: Deterministic Chaotic Harness_ + +- [ ] 1.4 Stand up the unit/integration harness infrastructure + - Create `test_ha_core_safety.c` with the deterministic gate/probe seams (no sleeps; teardown drains pending delaymgr tasks to avoid the bug-1224 teardown-race UAF), the `(term, lsn)`/apply-gap/kill-injection fixtures, and register it in the controller unit-test build; include one smoke-RED case proving the harness can observe a divergence. The per-defect RED bodies are owned by their phases (2.3 ingest, 4.1 reconvergence, 5.1 election) — this task provides the shared seams they build on, not those cases + - Observable completion: `test_ha_core_safety` builds and runs, the injection fixtures and probe seams are usable by later phases, and the smoke-RED case FAILs on the baseline with no ASAN teardown errors + - _Requirements: 7.1, 7.4_ + - _Boundary: Deterministic Chaotic Harness_ + +- [ ] 2. Committed-prefix divergence fix: persistence conflict guard and single-seam ingest +- [ ] 2.1 RED: persistence `(term, lsn)`-conflict tests + - Add cases: a wrong-term entry written at an LSN followed by the correct term-carrying entry must end with the leader's term (today the LSN-only idempotency guard silently no-ops); a conflicting entry below local `durable_tip` must be truncatable; a matching entry must never be truncated + - Observable completion: the new persistence cases FAIL on the baseline because the guard is LSN-only and silently skips the corrective write + - _Requirements: 1.1, 1.2, 2.1, 2.2, 3.1, 3.4_ + - _Boundary: Persist Conflict Guard_ + +- [ ] 2.2 Make the persistence guard `(term, lsn)`-conflict-aware + - Replace the LSN-only idempotency check: same LSN + same term is an idempotent no-op; same LSN + different term is surfaced as a conflict for the seam to truncate-then-repersist (never a silent no-op); never persist a non-leader term for a live-replicated entry; preserve persist-before-ACK and the term index + - Observable completion: the 2.1 cases pass; the WAL at a re-written LSN holds exactly the leader's `(term, payload)`; persist-before-ACK behavior unchanged + - _Requirements: 1.1, 1.2, 2.1, 2.2, 3.1_ + - _Boundary: Persist Conflict Guard_ + +- [ ] 2.3 RED: single-seam ingest and unrecoverable-fence tests + - Add cases asserting that every replicated/catch-up entry — including a `term==0`-shaped legacy entry — passes through the reconcile seam (logOk + conflict-truncate + leader-term stamp) and is reconciled, with no direct-persist bypass branch remaining; and a case where a node that cannot obtain the majority-agreed committed prefix takes the loud non-promotable fence and refuses promotion instead of serving divergent committed state + - Observable completion: the ingest cases FAIL on the baseline because the term-blind branch bypasses the seam; the fence case FAILs because no unrecoverable-divergence fence exists yet + - _Requirements: 1.2, 3.2, 3.3, 7.1_ + - _Boundary: Unified Entry Ingest_ + +- [ ] 2.4 Collapse the term-blind ingest path into the single reconcile seam + - Route batch/single/catch-up ingest through `raft_on_append_entries_request`; retire the direct-persist branch; apply the follower-AE-conflict-truncate contract (logOk-gated, no promotion clamp) so a conflicting entry is repaired regardless of local `durable_tip`; surface genuinely-unrecoverable divergence as a loud non-promotable fence (never self-terminate); defer file I/O/truncation to delaymgr (never the callback/iomgr thread) + - Observable completion: 2.3 passes (including the fence case — an unrepairable node refuses promotion and stays alive retrying); an injected divergent `(term, lsn)` is repaired through the seam; `walcompare` is clean on the 1.3 reproduction path + - _Requirements: 1.1, 1.2, 1.3, 2.1, 2.3, 3.1, 3.2, 3.3, 3.4_ + - _Boundary: Unified Entry Ingest_ + - _Depends: 2.2_ + +- [ ] 3. Snapshot-boundary reconciliation +- [ ] 3.1 RED: snapshot post-install reconcile test + - Add a case where a divergent pre-snapshot entry must be reconciled against the installed checkpoint's `(term, lsn)` boundary (today snapshot install sets applied-lsn without a `(term, lsn)` reconcile) + - Observable completion: the case FAILS on the baseline because no `(term, lsn)` reconcile occurs at the snapshot boundary + - _Requirements: 3.1, 5.1_ + - _Boundary: Snapshot Boundary Reconcile_ + +- [ ] 3.2 Establish the snapshot `(term, lsn)` boundary + - Make snapshot install establish a verifiable `(term, lsn)` at the checkpoint so post-install AE intake reconciles correctly and no committed entry below the checkpoint can diverge + - Observable completion: 3.1 passes; a member that installed a snapshot matches the leader's `(term, payload-crc)` at and below the checkpoint boundary + - _Requirements: 3.1, 5.1_ + - _Boundary: Snapshot Boundary Reconcile_ + - _Depends: 2.4_ + +- [ ] 4. Standby reconvergence completeness +- [ ] 4.1 RED: reconvergence-gate test + - Add a case where a rejoining node with an injected catch-up apply gap must not be reported converged/healthy until `applied_lsn` and `log_len` equal the leader's committed boundary (today the node is healthy at match-index alone) + - Observable completion: the case FAILS on the baseline because convergence is reported before applied-state agreement + - _Requirements: 5.1, 5.2, 5.3, 5.4, 7.2_ + - _Boundary: Standby Reconvergence_ + +- [ ] 4.2 Gate healthy/3-of-3 on committed-and-applied agreement + - Synchronize post-recovery `log_len`/applied-lsn and complete outstanding catch-up apply (routed through the seam, reusing the rejoin intake hook) before reporting a node converged; bound the recovery window + - Observable completion: 4.1 passes; a rejoining node reports converged only after `applied_lsn == log_len == leader committed`, restoring a healthy 3-of-3 + - _Requirements: 5.1, 5.2, 5.3, 5.4_ + - _Boundary: Standby Reconvergence_ + - _Depends: 2.4_ + +- [ ] 5. Election liveness and safety under churn +- [ ] 5.1 RED: election liveness and Election Safety tests + - Add cases: repeated rapid kill/restart rounds must yield a higher-term primary each round; a scripted failed election round must be followed by another election; across overlapping kills, at most one member ever reaches committing-primary per term (Election Safety, mapped to `raft/raft.tla`) + - Observable completion: the liveness cases FAIL on the baseline (round-N stall); the Election Safety assertion runs and reports per-term primary counts + - _Requirements: 4.1, 4.3, 6.1, 6.2, 6.3, 7.3_ + - _Boundary: Election Liveness_ + +- [ ] 5.2 Guarantee election completion without losing single-leader safety + - Ensure a failed election round triggers a further election (no permanent stall while quorum is reachable); pair step-down-on-higher-term with election restart; preserve at-most-one-committing-primary-per-term + - Observable completion: 5.1 liveness cases pass with no permanent stall across rapid rounds; the Election Safety assertion still holds (no two committing primaries in a term) + - _Requirements: 4.1, 4.2, 4.3, 6.1, 6.2, 6.3_ + - _Boundary: Election Liveness_ + +- [ ] 6. Integration: single-seam invariant end-to-end +- [ ] 6.1 Route live leader replication and catch-up streaming through the seam only + - Ensure the leader-side streamer always emits term-carrying entries (no `term==0` on the wire from a live leader) and the walAck catch-up continuation feeds only the seam ingest path + - Observable completion: live leader replication carries the leader's term end-to-end; no term-0 entries reach a standby from a live leader; streamed catch-up entries reconcile through the seam + - _Requirements: 2.1, 2.3, 3.2_ + - _Boundary: Unified Entry Ingest, Persist Conflict Guard_ + - _Depends: 2.4_ + +- [ ] 6.2 Verify the single-seam invariant and restart identity + - Assert no ingest path (live replication, catch-up batch, snapshot post-install, rejoin) writes the WAL outside the reconcile seam, and that each entry's `(term, lsn)` identity survives a restart/recovery + - Observable completion: an integration test confirms every ingest path reconciles through the seam and that entry terms are preserved across restart; no direct-persist bypass remains + - _Requirements: 1.1, 1.2, 2.4, 3.1_ + - _Depends: 2.4, 3.2, 4.2_ + +- [ ] 7. Validation: regression and acceptance soak gate +- [ ] 7.1 Full controller suite under ASAN with the new harness green + - Build and run the full controller unit suite under ASAN (suite-by-suite if the aggregate hits the known 300s cap); fix any regression from the ingest/persist/election changes + - Observable completion: the full suite is green under ASAN and every `test_ha_core_safety` case passes with no ASAN errors + - _Requirements: 7.4_ + - _Depends: 6.2, 5.2_ + +- [ ] 7.2 Acceptance chaotic-failover soak gate + - Run the chaotic soak (≥6 rounds, overlapping kills) against real-fs release binaries and assert zero `walcompare` divergence, zero committed-job loss, and 3-of-3 reconvergence plus a new primary after every kill round, with bug-854/bug-924 isolated from the verdict + - Observable completion: the soak exits PASS over ≥6 chaotic rounds with zero divergence and zero job loss — the green result that gates the `raft-drive-async` production flag-flip + - _Requirements: 1.4, 7.4, 8.1, 8.2, 8.3, 8.4_ + - _Depends: 6.2, 5.2_ + +- [ ] 7.3 Hypothesis-validation closeout + - Confirm the live soak divergence is eliminated by the implemented fixes (not by synthetic unit tests alone); if any residual divergence remains, classify it via `walcompare (term, lsn, pcrc)`, record the new finding in `research.md`, and add a follow-up fix task rather than declaring closure + - Observable completion: the soak divergence count is zero with closure justified by the live soak, OR a recorded new finding plus a follow-up task; `raft/raft.tla` invariants (Election Safety, Leader Completeness, State Machine Safety, Commit Ordering) reviewed against the final behavior + - _Requirements: 1.1, 1.4, 7.1, 8.1_ + - _Depends: 7.2_