docs(raft-ha-core-safety): document term-blind bypass safe-by-guard + pin restart identity (task 6.2)
The term-blind ha_apply_single_entry direct-persist branch (carried forward from task 2.4) is unreachable with a real non-zero term in any current production path: its only trigger, raft_send_append_entries, has zero production call sites (library simulator only). Both real senders (repl_broadcast, ha_stream_wal_to_standby) always emit a non-zero term for a present LSN. Documents this in ha.c and design.md rather than structurally retiring a dormant path (YAGNI), with an explicit flag for future re-evaluation if a heartbeat-driver task ever wires up raft_send_append_entries. Adds two regression pins: restart preserves (term,lsn) identity (replay_wal already reconstructs the term index via record_entry_term), and real senders never produce the term-blind wire shape. test_ha_core_safety 28/0, no regression, no ASAN reports.
This commit is contained in:
parent
4e1e3e07fb
commit
9344b8ed60
4 changed files with 338 additions and 8 deletions
|
|
@ -235,11 +235,11 @@ A node is reported healthy/3-of-3 only when committed **and** applied state matc
|
|||
#### Unified Entry Ingest
|
||||
| Field | Detail |
|
||||
|-------|--------|
|
||||
| Intent | Every replicated/catch-up entry passes `raft_on_append_entries_request` (logOk + `(term,lsn)` conflict + leader-term stamp). Defensively retire the term-blind direct-persist branch so no path can bypass the seam. |
|
||||
| Intent | Every replicated/catch-up entry passes `raft_on_append_entries_request` (logOk + `(term,lsn)` conflict + leader-term stamp). The term-blind direct-persist branch is closed by reachability, not by removal — task 6.2 traced every production sender and found it unreachable with real data; documented SAFE-BY-GUARD rather than structurally retired (see task 6.2's Implementation Note below). |
|
||||
| Requirements | 1.2, 2.3, 3.1, 3.2, 3.4 |
|
||||
|
||||
**Responsibilities & Constraints**
|
||||
- Defensive: remove the `entry->term == 0` direct-persist branch (`ha_apply_single_entry`); route batch/single/catch-up ingest through the seam. (Not the cause of the observed divergence, but a latent bypass hazard.)
|
||||
- Defensive: the `entry->term == 0` direct-persist branch (`ha_apply_single_entry`) is NOT structurally removed (task 6.2 decision). Every real sender (`repl_broadcast` for live replication, `ha_stream_wal_to_standby` for catch-up) always supplies `n_entries>=1` with a non-zero term, so this branch is unreachable with production data; the sole function that could ever drive it (`repl_send_append_entries`) is invoked only by the raft library's `raft_send_append_entries`, which has zero production callers today. It remains documented and guarded in `ha.c`, not retired. (Not the cause of the observed divergence, but a latent bypass hazard — now shown to be dormant, not live.)
|
||||
- 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).
|
||||
|
||||
|
|
@ -253,8 +253,8 @@ A node is reported healthy/3-of-3 only when committed **and** applied state matc
|
|||
- 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 an entry through the term-blind branch and asserts it now routes through the seam and is reconciled.
|
||||
- Integration: reuse `ha_apply_term_entry_via_library`/`ha_consume_append_entries` for every reachable ingest path.
|
||||
- Validation (task 6.2, closing task 2.4's deferral): rather than retiring `ha_apply_single_entry`'s direct-persist path, task 6.2 traced every production sender of a `wal_catchup_entry_t` message and found the term-blind branches (the batch-mode `sub->term==0` fallthrough and the unconditional `entry->n_entries==0` single-entry mode in `ha_apply_wal_catchup_entry`) unreachable with real data: `repl_broadcast` always sends `n_entries==1` with the real stamped term; `ha_stream_wal_to_standby` always batches `n_entries>=1` and sources each sub-entry's term via `persist_entry_term`, whose legacy-zero rule maps a stored term of 0 to `WAL_LEGACY_TERM` for any present LSN (never 0). The only function that could emit the term-blind shape, `repl_send_append_entries`, is driven solely by the raft library's `raft_send_append_entries`, which has zero production call sites (only the pure library simulator and an `XCE_TESTING` test helper call it). Decision: **document safe-by-guard, do not structurally retire** — retiring would mean building a term-0-safe seam variant for a path with no live trigger, which is speculative work the evidence does not warrant. The disposition and reachability trace are recorded as comments at `ha_apply_single_entry` and both term-blind branches in `ha.c`, and pinned by `test_2_3_termblind_route_payload_guarded`, `test_2_3_unrepairable_catchup_raises_nonpromotable_fence`, and `test_6_2_real_senders_never_produce_termblind_shape` (`test_ha_core_safety.c`).
|
||||
- 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
|
||||
|
|
|
|||
|
|
@ -188,7 +188,7 @@
|
|||
- _Boundary: Catch-up Payload Integrity, Unified Entry Ingest_
|
||||
- _Depends: 2.4_
|
||||
|
||||
- [ ] 6.2 Verify the single-seam invariant and restart identity
|
||||
- [x] 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. **Carry-forward from 2.4:** the term-blind `ha_apply_single_entry` direct-persist branch was NOT structurally retired in 2.4 (its empty-over-full safety is enforced by the 2.2 payload guard and pinned by `test_2_3_termblind_route_payload_guarded`/`test_2_3_unrepairable_catchup_raises_nonpromotable_fence`). This task must either structurally retire that bypass (route term-0 catch-up through the seam, with its own RED first) OR formally document it as safe-by-persist-guard and adjust the invariant wording accordingly
|
||||
- Observable completion: an integration test confirms every ingest path reconciles through the seam and that entry terms are preserved across restart; the term-blind bypass is either removed (no direct-persist path remains) or documented safe-by-guard with the pin tests cited
|
||||
- _Requirements: 1.1, 1.2, 2.4, 3.1_
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue