docs(raft-ha-core-safety): retarget Phase 2 to cross-term payload loss (post-1.3)
Task 1.3 refuted the term-blind hypothesis; real defect = cross-term catch-up payload loss (streamer ships PLEN=0 on short pread/malloc-fail; (term,lsn)-only guard is payload-blind). Retarget: - design: new Catch-up Payload Integrity component (config_repl.c core fix); Persist guard -> (term,lsn,payload-crc); term-blind demoted to defensive; flows/traceability/testing/hypothesis sections updated. - tasks: Phase 2 retargeted (streamer fail-loud + payload-aware guard); new 1.5 (release-log-buffer soak robustness); 7.2 _Depends 1.5_. - research: old root-cause + synthesis D1/D3 marked SUPERSEDED. Independent sanity review PASS after adding the 1.5 dependency edge.
This commit is contained in:
parent
21ce4c4f4a
commit
9e55f09274
3 changed files with 120 additions and 75 deletions
|
|
@ -1,10 +1,10 @@
|
|||
# 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.
|
||||
> **No `(P)` markers:** Phases 2–6 modify shared files (`ha.c`, `persist.c`, `config_repl.c`) and the RED cases share `test_ha_core_safety.c`, so the plan is sequential — concurrent edits would contend (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.
|
||||
> **Root cause (task 1.3, DONE + verified):** the term-blind hypothesis is **refuted**. The real defect is **cross-term catch-up payload loss** — `ha_stream_wal_to_standby` silently ships `PLEN=0` on a short `pread`/`malloc`-fail, and the `(term,lsn)`-only persist guard is payload-blind. Phase 2 targets the streamer + payload-aware guard (`research.md` §"Task 1.3"). A green synthetic unit test over a still-red soak does not satisfy 1.1/8.1 — closure requires the real-fs chaotic soak (7.2) clean.
|
||||
|
||||
- [ ] 1. Foundation: deterministic chaotic harness and live-divergence reproduction
|
||||
- [x] 1.1 Extend the divergence detector to a machine-checkable per-member report
|
||||
|
|
@ -31,32 +31,41 @@
|
|||
- _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
|
||||
- [ ] 1.5 Make soak primary-detection robust to the release-build log buffer
|
||||
- Task 1.3 found the release build block-buffers its `--logfile` and flushes only on signal, so the soak's log-grep primary/convergence detection cannot see an elected primary and false-aborts. Switch the soak's primary + convergence detection to a client probe (`ctrlstatus`/`xce_test`) instead of log-grep, so the chaotic soak runs unattended on release binaries. (Only if a client probe is insufficient, fall back to a controller release log-flush change that does not alter production logging semantics.)
|
||||
- Observable completion: `run-ha-soak.sh --chaotic` on the `~/tmp` real-fs path with the `src/build.release` binaries detects the elected primary each round via the client probe and no longer false-aborts on "no primary within Ns"; a short (2-round) run reaches its verdict line
|
||||
- _Requirements: 7.5, 8.3_
|
||||
- _Boundary: Acceptance Soak Gate, Deterministic Chaotic Harness_
|
||||
- _Depends: 1.2_
|
||||
|
||||
- [ ] 2. Committed-prefix divergence fix: cross-term catch-up payload integrity + payload-aware guard
|
||||
- _Root cause (task 1.3, verified): the cross-term catch-up streamer silently ships `PLEN=0` on a short `pread`/`malloc`-fail, and the `(term,lsn)`-only persist guard is payload-blind. Term and ingest-path are correct — the payload is lost._
|
||||
- [ ] 2.1 RED: payload-aware persistence guard tests
|
||||
- Add cases: an empty-payload (`PLEN=0`) entry written at an existing LSN N followed by the correct full-payload entry must end with the FULL payload (today the `(term,lsn)`-only idempotency guard silently no-ops the corrective write); the reverse (full then empty at the same `(term,lsn)`) must NOT degrade the stored entry to empty; an identical `(term, payload-crc)` re-write is a safe idempotent no-op
|
||||
- Observable completion: the new persistence cases FAIL on the baseline because the guard compares only `(term, lsn)` and cannot see the payload difference
|
||||
- _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_
|
||||
- [ ] 2.2 Make the persistence guard `(term, lsn, payload-crc)`-aware
|
||||
- Extend the LSN-only idempotency check (persist.c:822): identical `(term, payload-crc)` at an LSN is an idempotent no-op; a differing payload-crc (or term) at an existing LSN is surfaced for repair, never a silent `return 0`; never accept a `PLEN=0` write as the durable form of a known-non-empty entry; preserve persist-before-ACK, the term index, and `WAL_LEGACY_TERM`
|
||||
- Observable completion: the 2.1 cases pass; an empty-over-real write at an existing committed LSN no longer silently survives; identical re-writes stay idempotent
|
||||
- _Requirements: 1.1, 1.2, 2.1, 2.2, 2.4, 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_
|
||||
- [ ] 2.3 RED: cross-term catch-up streamer payload-integrity tests
|
||||
- Add a case reproducing the live signature: a cross-term catch-up (stored term < leader current term, bug-1067 path) whose entry-body `pread` returns short / `malloc` fails must NOT result in a persisted `PLEN=0` entry at a committed LSN — instead the stream must error and retry. Also add the defensive case that a `term==0`-shaped catch-up entry routes through the reconcile seam (no direct-persist bypass), and the unrecoverable-fence case (a node that cannot obtain the majority-agreed committed prefix refuses promotion instead of serving divergent state). Instrument `payload_len` at send and at `on_persist_entry` to disambiguate send-side-empty vs transitive-empty propagation
|
||||
- Observable completion: the streamer case FAILs on the baseline because the short-read/malloc-fail path silently ships `PLEN=0`; the seam-routing and fence cases FAIL because those paths/fence do not exist yet
|
||||
- _Requirements: 1.1, 1.2, 2.1, 3.1, 3.2, 3.3, 7.1_
|
||||
- _Boundary: Catch-up Payload Integrity, Unified Entry Ingest_
|
||||
- _Depends: 2.2_
|
||||
|
||||
- [ ] 2.4 Fix the streamer payload integrity + defensive seam routing + fence
|
||||
- Core: in `ha_stream_wal_to_standby` (config_repl.c ~2154-2163), fail loud on a short `pread`/`malloc`-fail with a bounded retry instead of freeing the buffer and shipping `PLEN=0`; a non-empty entry is never transmitted empty. Disambiguate and handle send-side-empty vs transitive-empty propagation per the 2.3 instrumentation. Defensive: retire the term-blind `ha_apply_single_entry` direct-persist branch so all ingest routes through the seam; surface genuinely-unrecoverable divergence as a loud non-promotable fence (never self-terminate); defer file I/O to delaymgr (never the callback/iomgr thread)
|
||||
- Observable completion: 2.3 passes (streamer never ships empty; seam-routing and fence cases green); on the 1.3 reproduction path (real-fs release chaotic soak) `walcompare` reports zero committed divergence at the previously-divergent LSNs
|
||||
- _Requirements: 1.1, 1.2, 1.3, 2.1, 2.3, 3.1, 3.2, 3.3, 3.4_
|
||||
- _Boundary: Catch-up Payload Integrity, Unified Entry Ingest_
|
||||
- _Depends: 2.2, 2.3_
|
||||
|
||||
- [ ] 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)
|
||||
|
|
@ -99,11 +108,11 @@
|
|||
- _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
|
||||
- [ ] 6.1 Verify leader→standby replication carries (term, payload) faithfully end-to-end
|
||||
- Integration check that the live leader replication AND the cross-term catch-up streaming path transmit each entry's full `(term, payload)` to the receiver: no `term==0` on the wire from a live leader (defensive), and — the real concern — no non-empty entry ever arrives with `PLEN=0`; the walAck catch-up continuation feeds only the seam ingest path
|
||||
- Observable completion: an end-to-end test (or instrumented soak round) shows leader and standby hold identical `(term, payload-crc)` for every replicated and cross-term-caught-up LSN; `walcompare --report` clean across members
|
||||
- _Requirements: 2.1, 2.3, 3.2_
|
||||
- _Boundary: Unified Entry Ingest, Persist Conflict Guard_
|
||||
- _Boundary: Catch-up Payload Integrity, Unified Entry Ingest_
|
||||
- _Depends: 2.4_
|
||||
|
||||
- [ ] 6.2 Verify the single-seam invariant and restart identity
|
||||
|
|
@ -123,7 +132,7 @@
|
|||
- 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_
|
||||
- _Depends: 1.5, 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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue