From 04b9d1ea5325e41f25a6d60b6f92a8904ec23818 Mon Sep 17 00:00:00 2001 From: pingqiu Date: Wed, 29 Apr 2026 17:15:38 -0700 Subject: [PATCH] sw-block/design: relocate V3 recovery dev docs from seaweed_block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per repository policy: dev/design docs live in seaweedfs/sw-block/design/, not in seaweed_block/docs/. Formal product docs come later. This commit relocates the 7 recovery design markdown docs (4 trunk-merged in seaweed_block phase-15; 3 in-flight on g7-redo branches) plus the 1 hardware canonical YAML to sw-block/design/ with v3-recovery-* prefix to match the existing naming pattern (v3-recovery-live-line-backlog-spec.md). Companion cleanup: a follow-on PR on seaweed_block removes the docs from docs/ (and the YAML from testrunner/scenarios/) — that PR is the seaweed_block side of the relocation. Files added: v3-recovery-pin-floor-wire.md — was docs/recovery-pin-floor-wire.md on seaweed_block phase-15 (PR #11+#16) v3-recovery-wiring-plan.md — was docs/recovery-wiring-plan.md (PR #13) v3-recovery-execution-institution.md — was docs/recovery-execution-institution.md v3-recovery-inv-test-map.md — was docs/recovery-inv-test-map.md (PR #11/#14/#15) v3-recovery-unified-wal-stream-kickoff.md — was docs/recovery-unified-wal-stream-kickoff.md g7-redo/unified-wal-kickoff (v0.3) v3-recovery-unified-wal-stream-mini-plan.md — was docs/recovery-unified-wal-stream-mini-plan.md g7-redo/unified-wal-mini-plan (v0.2) v3-recovery-dual-lane-canonical-runbook.md — was docs/recovery-dual-lane-canonical-runbook.md g7-redo/hardware-canonical-paper v3-recovery-dual-lane-canonical.yaml — was testrunner/scenarios/recovery-dual-lane-canonical.yaml g7-redo/hardware-canonical-paper Internal cross-references updated in-place via sed: - docs/recovery-inv-test-map.md → v3-recovery-inv-test-map.md - docs/recovery-pin-floor-wire.md → v3-recovery-pin-floor-wire.md - docs/recovery-wiring-plan.md → v3-recovery-wiring-plan.md - testrunner/scenarios/recovery-dual-lane-canonical.yaml → v3-recovery-dual-lane-canonical.yaml Hand-edits: - runbook §1 companion-YAML link: was "../v3-recovery-dual-lane-canonical.yaml" (parent dir from seaweed_block/docs); now same-directory link in design/. - runbook §8 §3.2 #3 reference: was relative to seaweed_block memory file (../../.claude/...); rewritten to point to v3-recovery-unified-wal-stream-kickoff.md §4 directly. - mini-plan Q15: docs/archive/ wording updated to sw-block/design/archive/. Stages-of-evidence still readable from the docs themselves (kickoff §11, mini-plan §10 resolution logs, inv-test-map row versions). Original seaweed_block branches preserve git history for the in-flight content; the cleanup PR closes them once this lands. NOTE: this commit does NOT include the user's unrelated ongoing edits in feature/sw-block (M v3-batch-process.md, M v3-dev-roadmap.md, M v3-phase-15-g6-mini-plan.md, etc.). Those stay uncommitted for the user to handle separately. --- ...v3-recovery-dual-lane-canonical-runbook.md | 170 ++++++++ .../v3-recovery-dual-lane-canonical.yaml | 280 +++++++++++++ .../v3-recovery-execution-institution.md | 124 ++++++ sw-block/design/v3-recovery-inv-test-map.md | 134 ++++++ sw-block/design/v3-recovery-pin-floor-wire.md | 238 +++++++++++ .../v3-recovery-unified-wal-stream-kickoff.md | 295 +++++++++++++ ...3-recovery-unified-wal-stream-mini-plan.md | 394 ++++++++++++++++++ sw-block/design/v3-recovery-wiring-plan.md | 184 ++++++++ 8 files changed, 1819 insertions(+) create mode 100644 sw-block/design/v3-recovery-dual-lane-canonical-runbook.md create mode 100644 sw-block/design/v3-recovery-dual-lane-canonical.yaml create mode 100644 sw-block/design/v3-recovery-execution-institution.md create mode 100644 sw-block/design/v3-recovery-inv-test-map.md create mode 100644 sw-block/design/v3-recovery-pin-floor-wire.md create mode 100644 sw-block/design/v3-recovery-unified-wal-stream-kickoff.md create mode 100644 sw-block/design/v3-recovery-unified-wal-stream-mini-plan.md create mode 100644 sw-block/design/v3-recovery-wiring-plan.md diff --git a/sw-block/design/v3-recovery-dual-lane-canonical-runbook.md b/sw-block/design/v3-recovery-dual-lane-canonical-runbook.md new file mode 100644 index 000000000..23d9cfe76 --- /dev/null +++ b/sw-block/design/v3-recovery-dual-lane-canonical-runbook.md @@ -0,0 +1,170 @@ +# Dual-lane recovery canonical — runbook + +**Status**: paper artifact. Live execution waits for the G7-redo train (#11..#15) to merge to trunk (`main` or `phase-15`, whichever the team uses as the integration target). Architect 2026-04-29: "实测 在 主干含 train 之后". This runbook is the "你们定义的 canonical" — it ships ahead so the run is one-script-away the moment trunk includes the train. + +**Companion YAML**: [`v3-recovery-dual-lane-canonical.yaml`](v3-recovery-dual-lane-canonical.yaml) (same directory) + +## 1. What is "canonical"? + +Architect-defined canonical = **one green end-to-end run that exercises every PR in the G7-redo train against a real binary**. + +Concretely: + +| PR | Behavior surfaced by canonical | +|---|---| +| **#11** (POC + memorywal) | Sender/Receiver session lifecycle — `frameSessionStart` / `frameBaseDone` / `frameBarrierResp` round-trip on the dual-lane port | +| **#12** (BaseBatchAck + pin_floor) | `frame 0x07 frameBaseBatchAck` flows from replica; `coord.SetPinFloor` advances on primary | +| **#13** (transport+cmd wiring) | `--recovery-mode=dual-lane` reaches the binary; per-replica dual-lane listener bound on `ctrl-port + 1` | +| **#14** (recycle gate consumes MinPin) | Under active session, `walstore.persistCheckpoint` clamps at `min(pin_floor_active_sessions)` — checkpoint does NOT advance past pinned LSNs | +| **#15** (PinUnderRetention bypass) | Engine retry path observably skips when `FailureKind=PinUnderRetention`; `Attempts` counter does NOT increment; `PublishDegraded` emitted instead of Start* re-emit | + +One green run on **rf2-localhost** = engineering-side trust closure. One green run on **rf2-rdma** (m01 + M02 with 25 Gbps RoCE link, per memory) = production-side trust closure. **rf3-localhost** is an optional nice-to-have for multi-pin coverage. + +## 2. Pre-conditions before triggering the canonical + +| Check | Required state | +|---|---| +| Trunk includes #11..#15 | `git log trunk --oneline` shows commit shas of all 5 train PRs (or their squash-merge equivalents) | +| Binary rebuilt from trunk | `cmd/blockvolume --version` reports a build sha matching trunk tip; otherwise behavior under test is stale | +| `--recovery-mode=dual-lane` honored | startup log emits `"G7-redo recovery-mode=dual-lane"` and `"G7-redo-2.5 recycle gate installed"` | +| Test infra clear of prior state | no leftover `blockmaster` / `blockvolume` PIDs; `${artifact_dir}` empty so log scraping is unambiguous | +| (rf2-rdma only) RDMA link healthy | `ibstat` reports both endpoints active; `iperf` between m01 (10.0.0.1) and M02 (10.0.0.3) ≥ 20 Gbps line rate | + +If any pre-condition fails: do NOT run the canonical. Fix the pre-condition first; the canonical's value is its trust signal, and a flaky pre-condition makes the signal noise instead of trust. + +## 3. Topology — three variants + +### 3.1 rf2-localhost + +Single-host smoke. No network — both replicas on `127.0.0.1`. Useful for CI or local dev validation. + +``` +Host: any (developer workstation; CI runner) +Master: 127.0.0.1:7000 +Primary: data=127.0.0.1:7041 ctrl=127.0.0.1:7141 dual-lane=7142 +Replica: data=127.0.0.1:7042 ctrl=127.0.0.1:7143 dual-lane=7144 +``` + +### 3.2 rf2-rdma — production trust variant + +Two-host RDMA setup per `MEMORY.md` test infrastructure. + +``` +Host m01: 192.168.1.181 (RDMA 10.0.0.1, NIC rocep1s0f0) +Host M02: 192.168.1.184 (RDMA 10.0.0.3, NIC mlx5_0) +SSH user: testdev +Remote: /opt/work/sra (existing rsync target) + +Master on m01: 192.168.1.181:7000 +Primary on m01: data=10.0.0.1:7041 ctrl=10.0.0.1:7141 dual-lane=7142 +Replica on M02: data=10.0.0.3:7042 ctrl=10.0.0.3:7143 dual-lane=7144 +``` + +Note: dual-lane port = `ctrl-addr port + 1` per architect Option A separation. The binary's `replica_barrier.go` handles binding both `ctrl` and `ctrl+1`. + +### 3.3 rf3-localhost — optional multi-pin coverage + +Three replicas, single host. Useful only to demonstrate that `MinPinAcrossActiveSessions()` reports the lowest of multiple concurrent sessions' pins, NOT that recycle-gate works (which is already the rf2 demonstration). + +## 4. Run procedure + +For each variant in `variant_matrix`: + +```bash +# 1. Build trunk binary +cd ~/work/seaweed_block +go build -o ${ARTIFACT_DIR}/bin/blockmaster ./cmd/blockmaster +go build -o ${ARTIFACT_DIR}/bin/blockvolume ./cmd/blockvolume + +# 2. (rf2-rdma only) deploy to m01 + M02 +ssh testdev@m01 "mkdir -p /opt/work/sra/seaweed_block/bin" +ssh testdev@M02 "mkdir -p /opt/work/sra/seaweed_block/bin" +scp ${ARTIFACT_DIR}/bin/blockmaster testdev@m01:/opt/work/sra/seaweed_block/bin/ +scp ${ARTIFACT_DIR}/bin/blockvolume testdev@m01:/opt/work/sra/seaweed_block/bin/ +scp ${ARTIFACT_DIR}/bin/blockvolume testdev@M02:/opt/work/sra/seaweed_block/bin/ + +# 3. Start packet capture on dual-lane port (audit artifact) +sudo tcpdump -i any -w ${ARTIFACT_DIR}/dual_lane.pcap port 7144 & +TCPDUMP_PID=$! + +# 4. Run scenario +sw-test-runner run v3-recovery-dual-lane-canonical.yaml \ + --variant=${VARIANT} \ + --artifact-dir=${ARTIFACT_DIR} \ + --topology=${TOPOLOGY_FILE} + +# 5. Stop capture, collect artifacts +sudo kill -TERM $TCPDUMP_PID +ls -la ${ARTIFACT_DIR} +``` + +**Capture window**: tcpdump runs across the full scenario so `dual_lane.pcap` shows the complete frame sequence — `frameSessionStart` (1) → `frameBaseBlock` (2) → `frameBaseDone` (3) → `frameWALEntry`/Kind=Backlog (4/1) → `frameBaseBatchAck` (7) interleaved → `frameWALEntry`/Kind=SessionLive (4/2) → `frameBarrierReq` (5) → `frameBarrierResp` (6). The pcap is the audit trail when the test report is the question. + +## 5. Pass / fail decision tree + +``` +All YAML phases PASS (excluding optional bypass phase)? +├─ YES: variant green for the run +│ ├─ rf2-localhost green only: +│ │ → engineering trust signal CLOSED. Architect notification: "canonical green on rf2-localhost; rf2-rdma scheduled." +│ ├─ rf2-localhost + rf2-rdma green: +│ │ → production trust signal CLOSED. Architect notification: "G7-redo train trust closure complete." +│ └─ + rf3-localhost green: +│ → multi-pin coverage demonstrated. Optional artifact for §3.2 #3 kickoff context. +└─ NO: variant FAILED + ├─ Investigate per phase that failed (logs + pcap + metrics timeline) + ├─ DO NOT cherry-pick "the bug" to a fix branch and re-run on the same trunk — + │ the canonical is the trust signal; flake-on-bug-fix-on-flake spirals undermine it. + ├─ File a bug PR targeting trunk (or one of the train PRs if pre-merge). + └─ Re-run canonical clean on the bug-fixed trunk. +``` + +**The canonical is binary** — green or not green. There is no "mostly green". A flake on phase 4 (`observe_pin_advancement`) is a real signal; do not silently rerun. + +## 6. Optional bypass phase substitution + +The phase `observe_retry_bypass_pin_under_retention` requires triggering a `PinUnderRetention` mid-session, which naturally requires retention to advance past a still-active pin. Two options: + +### 6.a Live-injection path (preferred when fault injection exists) + +Add a fault-injection flag to `cmd/blockvolume`: `--inject-recycle-bypass-pin-under-retention=true`. The flag patches `walstore.persistCheckpoint` to skip its gate check exactly once during a session, forcing the next `BaseBatchAck` to land below the (advanced) S boundary, generating a typed `recovery.FailurePinUnderRetention`. The canonical asserts the engine bypasses retry as documented. + +This requires a small **dev-only** wiring patch in the binary (gated behind a flag that defaults to off; never compiled into release binaries). Acceptable for canonical exercise; not acceptable as a long-term injection point. + +### 6.b Unit-test substitution (zero binary patching) + +Skip the live phase and pin the bypass behavior to the unit tests: + +| Test | Pinning | +|---|---| +| `TestT4d_PinUnderRetention_BypassesRetryBudget` (`core/engine/recovery_pin_retention_test.go`) | Engine retry path: Attempts unchanged, no Start* re-emit, PublishDegraded emitted. | +| `TestT4d_PinUnderRetention_AtBudgetEdge_DoesNotEscalate` (same) | Counter-pin against round-47 path. | +| `TestClassifier_RecoveryPinUnderRetention_MapsToEngineKind` (`core/transport/classifier_pin_under_retention_test.go`) | Boundary mapping closes silent fall-through. | + +The architect's "trust signal" framing ratifies this substitution: canonical = the train's behavior demonstrably correct on real binary; the bypass phase is one node in that train and its mechanism is fully unit-pinned. + +**Decision**: rf2-localhost runs the bypass phase via 6.a if the fault-injection flag landed; otherwise substitutes 6.b. rf2-rdma always substitutes 6.b (no fault injection on the trust-signal infra). rf3-localhost is identical to rf2-localhost. + +## 7. After the canonical lands green + +| Trigger | Action | +|---|---| +| rf2-localhost green | Notify architect; flip the architect priority list status — train trust signal CLOSED at engineering side. | +| rf2-rdma green | Notify architect; production trust signal CLOSED. Eligible to flip `--recovery-mode` default from `legacy` to `dual-lane` per Open Question 3 of the wiring plan (separate one-line PR). | +| Default flip merged | After 1 release cycle (per architect Open Question 4), legacy code path eligible for removal (`g7-redo/legacy-removal` branch). | +| Both variants green + default flipped | Engineering & ops both satisfied; G7-redo train fully delivered. §3.2 #3 implementation kickoff is the next binding action. | + +## 8. What this runbook does NOT cover + +- **§3.2 #3 (single-queue / sliding-window) coverage** — the canonical exercises the *current* dual-lane shape (sequential phases with Backlog→SessionLive transition gated by closeCh). After §3.2 #3 lands the gate is gone but **wire format is byte-identical**; the canonical re-runs without modification. Per [`v3-recovery-unified-wal-stream-kickoff.md`](v3-recovery-unified-wal-stream-kickoff.md) §4: only the kind byte's transition rule changes, not the framing. +- **Option B retry policy** — Option A (this train's PR #15) covers PinUnderRetention only. Option B (per-failure-kind budget + lossless recovery↔engine mapping + observability + #3 backoff) is a separate PR with its own canonical-prep cycle. +- **Long-running soak** — canonical is one full session. Long-running stability is operator-territory, scheduled separately on the production trust infrastructure. +- **Multi-volume / topology stress** — canonical runs against a single volume `v1`. Multi-volume stress is a separate hardening item. + +## 9. Open questions for architect + +| Q | Status | +|---|---| +| Is the architect's "你们定义的 canonical" satisfied by this scope, or should the bypass phase be live-only (no 6.b substitution)? | **Open** — assumes 6.b acceptable per architect's "测试一次, in 二进制" lower-bound framing | +| Does fault-injection-via-binary-flag (6.a) need a separate kickoff before adding to `cmd/blockvolume`? | **Open** — flag is dev-only / off-by-default; minimal blast radius. Architect ratification needed before binary patch lands. | +| Does rf3-localhost run as part of the canonical-green gate, or always optional? | **Provisionally optional** — runbook §1 treats it as nice-to-have; architect can promote to gate if multi-pin behavior needs hard pin | diff --git a/sw-block/design/v3-recovery-dual-lane-canonical.yaml b/sw-block/design/v3-recovery-dual-lane-canonical.yaml new file mode 100644 index 000000000..3443b0d08 --- /dev/null +++ b/sw-block/design/v3-recovery-dual-lane-canonical.yaml @@ -0,0 +1,280 @@ +# Hardware canonical for the G7-redo dual-lane train (#11..#15). +# +# Architect 2026-04-29: "信任状 — 最少 dual-lane + recycle-pin + retry-A +# 在二进制里跑一次 你们定义的 canonical". This file is the "你们定义的 canonical" +# half — the YAML shape. Live execution waits for the train to merge to +# trunk; the paper artifact ships ahead so the run is one-script-away +# the moment trunk includes #11..#15. +# +# Companion: docs/recovery-dual-lane-canonical-runbook.md +# +# Ledger pins (each phase asserts at least one): +# - INV-PIN-EXISTS-ONLY-DURING-SESSION +# - INV-PIN-COMPATIBLE-WITH-RETENTION +# - INV-PIN-ADVANCES-ONLY-ON-REPLICA-ACK +# - INV-RECYCLE-GATED-BY-MIN-ACTIVE-PIN +# - INV-SESSION-COMPLETE-CLOSURE +# +# This is a SCENARIO SHAPE document — the authoritative live binary +# runner is `cmd/blockvolume` + `cmd/blockmaster` + `cmd/m01verify` +# with the actions below scripted into a runbook. The Go-native replay +# (when written) lives at +# core/replication/component/dual_lane_canonical_replay_test.go and +# walks these phases against memorywal in-process. + +name: recovery-dual-lane-canonical +purpose: | + G7-redo train trust signal. Single end-to-end run that exercises + PR #11 (POC + memorywal substrate), PR #12 (BaseBatchAck + + pin_floor advancement), PR #13 (transport+cmd dual-lane wiring), + PR #14 (recycle gate consumes MinPin), PR #15 (PinUnderRetention + retry-budget bypass). One green run = trust-signal closure for + the train. + +precondition: + branch: trunk-includes-train + notes: | + Run requires #11..#15 merged to phase-15 (or main) — the binary + must already carry RecoveryFailurePinUnderRetention, the + dual-lane port code-path, and the substrate recycle gate. Running + this on an unmerged branch is explicitly out-of-scope per + architect 2026-04-29. + +variant_matrix: + # Two-replica is the minimum architect-defined canonical + # ("最少 dual-lane + recycle + retry-A"). Three-replica adds + # multi-pin coverage and is OPTIONAL — keep ordered list for + # explicit run targeting. + - rf2-localhost + - rf2-rdma # m01 + M02 with RDMA NIC; requires test infra + - rf3-localhost # OPTIONAL — multi-pin coverage + +phases: + - name: setup + actions: + - start: cmd/blockmaster + flags: + - --authority-store=${artifact_dir}/store + - --listen=${master_addr} + - --topology=${artifact_dir}/topology.yaml + - --t0-print-ready + - start: cmd/blockvolume + instance: primary + flags: + - --master=${master_addr} + - --server-id=s1 + - --volume-id=v1 + - --replica-id=r1 + - --data-addr=${primary_data_addr} + - --ctrl-addr=${primary_ctrl_addr} + - --recovery-mode=dual-lane # PR #13 + - start: cmd/blockvolume + instance: replica + flags: + - --master=${master_addr} + - --server-id=s2 + - --volume-id=v1 + - --replica-id=r2 + - --data-addr=${replica_data_addr} + - --ctrl-addr=${replica_ctrl_addr} + - --recovery-mode=dual-lane # PR #13 + - wait_until: both volumes visible in master topology view + - assert_log_line: + instance: primary + contains: "G7-redo recovery-mode=dual-lane" + - assert_log_line: + instance: primary + contains: "G7-redo-2.5 recycle gate installed" # PR #14 + + - name: initial_assignment + purpose: master assigns primary + replica; baseline writes commit on both + actions: + - wait_until: v1 has assigned authority (primary=r1, replica=r2) + - write_n_distinct_blocks: + target: primary + count: 16 + seed: deterministic + # baseline: replica must catch these via steady-state ship + - sync: { target: primary } + - wait_until: + condition: replica frontier R == primary frontier R + timeout: 5s + - snapshot: authority_timeline → artifact + + - name: trigger_dual_lane_rebuild + purpose: | + Take down replica; primary writes during the gap; replica + restarts; engine probes; rebuild session opens on dual-lane + port. Pin #11 (session opens) + #13 (dual-lane port active). + actions: + - signal: + target: replica + signal: SIGTERM # graceful — replica stops accepting + - wait_until: master observes replica probe-fail + - write_n_distinct_blocks: + target: primary + count: 32 # creates the gap that forces rebuild + seed: deterministic-2 + - sync: { target: primary } + - restart: replica # comes back; engine probes; gap detected + - assert_log_line: + instance: primary + contains: "rebuild start replica=r2" # transport rebuild_sender + - assert_log_line: + instance: primary + contains: "dual-lane" # PR #13 path + - assert_tcp_connection: + # Dual-lane port = ctrl-addr port + 1 (architect Option A) + src: primary + dst_port: ${replica_ctrl_port_plus_one} + timeout: 10s + + - name: observe_pin_advancement + purpose: | + Receiver emits BaseBatchAck (frame 0x07) periodically; + primary's coord.SetPinFloor advances. Pin #12. + actions: + - sample_metric: + # Periodic sample (every 100ms for 5s) of pin_floor on primary. + # Asserts strict-monotonic across consecutive non-zero samples. + source: primary + metric: dual_lane.pin_floor[r2] + interval: 100ms + duration: 5s + assert: monotonic_nondecreasing + - assert_metric_eventually: + source: primary + metric: dual_lane.pin_floor[r2] + predicate: ">= 16" # baseline (16 baseline + at least one ack into rebuild) + timeout: 10s + + - name: observe_recycle_gate_active + purpose: | + Under active session, primary's checkpoint MUST NOT advance + past pin_floor. Pin #14 (INV-RECYCLE-GATED-BY-MIN-ACTIVE-PIN). + Negative observation: even if primary writes more during the + session, checkpointLSN clamps at min(pin_floor_active_sessions). + actions: + - capture_metric: + source: primary + metric: walstore.checkpoint_lsn + as: checkpoint_pre + - capture_metric: + source: primary + metric: dual_lane.pin_floor[r2] + as: pin_floor_during + - write_n_distinct_blocks: + # Adds primary-side writes that would, absent the gate, + # let checkpoint advance. With the gate, checkpoint is + # clamped — that's the assertion below. + target: primary + count: 8 + seed: deterministic-during-session + - sync: { target: primary } + - capture_metric: + source: primary + metric: walstore.checkpoint_lsn + as: checkpoint_during + - assert: | + checkpoint_during <= pin_floor_during # the gate is doing its job + + - name: complete_session + purpose: barrier-ack returns; coord.EndSession; pin released; recycle resumes + actions: + - wait_until: + condition: replica frontier R == primary frontier H + timeout: 30s + - assert_log_line: + instance: primary + contains: "SessionClosedCompleted" + - assert_metric: + source: primary + metric: dual_lane.session_active[r2] + equals: 0 # session closed + - assert_metric: + source: primary + metric: dual_lane.pin_floor[r2] + equals: 0 # pin released (INV-PIN-EXISTS-ONLY-DURING-SESSION) + - sync: { target: primary } + - assert_metric_eventually: + source: primary + metric: walstore.checkpoint_lsn + predicate: ">= primary frontier H" # recycle resumes; checkpoint catches up + timeout: 5s + + - name: observe_retry_bypass_pin_under_retention + purpose: | + Pin #15 (Option A). Force a PinUnderRetention by injecting + an explicit retention-violation scenario; assert engine emits + PublishDegraded WITHOUT re-emitting Start*; assert + retry_attempts counter does NOT increment. + notes: | + Triggering PinUnderRetention naturally requires retention to + advance past a pin mid-session — possible via fault injection + (--inject-recycle-bypass=true on primary, or a manual flusher + override at the right moment). If fault injection is too + heavy-hand for the canonical, this phase MAY be skipped on + live hardware and pinned by the unit test + TestT4d_PinUnderRetention_BypassesRetryBudget instead. See + runbook §6 for the option matrix. + optional: true + actions: + - inject_fault: + target: primary + fault: recycle_bypass_pin_under_retention + duration: 1 + - wait_until: + condition: | + log line on primary contains + "FailureKind=PinUnderRetention — skip retry" + timeout: 10s + - assert_log_line: + instance: primary + contains: "PublishDegraded" + - assert_log_line: + instance: primary + NOT_contains: "StartCatchUp (retry)" # bypass = no Start* re-emit + NOT_contains: "StartRebuild (retry)" + - assert_metric: + source: primary + metric: engine.recovery.attempts[r2] + unchanged_from: last_session_end # Attempts MUST NOT bump + + - name: verify_final + actions: + - assert: no double-primary in authority timeline + - assert: no orphan assignment + - assert: replica r2 in InSync state for full session-closed-window + - assert: zero "FailureKind=Transport" lines after PinUnderRetention + (proves boundary mapping took the recovery.Failure path, + NOT the Transport fallback — PR #15 closes this) + +acceptance: + - all phases except `observe_retry_bypass_pin_under_retention` + PASS on each variant in `variant_matrix` (the bypass phase is + optional — runbook §6 documents the unit-test substitution) + - barrier-ack-driven completion (NOT shipper-liveness inferred) + pinned by `INV-LIVE-CAUGHT-UP-IFF-FRONTIER-AT-BARRIER` + - one full canonical run on rf2-localhost = engineering trust signal + closure for the train (architect's "你们定义的 canonical") + - rf2-rdma = production trust signal (canonical run on test + infrastructure described in runbook §3) + - rf3-localhost = nice-to-have multi-pin coverage; NOT a gate + +artifacts: + - blockmaster.log + - blockvolume-primary.log + - blockvolume-replica.log + - authority_timeline.txt + - dual_lane_metrics_timeline.json # pin_floor + checkpoint_lsn samples + - tcpdump-port-${replica_ctrl_port_plus_one}.pcap # dual-lane port traffic + +pins_ledger: + - INV-PIN-EXISTS-ONLY-DURING-SESSION + - INV-PIN-COMPATIBLE-WITH-RETENTION + - INV-PIN-ADVANCES-ONLY-ON-REPLICA-ACK + - INV-PIN-STABLE-WITHIN-SESSION + - INV-RECYCLE-GATED-BY-MIN-ACTIVE-PIN + - INV-SESSION-COMPLETE-CLOSURE + - INV-LIVE-CAUGHT-UP-IFF-FRONTIER-AT-BARRIER + - INV-SINGLE-FLIGHT-PER-REPLICA diff --git a/sw-block/design/v3-recovery-execution-institution.md b/sw-block/design/v3-recovery-execution-institution.md new file mode 100644 index 000000000..4bcc38788 --- /dev/null +++ b/sw-block/design/v3-recovery-execution-institution.md @@ -0,0 +1,124 @@ +# Recovery Execution Institution + +The recovery-execution institution is the bounded layer that turns +engine-issued recovery commands into real runtime lifecycle facts. +It owns session preparation, real execution start, cancellation, +timeout wiring, and terminal close callbacks. It does not own +recovery classification, failover decisions, topology authority, +or operator-facing publication. + +This doc names what the institution owns, what it does not, and +what is explicitly carried forward to later phases. + +## Files + +| File | Role | +|---|---| +| `core/adapter/adapter.go` | Command preparation, lifecycle callback ingress, single route from engine commands to runtime execution | +| `core/adapter/executor.go` | `CommandExecutor` contract, including `SetOnSessionStart` and `SetOnSessionClose` | +| `core/adapter/normalize.go` | Runtime lifecycle facts to engine events: `SessionPrepared`, `SessionStarted`, `SessionClosed*` | +| `core/engine/apply.go` | Semantic lifecycle reducer for prepare / start / fail / complete / invalidate | +| `core/transport/executor.go` | Runtime session registry, conn ownership, invalidation, start/close signaling | +| `core/transport/catchup_sender.go` | Catch-up execution path; emits start only after real execution path begins | +| `core/transport/rebuild_sender.go` | Rebuild execution path; same lifecycle rule as catch-up | + +## What the Institution Owns + +1. **Command admission into execution** — engine-issued `StartCatchUp`, + `StartRebuild`, and `InvalidateSession` become runtime actions + through one adapter route. +2. **Prepared versus running split** — `SessionPrepared` is emitted + when the adapter binds lineage and queues runtime work; `SessionStarted` + is emitted only when the runtime actually begins execution. +3. **Bounded start timeout** — a prepared session that never reaches + real execution is failed by the adapter's bounded start-timeout + watchdog instead of remaining indefinitely in `starting`. +4. **Lifecycle callbacks** — the executor reports real start and + terminal close through `SetOnSessionStart` and `SetOnSessionClose`. +5. **Session invalidation** — `InvalidateSession` removes the active + session, closes its conn, and prevents stale delayed callbacks from + becoming current truth. +6. **Fail-closed start / close behavior** — start failure, timeout, + or invalidation must not fabricate `running` or `healthy`. +7. **Runtime-local ownership only** — the executor may own conn state, + deadlines, and cancellation wiring, but not recovery policy. + +## What the Institution Does NOT Own + +The following belong to later phases. The recovery-execution institution +must not silently absorb them: + +| Concern | Owner | Why not here | +|---|---|---| +| Choosing catch-up vs rebuild | engine | semantic policy, not execution lifecycle | +| Widening `targetLSN` or rewriting recovery meaning | engine | execution must honor the frozen command contract | +| Cross-session retry policy and backoff strategy | later P10 / P12 work | broader lifecycle policy than the current bounded institution | +| Live WAL streaming during rebuild | P10 / P12 | needs richer lifecycle orchestration than the current single-run route | +| Multi-replica coordination | P12 | replicated contract, not one replica's runtime lifecycle | +| Promotion / failover selection | P14 | topology authority | +| Epoch minting / endpoint authority | P14 | topology authority produced above the executor | +| Operator-facing health / repair controls | P15 | governance surface | + +## Fail-Closed Contract + +The current recovery-execution institution proves these boundaries: + +| Scenario | Property proven | +|---|---| +| Session prepared but not yet started | projection stays `starting`; no fabricated `running` without a real start callback | +| Prepared session exceeds bounded start timeout | adapter emits `SessionClosedFailed(start_timeout)`; prepared does not hang forever | +| Immediate executor start error | adapter converts the failure into `SessionClosedFailed`; lifecycle fails closed without a `started` event | +| Delayed start after timeout | same-session `SessionStarted` is ignored once the session has already failed | +| Delayed success close after timeout | same-session `SessionClosedCompleted` is ignored; late success cannot revive a failed session | +| Delayed stale start callback after newer assignment | old `SessionStarted` is rejected; it cannot advance the new session | +| Delayed stale close callback after newer assignment | old `SessionClosed*` is rejected; it cannot make the new session healthy or failed | +| Transport dial failure | executor emits failed close but no `SessionStarted`; command issuance alone does not count as running | +| Session invalidation mid-run | invalidated session does not emit terminal close; runtime cancellation is semantically dead | + +The key truth split is: + +- `SessionPrepared` means the engine accepted one bounded recovery contract. +- `SessionStarted` means runtime execution actually began. +- `SessionClosedCompleted` / `SessionClosedFailed` are the only terminal lifecycle truth. + +## Watchdog Guard Note + +One non-blocking implementation note is important for future maintainers: + +1. the adapter watchdog's read of engine state before firing is advisory, not authoritative +2. after that pre-check, engine state may still move before the watchdog calls back into the adapter +3. the final authoritative guard remains the engine's phase checks in `applySessionFailed` + +This dual layer is intentional: + +1. the watchdog should suppress obviously stale timeout fires where it can +2. the engine must remain the last semantic guard against late timeout/start/close races + +Do not simplify this into one convenience check unless the replacement preserves +both properties explicitly. + +## Reading Order + +For someone new to this institution: + +1. `core/adapter/executor.go` — lifecycle contract surface +2. `core/adapter/adapter.go` — command preparation and callback ingress +3. `core/engine/apply.go` — semantic lifecycle reducer +4. `core/transport/executor.go` — session registry and invalidation +5. `core/transport/catchup_sender.go` and + `core/transport/rebuild_sender.go` — where real execution start is admitted +6. `core/adapter/adapter_test.go` and + `core/transport/transport_test.go` — lifecycle proof set + +## Carry-Forward + +Items the recovery-execution institution leaves explicitly to later phases: + +- **Later P10 work** — per-session timeout classification, retry/backoff policy, + richer execution-phase observability, and live-WAL-during-rebuild orchestration. +- **P12 (bounded replicated failover contract)** — rejoin contract, + multi-replica lifecycle coordination, and bounded takeover guarantees. +- **P14 (topology / failover policy)** — epoch authority, promotion, + failover selection, and replica placement truth. +- **P15 (operator-facing governance surface)** — operator controls for + retire / repair / rebalance and their publication semantics. diff --git a/sw-block/design/v3-recovery-inv-test-map.md b/sw-block/design/v3-recovery-inv-test-map.md new file mode 100644 index 000000000..e0b041396 --- /dev/null +++ b/sw-block/design/v3-recovery-inv-test-map.md @@ -0,0 +1,134 @@ +# Recovery INV ↔ Test Mapping (G7-redo POC) + +Each invariant declared by `core/recovery/doc.go` (or by +`sw-block/design/v3-recovery-live-line-backlog-spec.md`) maps below to +the test function(s) that pin it. File paths are repo-relative; +line numbers are the `func TestXxx` declaration line at branch +`g7-redo/dual-lane-recovery-poc` HEAD (commits `42f351d` / +`dc7f550` / `a5b50db`). + +Status legend: + +- **Layer 1** — receiver-side mechanism (`core/recovery/`, layer-1 files). +- **Layer 2** — primary-side orchestration (coordinator + bridge). +- **Layer 3** — error taxonomy. +- **Substrate** — `core/storage/memorywal` contract conformance. +- **Forward** — declared but deliberately untested in this PR; pending later milestone. + +--- + +## Layer 1 — receiver-side mechanism + +| INV | Definition | Test(s) | Status | +|---|---|---|---| +| `INV-DUAL-LANE-WAL-WINS-BASE` | Same LBA: once WAL lane has applied (`bitmap.MarkApplied`), base lane MUST skip subsequent base blocks for that LBA. Symmetric: base-then-WAL → WAL data overwrites via substrate's last-writer-wins LSN ordering. | `TestRebuildSession_WALWinsThenBaseSkips` (`core/recovery/rebuild_session_test.go:29`)
`TestRebuildSession_BaseFirstThenWALOverwrites` (`core/recovery/rebuild_session_test.go:54`) | ✅ pinned both directions | +| `INV-SESSION-COMPLETE-ON-CONJUNCTION-LAYER1` | `TryComplete` returns `done=true` iff `baseDone ∧ walApplied ≥ targetLSN`. Latched: subsequent calls return `done=true` without state change. NOT including barrier-ack — that is layer-2's system-level confirmation. | `TestRebuildSession_TryComplete_Conjunction` (`core/recovery/rebuild_session_test.go:74`)
`TestRebuildSession_TryComplete_LatchesCompleted` (`core/recovery/rebuild_session_test.go:108`) | ✅ pinned (3 negative + 1 positive + latch) | +| `INV-BITMAP-NO-INDEPENDENT-LOCK` | `RebuildBitmap` exposes no synchronization; callers (`RebuildSession`) serialize access via session mutex. | `TestRebuildSession_ConcurrentLanes` (`core/recovery/rebuild_session_test.go:130`) | ✅ stress test (4 base + 4 WAL goroutines × 1024 LBAs); needs `-race` on Linux for full proof | + +--- + +## Layer 2 — primary-side orchestration + +| INV | Definition | Test(s) | Status | +|---|---|---|---| +| `INV-PIN-EXISTS-ONLY-DURING-SESSION` | Primary maintains no `pin_floor` in steady state; recycle proceeds per retention. `pin_floor` appears only during an active session. | `TestCoordinator_MinPinAcrossActiveSessions` (`core/recovery/peer_ship_coordinator_test.go:159`) — `anyActive=false` when no sessions; emerges on `StartSession`; vanishes on `EndSession` | ✅ pinned | +| `INV-PIN-STABLE-WITHIN-SESSION` | Same session: `pin_floor` is monotonically non-decreasing. To pick a different anchor, invalidate session and start new lineage. | `TestCoordinator_SetPinFloor_Monotonic` (`core/recovery/peer_ship_coordinator_test.go:200`) — regression attempts ignored | ✅ pinned | +| `INV-PIN-ADVANCES-ONLY-ON-REPLICA-ACK` | `pin_floor` advances iff replica has emitted `BaseBatchAcked` for an LBA range whose base data is now installed. Primary cannot advance pin on its own. | `TestE2E_PinFloorAdvancesIncrementally` (`core/recovery/e2e_test.go:466`) — K=8 cadence drives multiple acks across 50-LBA backlog; pin advances through coord.SetPinFloor. Wire impl in `g7-redo/pin-floor` branch (`6cb89ee`). | ✅ pinned | +| `INV-PIN-COMPATIBLE-WITH-RETENTION` | `pin_floor ≥ retained(S)` always holds. A session demanding a pin below S MUST fail-loud → invalidate → new lineage. | Coord layer: `TestCoordinator_SetPinFloor_RejectsBelowRetention` (`core/recovery/peer_ship_coordinator_test.go:225`) — typed `*Failure(PinUnderRetention)` on `floor < primaryS`; `TestCoordinator_SetPinFloor_ZeroBoundaryDisablesCheck` (`core/recovery/peer_ship_coordinator_test.go:262`) — `S=0` legacy path.
Engine retry-budget gate (G7-redo priority #3 [retry] Option A): `TestT4d_PinUnderRetention_BypassesRetryBudget` + `TestT4d_PinUnderRetention_AtBudgetEdge_DoesNotEscalate` (`core/engine/recovery_pin_retention_test.go`) — `RecoveryFailurePinUnderRetention` skips retry, leaves `Attempts` unchanged, emits `PublishDegraded`, does NOT auto-flip Decision (counter-WALRecycled).
Boundary mapping: `TestClassifier_RecoveryPinUnderRetention_MapsToEngineKind` (`core/transport/classifier_pin_under_retention_test.go`) — `*recovery.Failure(PinUnderRetention)` → `engine.RecoveryFailurePinUnderRetention` (closes silent fall-through to retryable Transport). | ✅ pinned (coord + engine retry-bypass + boundary mapping) | +| `INV-RECYCLE-GATED-BY-MIN-ACTIVE-PIN` | Primary's WAL recycle floor = `min(pin_floor)` over active sessions; no sessions ⇒ pure retention. | `TestCoordinator_MinPinAcrossActiveSessions` (`core/recovery/peer_ship_coordinator_test.go:159`) — coord-level min/release.
`TestWALStore_RecycleGate_*` (`core/storage/walstore_recycle_pin_test.go`) — 6 cases: nil source / inactive source / active clamp / floor-above-proposed / no-regress / disable. Substrate `persistCheckpoint` honors gate.
`TestStore_RecycleGate_*` (`core/storage/memorywal/store_recycle_pin_test.go`) — 6 cases mirroring walstore on the in-memory substrate; pins gate interface + AdvanceWALTail clamp. | ✅ pinned (coord + walstore + memorywal substrate integration) | +| `INV-SESSION-COMPLETE-CLOSURE` | System-level done = layer-1 `TryComplete` ∧ barrier-ack(`achieved == targetLSN`). Both halves required before declaring InSync. | `TestCoordinator_CanEmitSessionComplete` (`core/recovery/peer_ship_coordinator_test.go:79`)
`TestE2E_RebuildHappyPath` (`core/recovery/e2e_test.go:33`) — full round-trip
`TestE2E_RebuildWithLiveWritesDuringSession` (`core/recovery/e2e_test.go:127`) — closure with live | ✅ pinned (closure predicate + e2e round-trip) | +| `INV-LIVE-CAUGHT-UP-IFF-FRONTIER-AT-BARRIER` | "Live caught up" is provable only via probe/barrier comparing `R_repr` against `H` at frozen time, not via shipper liveness. | `TestE2E_RebuildHappyPath` and `TestE2E_RebuildWithLiveWritesDuringSession` — both require barrier achieved ≥ target before passing | ✅ pinned indirectly (no test passes without barrier) | +| `INV-SESSION-TEARDOWN-IS-EXPLICIT` | Convergence → InSync → teardown session → drop `pin_floor` → recycle resumes. All transitions are explicit events. | `TestCoordinator_EndSession_Idempotent` (`core/recovery/peer_ship_coordinator_test.go:218`)
e2e tests check `coord.Phase == Idle` post-Run | ✅ pinned | +| `INV-SINGLE-FLIGHT-PER-REPLICA` | At most one active rebuild session per replica. Concurrent attempts rejected at session start. | `TestCoordinator_SingleFlightPerReplica` (`core/recovery/peer_ship_coordinator_test.go:63`)
`TestIntegrationStub_LifecycleCallbacks` (`core/recovery/integration_stub_test.go:22`) — second `StartRebuildSession` errors | ✅ pinned (coord level + bridge level) | + +--- + +## Spec `CHK-*` checks (`v3-recovery-live-line-backlog-spec.md`) + +| CHK | Definition | Test(s) | Status | +|---|---|---|---| +| `CHK-PHASE-NEVER-STEADY-BEFORE-DRAIN` | While recover session active, do NOT transition peer to `SteadyLiveAllowed` until `BacklogDrained ∧ baseDone`. | `TestCoordinator_PhaseTransitionRequiresDrainAndBaseDone` (`core/recovery/peer_ship_coordinator_test.go:10`) — both negative cases (drained-but-not-done, done-but-not-drained) | ✅ pinned | +| `CHK-BARRIER-BEFORE-CLOSE` | No `SessionClosedCompleted` without prior successful barrier `AchievedLSN ≥ target`. | `TestCoordinator_CanEmitSessionComplete` (`core/recovery/peer_ship_coordinator_test.go:79`) | ✅ pinned | +| `CHK-NO-FAKE-LIVE-DURING-BACKLOG` | During scripted backlog, steady sentinel path either paused or fed from same ordered queue. | `TestCoordinator_RouteLocalWrite` (`core/recovery/peer_ship_coordinator_test.go:114`) — phase=DrainingHistorical → SessionLane; phase=SteadyLiveAllowed → still SessionLane (barrier-pending); only Idle returns SteadyLive | ✅ pinned (with architect-ratified semantics: SteadyLiveAllowed is publication-permission, not routing) | + +--- + +## Substrate (memorywal) contract conformance + +| INV / Property | Test(s) | Status | +|---|---|---| +| `LogicalStorage` contract — Write / Read round-trip | `TestContract_WriteReadRoundTrip` (`core/storage/contract_test.go:91`) — runs across BlockStore / WALStore / **MemoryWAL** | ✅ pinned (3 impls) | +| `LogicalStorage` — read-unwritten-LBA returns zeros | `TestContract_ReadUnwrittenLBAReturnsZeros` (`core/storage/contract_test.go:111`) | ✅ pinned (3 impls) | +| `LogicalStorage` — Write advances LSN strictly increasing | `TestContract_WriteAdvancesLSN` (`core/storage/contract_test.go:124`) | ✅ pinned (3 impls) | +| `LogicalStorage` — Sync returns highest written LSN | `TestContract_SyncReturnsHighestWrittenLSN` (`core/storage/contract_test.go:140`) | ✅ pinned (3 impls) | +| `LogicalStorage` — Sync monotonic non-decreasing | `TestContract_SyncMonotonicNonDecreasing` (`core/storage/contract_test.go:160`) | ✅ pinned (3 impls) | +| `LogicalStorage` — Boundaries (R/S/H) reflect writes | `TestContract_BoundariesReflectWrites` (`core/storage/contract_test.go:182`) | ✅ pinned (3 impls) | +| `LogicalStorage` — ApplyEntry uses supplied LSN | `TestContract_ApplyEntryUsesSuppliedLSN` (`core/storage/contract_test.go:204`) | ✅ pinned (3 impls) | +| `LogicalStorage` — AdvanceFrontier bumps H | `TestContract_AdvanceFrontierBumpsH` (`core/storage/contract_test.go:235`) | ✅ pinned (3 impls) | +| `LogicalStorage` — AdvanceWALTail moves S | `TestContract_AdvanceWALTailMovesS` (`core/storage/contract_test.go:253`) | ✅ pinned (3 impls) | +| `LogicalStorage` — AllBlocks current snapshot | `TestContract_AllBlocksReturnsCurrentSnapshot` (`core/storage/contract_test.go:271`) | ✅ pinned (3 impls) | +| `LogicalStorage` — Close idempotent | `TestContract_CloseIsIdempotent` (`core/storage/contract_test.go:286`) | ✅ pinned (3 impls) | +| `LogicalStorage` — Recover on fresh store returns 0 | `TestContract_RecoverOnFreshStoreReturnsZero` (`core/storage/contract_test.go:297`) | ✅ pinned (3 impls) | +| **MemoryWAL-specific:** ScanLBAs emits write-time LSN | `TestScanLBAs_EmitsWriteTimeLSN` (`core/storage/memorywal/store_test.go:59`) | ✅ pinned | +| **MemoryWAL-specific:** 3 writes to same LBA → 3 ScanLBAs entries (V2-faithful per-LSN, NOT state-convergence dedup) | `TestScanLBAs_ThreeWritesSameLBA_EmitsThreeEntries` (`core/storage/memorywal/store_test.go:98`) | ✅ pinned | +| **MemoryWAL-specific:** AdvanceWALTail → ErrWALRecycled gate (typed envelope + errors.Is compat) | `TestAdvanceWALTail_RecycledGate` (`core/storage/memorywal/store_test.go:166`) | ✅ pinned (typed `*RecoveryFailure` + `errors.Is(err, ErrWALRecycled)` via Unwrap) | +| **MemoryWAL-specific:** AppliedLSNs returns per-LBA highest LSN (BlockStore returns sentinel) | `TestAppliedLSNs_TracksHighestPerLBA` (`core/storage/memorywal/store_test.go:224`) | ✅ pinned | +| **MemoryWAL-specific:** RecoveryMode reports WALReplay (not StateConvergence) | `TestRecoveryMode_WALReplay` (`core/storage/memorywal/store_test.go:214`) | ✅ pinned | + +--- + +## Layer 3 — failure taxonomy + +| INV | Definition | Test(s) | Status | +|---|---|---|---| +| Failure retryable matrix (9 kinds) | Wire / Substrate / Contract → retryable; Protocol / Cancelled / SingleFlight / WALRecycled / **PinUnderRetention** / Unknown → not retryable. | `TestFailure_RetryableMatrix` (`core/recovery/failure_test.go:11`) | ✅ pinned (PinUnderRetention added in `g7-redo/pin-floor`) | +| Typed `*Failure` envelope: nil-safe Error/Retryable, errors.Is via Unwrap | `TestFailure_NilSafeAndUnwrap` (`core/recovery/failure_test.go:33`) | ✅ pinned | +| `AsFailure(err)` extraction works on chains; nil/plain → nil | `TestFailure_AsFailureExtraction` (`core/recovery/failure_test.go:52`) | ✅ pinned | +| Wire failure surfaces as typed `Failure(Wire, …)`, retryable=true | `TestIntegrationStub_FailureTypedOnReceiverDown` (`core/recovery/integration_stub_test.go:298`) — replica conn closed early | ✅ pinned | +| Ctx cancel surfaces as typed `Failure(Cancelled, AwaitClose)`, retryable=false | `TestIntegrationStub_FailureTypedOnCancellation` (`core/recovery/integration_stub_test.go:352`) | ✅ pinned | +| Sender's `defer` always seals + EndSessions, regardless of error path | `TestIntegrationStub_ReceiverFailsEarly_SenderUnblocks` (`core/recovery/integration_stub_test.go:163`) — post-error PushLiveWrite errors; coord back to Idle
`TestIntegrationStub_CtxCancelUnblocksSender` (`core/recovery/integration_stub_test.go:230`) — same on ctx cancel
`TestE2E_PushLiveWriteAtomicSeal` (`core/recovery/e2e_test.go:365`) — 8 goroutines × 5 pushes; every accepted push present on replica | ✅ pinned (3 angles) | + +--- + +## Forward-direction INVs (declared in `doc.go`, deliberately not yet tested) + +| INV | Why deferred | Target milestone | +|---|---|---| +| `INV-PIN-ADVANCES-ONLY-ON-REPLICA-ACK` | Requires wire ack frame from receiver → primary to drive `coord.SetPinFloor`. | #3 (BaseBatchAck → pin_floor) | +| `INV-PIN-COMPATIBLE-WITH-RETENTION` | Requires real substrate's `Boundaries().S` cross-check at session start. | #3 | +| `INV-BASE-SPARSE-REQUIRES-SUBSTRATE-CLOSURE` _(or alternate `INV-BASE-FULL-SCAN-NO-SPARSE-ASSUMPTION`)_ | POC chose dense base; sparse omit + substrate basement-clearing is a separate INV pair, one of two must hold. | Sparse base milestone | +| `INV-REPL-OVERLAP-HISTORY-NO-REGRESS` | Bitmap-as-WAL-claim refinement — when stale/duplicate LSN arrives that substrate apply gate skips, bitmap may still be marked to prevent base-lane historical refill over newer replica state. Code currently marks bitmap only after successful ApplyEntry. | After apply-gate redesign | +| `INV-BACKLOG-LIVE-INTERLEAVE-LSN-SERIALIZED` _(candidate name)_ | Spec §3.2 #3 promise: single ordered queue mixing recover-tagged + post-target traffic by LSN. POC ships sequential (backlog → seal → live). | #2 (single-queue real-time interleave) | +| `INV-PRIMARY-SHIP-PHASE` | Operator-visible "primary did not declare InSync on steady-live alone" check; needs engine-level integration to surface. | Wiring PR | + +--- + +## Test sweep summary + +``` +core/recovery/ 36 tests, all PASS (+3 from g7-redo/pin-floor) +core/storage/memorywal/ 14 tests, all PASS +core/storage/contract_test.go 12 tests × 3 impls = 36 sub-test runs, all PASS + ──── + 86 test results pinning the above INVs +``` + +Race-detector status: not run on Windows host (CGO_ENABLED required). +Linux CI must run `go test -race ./core/recovery/... ./core/storage/...` +to validate `INV-BITMAP-NO-INDEPENDENT-LOCK` under stress. + +--- + +## How to keep this current + +When a new INV lands in `core/recovery/doc.go` (or in the spec): + +1. Add a row to the corresponding section. +2. Cite the test function + file:line at branch HEAD. +3. If forward-only (no test yet), put it in the bottom section with the target milestone. +4. Update the "Test sweep summary" counts. + +When a test is renamed or moved, update the line number column. CI +gating (future): a small linter could parse this file and assert +each cited test actually exists and contains the INV name in a +comment — left for a later milestone. diff --git a/sw-block/design/v3-recovery-pin-floor-wire.md b/sw-block/design/v3-recovery-pin-floor-wire.md new file mode 100644 index 000000000..2a4124466 --- /dev/null +++ b/sw-block/design/v3-recovery-pin-floor-wire.md @@ -0,0 +1,238 @@ +# Recovery — `BaseBatchAck` wire + `pin_floor` advancement (design draft) + +**Status**: design draft, not yet implemented. Targets architect priority **#3** in the G7-redo backlog. +**Branch target**: `g7-redo/pin-floor` (or extend `g7-redo/dual-lane-recovery-poc` if QA / architect prefer one PR). + +This doc fixes the protocol surface for receiver→primary acknowledgement so `coord.SetPinFloor` has a real driver. Closes the two forward-direction INVs in `v3-recovery-inv-test-map.md` Layer-2 section: `INV-PIN-ADVANCES-ONLY-ON-REPLICA-ACK` and `INV-PIN-COMPATIBLE-WITH-RETENTION`. + +Implementation is intentionally NOT in this doc — only the contract, the cadence, the failure surface, and the test plan. + +--- + +## 1. Why this exists + +Today (`g7-redo/dual-lane-recovery-poc`): + +- Coordinator already exposes `SetPinFloor(replicaID, lsn) error` and `MinPinAcrossActiveSessions() (uint64, bool)` — see `core/recovery/peer_ship_coordinator.go`. They are testable but never called by anything: the wire has no path from receiver back to primary that delivers ack progress. +- `pin_floor` is therefore frozen at `fromLSN` for the entire session lifetime; recycle is gated indefinitely. + +Without `pin_floor` advancement, two things break under sustained writes / multi-replica: + +1. Primary's WAL retention sits at the lowest active session's `fromLSN` until session ends — recycle stalls. +2. Multi-replica `min(pin_floor)` cannot reflect heterogeneous progress; one slow replica blocks WAL recycle for everyone (`INV-RECYCLE-GATED-BY-MIN-ACTIVE-PIN`). + +`#3` adds the incremental ack so primary's recycle floor catches up with the slowest replica's actually-durable frontier. + +--- + +## 2. Wire frame + +**Decision: new frame type, NOT piggyback.** + +| Option | Choice | Why | +|---|---|---| +| New `frameBaseBatchAck` (receiver → primary) | ✅ chosen | Distinct cadence (incremental during base lane, NOT tied to barrier-end). Wire trace audit is unambiguous. | +| Piggyback on `frameBarrierResp` | rejected | Barrier fires once at session end. Pin only advances at session close — useless for retention recycle during a long base lane. | +| Piggyback on next inbound frame from primary | rejected | Receiver may not have an inbound to piggyback on (primary is also blocked-on-write during base lane). | + +### Frame layout + +``` +frameBaseBatchAck = 0x07 // next available type after BarrierResp(0x06) + +payload (20 bytes): + [8] SessionID uint64 BE + [8] AcknowledgedLSN uint64 BE // receiver's durable frontier as of this ack + [4] BaseLBAUpper uint32 BE // LBA prefix [0, BaseLBAUpper) durably installed; 0 if no base progress yet +``` + +`AcknowledgedLSN` is the load-bearing field; `BaseLBAUpper` is advisory (sender uses it for retransmit logic, NOT for pin floor). + +### Lineage / session binding + +`SessionID` in the payload identifies which session this ack belongs to. Receiver sends only on the same conn that it accepted SessionStart on (no fan-out, no cross-conn acks). Primary's sender validates `SessionID` matches its current session before calling `coord.SetPinFloor`; mismatched session → `FailureProtocol`. + +Lineage's `Epoch` and `EndpointVersion` are NOT in the ack payload — the conn already implies them (same conn established the session). Saves 16 bytes per ack and removes a redundant validation step. + +--- + +## 3. Cadence — when does receiver send `BaseBatchAck`? + +Receiver-side rule (per session): + +``` +After every K base-lane blocks applied → enqueue ack +After every T milliseconds elapsed → enqueue ack (whichever first) +On MarkBaseComplete → MUST send a final ack +On TryComplete returns done → MUST send a final ack just before BarrierResp +``` + +POC defaults: +- `K = 256` blocks (~1 MiB at 4 KiB blocks) +- `T = 100ms` + +Both are tunable via `Receiver` constructor params (test override) or session config (production). The cadence is the receiver's call — sender MUST tolerate any cadence including "zero acks until end" (legacy/fallback path). + +`AcknowledgedLSN` for each ack: +- During base lane: `min(walApplied, syncedLSN-at-ack-time)`. +- After `MarkBaseComplete`: substrate `Sync()` → use returned frontier. +- Final ack at `TryComplete`: same as barrier — receiver's syncedLSN after final Sync. + +--- + +## 4. `SetPinFloor` semantics + +Coordinator-side rule (already in code, just unused): + +``` +SetPinFloor(replicaID, lsn): + if no active session for replicaID → error (Idle peer) + if lsn ≤ current pinFloor → silently ignored (monotonic) + else → pinFloor = lsn +``` + +What `pinFloor = X` means in the spec: + +> Primary commits: replica has durably installed everything with LSN ≤ X. Primary MAY safely recycle WAL up to X for THIS replica's account. + +What it does NOT mean: +- Does NOT mean primary's WAL has actually been recycled (the recycle path consults `MinPinAcrossActiveSessions()` plus retention policy independently). +- Does NOT mean replica claims InSync — that requires barrier-ack, not incremental BaseBatchAck. + +--- + +## 5. Retention inequality (closes `INV-PIN-COMPATIBLE-WITH-RETENTION`) + +At any moment during an active session for replica P: + +``` +pin_floor(P) ≥ S_primary // (1) +pin_floor(P) ≤ walApplied(P) ≤ H_primary // (2) +``` + +Where: +- `S_primary` = primary's WAL retain start (`storage.LogicalStorage.Boundaries()` returns S). +- `H_primary` = primary's WAL head. +- `walApplied(P)` = receiver's highest applied WAL LSN. + +**Violation handling**: + +- **(1) violated** (`pin_floor < S_primary`): primary has already recycled past where this session committed. This is catastrophic: the session contract is unrecoverable on this primary. Action: invalidate session → `Failure(SingleFlight, ?)` or new typed kind `FailurePinUnderRetention` (see §7 below) → engine starts new lineage with fresher fromLSN. +- **(2) violated** (`pin_floor > walApplied`): receiver lied about its frontier OR coord computed wrongly. Treat as `Failure(Contract, …)`. + +The check happens at `coord.SetPinFloor`: + +```go +func (c *PeerShipCoordinator) SetPinFloor(id ReplicaID, floor uint64) error { + // existing monotonic guard + if floor <= existing { return nil } + // NEW: retention compatibility — needs primary's S boundary as input + if floor < primaryRetainStart { + return &FailureSingleFlight{...} // or a new kind, see §7 + } + // existing update +} +``` + +Open question for review: should `SetPinFloor` take `primaryRetainStart` as a parameter, or should the coordinator hold a callback that fetches it from the substrate? Latter is cleaner but introduces a dependency direction. **Recommend**: parameter form, caller (sender) fetches `primary.Boundaries()` before calling `SetPinFloor`. Substrate call is cheap; coordinator stays substrate-free. + +--- + +## 6. Failure surface — extends `core/recovery/failure.go` + +### Reuse of existing kinds + +| Wire / coord event | `FailureKind` | Phase | Retryable | +|---|---|---|---| +| Decode error on `frameBaseBatchAck` payload | `FailureProtocol` | `recv-dispatch` | no | +| `SessionID` in ack ≠ active session | `FailureProtocol` | `recv-dispatch` | no | +| `SetPinFloor` error: `floor > walApplied` (impossible without bug) | `FailureContract` | `pin-update` (new Phase) | yes (re-probe) | +| `SetPinFloor` error: `floor < S_primary` (recycle past commitment) | new kind `FailurePinUnderRetention` (§7) | `pin-update` | no — escalate to new lineage | +| Receiver wire write error sending ack | `FailureWire` | `recv-ack-write` (new Phase) | yes | + +### New `FailureKind`? + +`FailurePinUnderRetention` is semantically distinct from `FailureWALRecycled`: +- `WALRecycled` = sender's `ScanLBAs(fromLSN)` returned recycled error → couldn't even start streaming. +- `PinUnderRetention` = mid-session, primary's S advanced past replica's pin floor → session must invalidate even though it was streaming fine. + +**Recommend**: add the new kind. Mapping at engine boundary still feeds `RebuildPinned` for both (rebuild from a fresher anchor). + +--- + +## 7. Code surface (sender / receiver / coordinator changes) + +Only sketched; no implementation in this doc. + +``` +core/recovery/wire.go + + frameBaseBatchAck = 0x07 + + encodeBaseBatchAck / decodeBaseBatchAck + +core/recovery/receiver.go + + cadence config (K blocks, T duration; defaults 256/100ms) + + ack goroutine OR inline ack-send after each Apply* if cadence reached + + final ack on MarkBaseComplete + barrier path + +core/recovery/sender.go + + new goroutine: read frames from conn, multiplex BarrierResp + BaseBatchAck + (today sender only reads BarrierResp at end — needs a frame demux loop) + + on BaseBatchAck: validate sessionID, fetch primary.Boundaries().S, call coord.SetPinFloor + + new Phase tag: PhasePinUpdate + +core/recovery/peer_ship_coordinator.go + + SetPinFloor(id, floor uint64, primarySBoundary uint64) error + + new return: FailurePinUnderRetention (or whatever name lands) + +core/recovery/failure.go + + FailurePinUnderRetention (if new kind chosen) + + PhasePinUpdate, PhaseRecvAckWrite +``` + +The frame demux loop on the sender side is the largest delta — today the sender only reads ONE inbound frame (BarrierResp at end). Switching to a demux pattern with a goroutine + channel is straightforward but worth flagging. + +--- + +## 8. Test plan + +In `core/recovery/`: + +| Test | Pins | +|---|---| +| `TestWire_BaseBatchAck_RoundTrip` | encode/decode wire format | +| `TestCoordinator_SetPinFloor_RejectsBelowRetention` | INV-PIN-COMPATIBLE-WITH-RETENTION (new) | +| `TestCoordinator_SetPinFloor_RejectsAboveWalApplied` | inequality (2) — Contract failure | +| `TestE2E_PinFloorAdvancesIncrementally` | INV-PIN-ADVANCES-ONLY-ON-REPLICA-ACK — drive 1000 LBAs, observe pinFloor advance in a series of steps via Status snapshots | +| `TestE2E_AckCadence_K_Triggered` | every K blocks → one ack | +| `TestE2E_AckCadence_T_Triggered` | bursty workload + idle → time-based ack | +| `TestE2E_NoAckUntilEnd_StillCorrect` | receiver sends only the mandatory final ack; pinFloor advances exactly once at session end (legacy/fallback path) | +| `TestIntegrationStub_FailurePinUnderRetention` | typed `*Failure` surface for the new kind | + +Test count expected: ~8 new, all in `core/recovery/`. No new substrate work; `MemoryWAL` already exposes `Boundaries().S` in V2-faithful form. + +--- + +## 9. Cross-references + +After this lands, update: + +- `core/recovery/doc.go` — pin INV section: replace `_Pending #3_` markers with `see v3-recovery-pin-floor-wire.md §N`. +- `v3-recovery-inv-test-map.md` — Layer-2 forward rows for the two pin INVs: move from "forward" to "pinned" with the new test cites. +- `v3-recovery-inv-test-map.md` — Layer-3 taxonomy section: add row for `FailurePinUnderRetention` if that kind lands. + +--- + +## 10. Out of scope for this milestone + +- **§3.2 #3 single-queue real-time interleave** (architect priority #2). Pin advancement does NOT depend on it. +- **Wiring into `core/transport`**. Pin advancement lives entirely in `core/recovery/`'s self-contained POC; the wiring PR consumes it later. +- **Multi-replica fanout in primary**. `MinPinAcrossActiveSessions` is already implemented and tested; no new code there for this milestone. +- **Retention policy at WAL recycle**. The recycle path in `core/storage/walstore.go` is unchanged. A separate doc / PR will wire `MinPinAcrossActiveSessions()` into the recycle decision. + +--- + +## 11. Open questions for architect — Resolved (architect ACK 2026-04-29) + +1. **`FailurePinUnderRetention` as a new kind** vs reusing `FailureWALRecycled`. **RESOLVED: new kind.** Rationale: `WALRecycled` is a cold-start scan failure (typical fix: bump pin / new lineage); `PinUnderRetention` is a mid-session contract violation (typical fix: invalidate session, log Invariant breach). Different `Retryable()` defaults, different operator response. Implementation MUST update `failure.go` matrix + `doc.go` INV ledger + `recovery-inv-test-map.md` row in lockstep — no verbal-only distinction. +2. **`SetPinFloor` signature**: parameter vs callback. **RESOLVED: explicit parameter.** Caller computes floor from `(S, H, AckLSN)` and calls `SetPinFloor(floor)`. Coordinator stays substrate-free. If a unified entry is needed later, it should be a value type (`PinFloorInput struct`), NOT a `func() uint64` callback. +3. **Wire compat**: independent frame `0x07` vs piggyback on `MsgShipEntry`. **RESOLVED: keep independent frame for this milestone.** Ack / data split → packet-capture, log, review all simpler; control-plane (receiver→primary ack) decoupled from data-plane (rate, retry). Piggyback / merge is a later bandwidth optimization — does not gate #3 correctness. diff --git a/sw-block/design/v3-recovery-unified-wal-stream-kickoff.md b/sw-block/design/v3-recovery-unified-wal-stream-kickoff.md new file mode 100644 index 000000000..9b8c5276e --- /dev/null +++ b/sw-block/design/v3-recovery-unified-wal-stream-kickoff.md @@ -0,0 +1,295 @@ +# Recovery — Unified WAL stream / cursor-rewind kickoff (v0.3) + +**Status**: kickoff v0.3 — round 2 erratum 2026-04-29 logged in §11. Round 1 ratified all Q1-Q7 with defaults (+Q8 follow-on, +Q9 receiver discipline). Round 2 corrects two read-time contradictions surfaced by architect re-read after mini-plan v0.1; **no invariant or scope changes**, documentation-only. Code-go-ahead **still gated** on mini-plan v0.2 ratification per governance sequence (`feedback_t4d_governance_sequence.md`). + +**Branch target**: `g7-redo/unified-wal-kickoff` (this branch carries the doc; implementation branch is a sibling once mini-plan ratifies). + +**Trunk base**: `phase-15` tip `afec29d` — train (#11/#13/#14/#15/#16) is in trunk; kickoff sits on the merged dual-lane recovery foundation rather than a moving base. + +**Ledger pin (forward)**: this work closes `INV-WAL-CURSOR-MONOTONIC-FROM-PINLSN` (Q2 ratified — short name; "monotonic-from-pinLSN" says the load-bearing thing without inviting "tracked window" overhead). + +**Working name**: "Unified WAL stream / cursor-rewind to pinLSN" (Q4 ratified). Spec text "§3.2 #3 single-queue real-time interleave" stays in citations to other docs. + +--- + +## 1. Why this exists — the framing flip + +The spec calls this `§3.2 #3 single-queue real-time interleave`. "Interleave" reads as "two streams mixed", and the POC literally implements that: a backlog stream meets a live-write queue at a `drainAndSeal` step. Architect 2026-04-29 framing flip — quoted verbatim, design-quotable: + +> 单队列交织 ≠ 两段物理 phase。 + +The follow-on architect clarification (round 1 ratification) sharpens the working description without leaning on "sliding window" abstraction: + +> WAL pump 在开始 rewind 到 pinLSN，再向前走到 head——这就够描述 §3.2 #3 相对 POC 的差别。 + +So: **a single session, single WAL queue. The only special operation is `cursor := pinLSN` once at session start. After that, the cursor is monotonically increasing — same shape as steady-state shipping.** No "sliding window" abstraction; no tracked bandwidth; just one rewind and a forward walk. + +The work this milestone delivers is therefore: + +1. Add the **rewind-to-pinLSN** at session start (where today the recovery sender opens a separate scan path). +2. **Delete** the two-phase gate (`liveQueue` / `sealed` / `PushLiveWrite` / `drainAndSeal` / explicit `TryAdvanceToSteadyLive`). +3. Replace with a single forward loop. Optionally tag frames with a `Kind=Backlog→SessionLive` byte that flips once when the cursor catches `head` (observability only — see §4). + +--- + +## 2. Current shape — the gate that's being deleted + +Trunk tip `afec29d` carries the train, including `core/recovery/sender.go`. The two-phase gate is concentrated in five concrete artifacts: + +| Artifact | Role | Disposition | +|---|---|---| +| `Sender.liveQueue []walItem` field (~line 65) | Separate buffer for live writes pushed during backlog phase | **DELETE** | +| `Sender.sealed bool` flag + `Sender.queueMu sync.Mutex` | Atomic-seal protection for the live queue | **DELETE** (no buffer = no seal needed) | +| `Sender.PushLiveWrite(lba, lsn, data) error` API (~line 146) | Caller-side push-into-queue model | **DELETE** (writes arrive naturally via substrate's growing tail) | +| `Sender.Run()` step 5 — `<-closeCh` wait (~line 254) | Caller signals "no more live writes" before barrier | **REPLACE** with "cursor reached head + idle window elapsed" | +| `Sender.drainAndSeal()` method (~line 432) + `coord.TryAdvanceToSteadyLive()` step (~line 273) | Drains buffered live writes; flips coordinator phase | **DELETE** (transition becomes implicit: kind-byte flip when `head - cursor < ε`) | + +POC author's own admission at sender.go line 141-145: + +> Live writes are buffered until the backlog drain phase finishes, then flushed in LSN order before barrier. **POC simplification — in production the sender would interleave them with backlog by LSN order in real time.** + +The wiring layer (`core/transport/rebuild_sender.go:138-144`) explicitly notes the workaround is current-binary scope; the `FinishLiveWrites` early-call at line 164-167 is the production-side workaround that becomes obsolete once §3.2 #3 lands. + +The gate is architecturally wrong but **operationally harmless today** — `liveQueue` is empty at `drainAndSeal` time because `FinishLiveWrites` is called before any live write can land. Lifting the restriction (= live writes during rebuild become legal) requires deletion of the gate. + +--- + +## 3. Target shape — one cursor, one rewind, monotonic forward + +### 3.1 Sender lifecycle (after the deletion) + +Sender's WAL-lane work becomes: + +``` +1. cursor := pinLSN // the one rewind +2. ship base lane (extent dump, parallel) +3. send frameBaseDone +4. loop: + scan ScanLBAs(cursor, fn) where + fn(entry): + emit frameWALEntry{Kind=kind, LBA, LSN, data} + cursor = entry.LSN + // Kind-flip rule (§4) + _, _, head := primaryStore.Boundaries() + if kind == Backlog && head - cursor < ε: + kind = SessionLive + until cursorAtHead(cursor) AND idleWindowElapsed() +5. send frameBarrierReq +6. read frameBarrierResp; verify AchievedLSN ≥ targetLSN +``` + +`cursor` is just `uint64`. It starts at `pinLSN` (the one and only rewind), increases monotonically per `ScanLBAs` callback, never decreases. There is no separate "window" or "bandwidth" or "queue" — just an integer that advances. + +**Naming note**: `pinLSN` (this doc's design term) and `fromLSN` (sender code's existing field name carrying the same value into the wire as `frameSessionStart.FromLSN`) refer to the same number within a session — the session's already-installed-on-replica watermark, the lower bound of the rewind. Implementation uses `fromLSN`; design text uses `pinLSN` for emphasis on the architect framing. Mini-plan Q13's `appliedLSN := fromLSN` initializer is consistent with this identity. + +### 3.2 idleWindow guard (not a convergence proof) + +The pump exits when (a) `cursor == head` at the moment the scan exhausts AND (b) an idle window passes without new appends. This is a **don't-barrier-mid-burst** guard. It is **not** a convergence proof — only barrier-ack with `AchievedLSN ≥ targetLSN` proves convergence (`INV-LIVE-CAUGHT-UP-IFF-FRONTIER-AT-BARRIER`). If the pump exits prematurely (rare: idle window elapses but a write lands during barrier round-trip), the barrier returns `AchievedLSN < targetLSN` and the session fails as `FailureContract` per existing taxonomy. + +### 3.3 Why this isn't a "sliding window" abstraction + +The v0.1 of this kickoff used "sliding window over `[pinLSN, head)`" as the framing. Architect ratification 2026-04-29 round 1 sharpened: the v0.1 phrase invites readers to think there's window-state to maintain (bandwidth, width, slide rules). There isn't. There's one `uint64` that goes up, plus an idle-window check at exit. Sharper text adopted: **rewind-once + monotonic forward**. + +(For contributors who want the geometric metaphor as a private mental model, "cursor walks `[pinLSN, head)` while the right edge moves" is fine — but should not appear in code comments or public design text, where the literal "rewind once, increment, exit when stuck" is shorter.) + +--- + +## 4. Wire format invariance + +**The wire is byte-identical from the replica's perspective.** Headline architectural claim: + +| Frame type | Today | After §3.2 #3 | +|---|---|---| +| `frameSessionStart` (1) | `[8 SessionID][8 FromLSN][8 TargetLSN][4 NumBlocks]` | **unchanged** | +| `frameBaseBlock` (2) | `[4 LBA][block]` | **unchanged** | +| `frameBaseDone` (3) | empty | **unchanged** | +| `frameWALEntry` (4) | `[1 Kind][4 LBA][8 LSN][block]` | **unchanged framing**; **transition rule** for `Kind` byte changes (see 4.1) | +| `frameBarrierReq` (5) | empty | **unchanged** | +| `frameBarrierResp` (6) | `[8 AchievedLSN]` | **unchanged** | +| `frameBaseBatchAck` (7) | `[8 SessionID][8 AcknowledgedLSN][4 BaseLBAUpper]` | **unchanged** | + +Receiver-side **wire decode/encode** (`frameWALEntry` layout, `WALEntryKind` byte values, `decodeWALEntry`) requires **zero changes**. Receiver-side **apply path** gets a NEW monotonic-discipline check (§5) — non-zero LOC. The two are different layers: the wire is invariant; the validation around `session.ApplyWALEntry` gets a new branch. The hardware canonical (`v3-recovery-dual-lane-canonical.yaml`) re-runs unmodified — only the kind-byte distribution in any frame-capture artifact (`dual_lane.pcap` or equivalent) changes. + +### 4.1 Kind byte — the only behavioral change on the wire + +Today: kind transitions at `drainAndSeal` boundary (explicit step in sender's state machine). + +After: kind transitions when `head - cursor < ε`. Once flipped to `SessionLive`, never flips back. + +**Q1 ratified default**: `ε = 8 LSNs`. Constant; observability-only (does not affect any correctness invariant). + +--- + +## 5. Receiver frame discipline (monotonic LSN contract) + +Per architect 2026-04-29 round 2, the receiver's monotonic-LSN expectation needs explicit handling rules. The sender's deletion work does NOT loosen the receiver contract; it tightens it (no more `drainAndSeal` boundary that hides ordering glitches). + +### 5.1 Three frame-arrival cases + +For receiver state `applied = highest LSN durably applied so far`, define `next.LSN` as the LSN on the next inbound `frameWALEntry`: + +| Case | Action | Failure class | Notes | +|---|---|---|---| +| `next.LSN == applied + 1` | **Apply normally** | — | Steady-state path; the only legal forward step | +| `next.LSN > applied + 1` (gap) | **Reject — fail-loud** | `FailureContract` | Hole in LSN sequence; primary did NOT send a contiguous stream. NOT a silent skip. Not allowed even in recover. WAL-recycled gaps go through `FailureWALRecycled` + new lineage, not via this path. | +| `next.LSN == applied` (exact duplicate on the wire) | **Reject — fail-loud** | `FailureProtocol` | Sender does not retransmit the same LSN within a session. TCP delivers in-order; recovery sender writes each LSN exactly once; this case shouldn't happen and indicates a real sender bug if it does. NOT to be confused with §5.2 duplicate-LSN claim semantics — those are about base-vs-WAL arbitration on the receiver, not wire retransmits. | +| `next.LSN < applied` (in-stream backward) | **Reject — fail-loud** | `FailureProtocol` | Backward LSN in same session/same lineage = protocol violation. The ONLY legitimate "rewind" is session-level (`cursor := pinLSN` at SessionStart, observable as the new session's `frameSessionStart`). NOT a normal recover branch. | + +### 5.2 Duplicate-LSN claim semantics — different layer than §5.1 + +The "skip data, update bitmap" pattern is a **per-LBA arbitration** concern at the substrate / bitmap layer (`INV-DUAL-LANE-WAL-WINS-BASE`), NOT a wire-frame ordering concern. Concretely: + +> The base lane and the WAL lane both touch the same LBA. The WAL lane writes LSN=N for LBA L; later, the base lane delivers L's snapshot bytes (which were captured at LSN ≤ N at session start). Without arbitration the base lane would clobber the WAL-won bytes. The bitmap records `MarkApplied(L)` when the WAL lane wins, and base-lane handlers skip `L` thereafter. This is per-LBA last-writer-wins by LSN, not a wire-frame gate. + +This **does not contradict §5.1**. §5.1 is wire-level (frame ordering on a single TCP session); §5.2 is substrate-level (per-LBA arbitration across two lanes). The two layers do not see each other's "duplicates": +- Same-LSN frame on the wire: §5.1, Protocol error (sender bug; never legitimate). +- Same-LBA write at lower LSN at the substrate after a higher-LSN write already applied: §5.2, no-op + bitmap-claim already in place. + +Mini-plan §2.3's `checkMonotonic` implements §5.1 only. §5.2 is existing receiver-side bitmap behavior; this milestone does not change it. + +### 5.3 The only legitimate "rewind" in recover + +`cursor := pinLSN` happens **once**, at session start, on the **sender** side. The replica observes a `frameSessionStart` followed by frames whose LSNs start at `pinLSN + 1` and increase monotonically. The replica never observes "LSN goes down" within one session. + +If a session ends and a new session begins (new `frameSessionStart` with new `SessionID`), the new session's `FromLSN` may be lower than the prior session's last applied LSN — but that's a **new session boundary**, not an in-stream rewind. The receiver's `applied` cursor resets per session (or is tracked per-lineage); existing receiver code already handles this. + +### 5.4 Implementation note + +`core/recovery/receiver.go` currently enforces some-but-not-all of these rules. Mini-plan stage audits the receiver's frame-handling for explicit gap/backward/duplicate branches. **Only NEW assertion in mini-plan: explicit FailureProtocol on `next.LSN ≤ applied` if not already pinned.** No code change to add a "skip data + bitmap update for backward frames" path — that was conflated logic, now explicitly disallowed. + +--- + +## 6. Invariants preserved (no regression) + +The deletion work must preserve every INV currently pinned. Audit: + +| INV | How current shape pins it | How target shape pins it | +|---|---|---| +| `INV-PIN-EXISTS-ONLY-DURING-SESSION` | `coord.StartSession`/`EndSession` bracket | unchanged — pump runs inside same bracket | +| `INV-PIN-ADVANCES-ONLY-ON-REPLICA-ACK` | `BaseBatchAck` (frame 7) → `coord.SetPinFloor` | unchanged — reader-loop and ack handling untouched | +| `INV-PIN-COMPATIBLE-WITH-RETENTION` | `coord.SetPinFloor` rejects `floor < primaryS` | unchanged | +| `INV-RECYCLE-GATED-BY-MIN-ACTIVE-PIN` | walstore + memorywal recycle gates consult `MinPinAcrossActiveSessions` | unchanged | +| `INV-SESSION-COMPLETE-CLOSURE` | barrier-ack with `AchievedLSN ≥ targetLSN` + layer-1 `TryComplete` | unchanged | +| `INV-LIVE-CAUGHT-UP-IFF-FRONTIER-AT-BARRIER` | only barrier proves convergence | **strengthened** — pump's idle-window is explicitly NOT a convergence proof; only barrier counts (§3.2) | +| `INV-DUAL-LANE-WAL-WINS-BASE` | bitmap arbitration receiver-side | unchanged — duplicate-LSN claim path (§5.2) honors this | +| `INV-SESSION-COMPLETE-ON-CONJUNCTION-LAYER1` | `TryComplete` requires `baseDone ∧ walApplied ≥ targetLSN` | unchanged | +| `INV-BITMAP-NO-INDEPENDENT-LOCK` | bitmap serialized via session mutex | unchanged | +| `INV-SINGLE-FLIGHT-PER-REPLICA` | coord rejects concurrent sessions | unchanged | +| `INV-SESSION-TEARDOWN-IS-EXPLICIT` | `coord.EndSession` runs in defer | unchanged | +| `INV-PIN-STABLE-WITHIN-SESSION` | monotonic SetPinFloor | unchanged | +| `INV-WAL-CURSOR-MONOTONIC-FROM-PINLSN` (NEW) | n/a (added by this milestone) | sender pump enforces monotonic increase from pinLSN; receiver enforces monotonic-applied (§5) | +| `CHK-PHASE-NEVER-STEADY-BEFORE-DRAIN` | `TryAdvanceToSteadyLive` requires `BacklogDrained ∧ baseDone` | **affected** — Q3 ratified default: collapse `Phase` enum to `{Idle, Active}`. Phase semantic recasts to "active session ⇒ ack-driven pin advancement; routing decisions move into kind-byte flip rule" | +| `CHK-BARRIER-BEFORE-CLOSE` | no `SessionClosedCompleted` without barrier | unchanged | +| `CHK-NO-FAKE-LIVE-DURING-BACKLOG` | RouteLocalWrite returns SessionLane during DrainingHistorical | **affected** — Q3 default: replaced by "session active = SessionLane until EndSession", since there is no "during backlog" sub-state once the gate is deleted | + +Q3 ratified default: collapse `Phase` enum. CHK-PHASE-NEVER-STEADY-BEFORE-DRAIN and CHK-NO-FAKE-LIVE-DURING-BACKLOG re-anchor to the new {Idle, Active} semantic — mini-plan owns the diff. + +--- + +## 7. Implementation plan (high-level — mini-plan owns the diff) + +### 7.1 Files touched + +- `core/recovery/sender.go` — DELETE the gate; ADD `streamUntilHead()` (or equivalent — name is bikeshed). Net: **~-40 LOC** (delete ~120, add ~80). +- `core/recovery/sender_test.go` — DELETE PushLiveWrite-path tests; ADD monotonic-cursor + rewind-once + idle-window tests. +- `core/recovery/peer_ship_coordinator.go` — Q3 default: collapse `Phase` enum to `{Idle, Active}`, drop `TryAdvanceToSteadyLive`. Update `RouteLocalWrite` accordingly. +- `core/transport/rebuild_sender.go` — DELETE the `FinishLiveWrites` early-call workaround at line 164-167. +- `core/recovery/wire.go` — **no changes** (wire format invariant; only kind-flip rule is sender-internal). +- `core/recovery/receiver.go` — verify monotonic-LSN discipline (§5); add explicit `FailureProtocol` branch for `next.LSN ≤ applied` if not already present. + +### 7.2 Test surface + +Existing tests must continue to pass byte-identical: `TestE2E_RebuildHappyPath`, `TestE2E_RebuildWithLiveWritesDuringSession`, `TestRebuildSession_*`, `TestCoordinator_*`. + +New tests: +- `TestSender_RewindOnce_CursorMonotonicForward` — primary writes during pump; cursor follows; barrier hits expected `AchievedLSN`. +- `TestSender_KindByte_FlipsOnceAtCatchUp` — sequence shows monotonic Backlog→SessionLive transition; never flips back. +- `TestSender_IdleWindow_NotPrematureBarrier` — head briefly catches cursor mid-burst; pump waits idle window; primary writes again; cursor resumes; barrier eventually fires only at true exhaustion. +- `TestReceiver_RejectsBackwardLSN_InSession` — synthesized inbound frame with `LSN ≤ applied` produces typed `FailureProtocol`; receiver does NOT silently skip or rewind. +- `TestReceiver_RejectsGap_InSession` — synthesized inbound frame with `LSN > applied + 1` produces typed `FailureContract`. + +### 7.3 LOC / complexity estimate + +- Production: net **~-40 LOC** (deletion-heavy). +- Tests: **+~150 LOC** (new pump tests; old PushLiveWrite tests deleted; new receiver-discipline tests). +- Dependent file count: **5** (sender, sender_test, coord, rebuild_sender, receiver). + +--- + +## 8. Risks & open hazards + +| # | Hazard | Mitigation | +|---|--------|-----------| +| H1 | `idleWindow` value is a tuning knob — too small barriers prematurely under sustained-write workload; too large stalls otherwise-complete sessions | Mini-plan: configurable; default `100ms`; pin `TestSender_IdleWindow_NotPrematureBarrier` | +| H2 | `ε` for kind-flip — observability assertions counting Backlog vs SessionLive frames could regress | Q1 ratified `ε = 8`; observability contract states kind transition is **monotonic** but exact split-point is not pinned | +| H3 | `coord.Phase` semantic collapse — Q3 ratified | Mini-plan owns the enum change; integration tests audit | +| H4 | Hardware canonical's `observe_recycle_gate_active` phase — pump runs longer (serves live writes); checkpoint clamping window extends | Mini-plan reviews canonical YAML; minor edit if needed | +| H5 | Substrate `ScanLBAs` called repeatedly — performance under heavy load | Mini-plan benchmarks; may require ScanLBAs streaming/cursor API extension | +| H6 (NEW) | Receiver monotonic-frame discipline — if existing receiver doesn't ALREADY enforce backward-frame rejection, it's a behavioral change observable to senders that incorrectly retransmit | Mini-plan audits `receiver.go`; if missing, the new branch becomes a **defense-in-depth** addition, not an observable change for current production senders (which never send backward in-stream) | + +--- + +## 9. Open questions for architect (round 1 status) + +| Q | Topic | Default | Round 1 | +|---|-------|---------|---------| +| **Q1** | `ε` for kind-flip | constant `ε=8` LSNs | **RATIFIED** | +| **Q2** | Invariant name | `INV-WAL-CURSOR-MONOTONIC-FROM-PINLSN` | **RATIFIED** (short form per architect; see §11) | +| **Q3** | Coordinator `Phase` enum after `TryAdvanceToSteadyLive` removal | collapse to `{Idle, Active}` | **RATIFIED** | +| **Q4** | Milestone working name | "Unified WAL stream / cursor-rewind to pinLSN" | **RATIFIED** | +| **Q5** | Implementation governance | standard (mini-plan → architect → G-1 → code) | **RATIFIED** | +| **Q6** | Mini-plan scope | include `coord.Phase` redesign | **RATIFIED** | +| **Q7** | Hardware canonical impact | minimal (only timeout tuning) | **RATIFIED** | +| **Q8** (NEW) | Merge with production WalShipper (one cursor mechanism for steady-state + recover) | **follow-on milestone B** — NOT this PR | **RATIFIED** as follow-on | +| **Q9** (NEW) | Receiver monotonic-frame discipline (§5) | architect-provided table; pin via 2 new tests | **RATIFIED** (architect provided the discipline; mini-plan implements) | + +All Q1-Q9 ratified or ratified-with-defaults. Mini-plan stage may surface more questions; new questions get appended (Q10+) at that stage. + +--- + +## 10. Out of scope (explicit) + +- **Production WalShipper merge** (Q8, follow-on milestone B). This kickoff explicitly does NOT collapse the steady-state (push) and recovery (pull) shipping paths into one cursor mechanism. That is a separate kickoff/PR. +- **Option B retry policy** (per-failure-kind budget + lossless mapping + observability): separate kickoff. +- **Backoff / wall-clock retry cap**: separate milestone from Option B. +- **Multi-volume / multi-replica (RF>2) interleave stress**: hardware canonical covers RF2; RF3 is a follow-on. +- **Substrate `ScanLBAs` streaming API**: H5 mitigation may surface a need; mini-plan flags it. Not gating §3.2 #3. +- **Substrate per-entry LSN assumption**: this milestone assumes the WAL substrate (`memorywal.Store`, `walstore.WALStore`) emits each `RecoveryEntry` with its **write-time LSN** — every callback in `ScanLBAs` carries a strictly-increasing `entry.LSN`. Substrates that synthesize a scan-time LSN (e.g., the in-memory `BlockStore` discussed in earlier POC reviews) are out of scope. The recovery-execution path uses MemoryWAL or WalStore; BlockStore is not on this milestone's substrate list. +- **Removing legacy mode entirely**: post-default-flip + ≥1 release cycle (per Open Question 4 of `v3-recovery-wiring-plan.md`). Independent of §3.2 #3. +- **Metric/log surface for kind transition**: nice-to-have for Option B. + +--- + +## 11. Resolution log + +### Round 1 — 2026-04-29 (architect ratification) + +**Naming change**: v0.1 used "sliding-window" framing throughout. Architect: "不必死守『滑动窗』隐喻；rewind-once + 单调 cursor 就够". Adopted **rewind-to-pinLSN + monotonic forward** as the standard text. "Sliding window" appears in §3.3 only as an explicit private-mental-model note, not in code/design. + +**Q1-Q7 ratified-with-defaults**: defaults stand. + +**Q8 added**: "merge recovery sender's WAL pump with production WalShipper into one cursor mechanism" — architect ratified as **follow-on milestone B**, NOT this kickoff/PR. Default rationale: the gate-deletion in §2 is the focused work; full shipper unification is a deeper architectural change that would inflate scope and slow §3.2 #3 closure. + +**Q9 added**: receiver monotonic-frame discipline (§5). Architect provided the gap/backward/duplicate handling table directly; mini-plan implements as audit + missing branches. + +**INV name (Q2)**: ratified as `INV-WAL-CURSOR-MONOTONIC-FROM-PINLSN` — shorter than the v0.1 `INV-WAL-STREAM-IS-ONE-CURSOR-OVER-RETAINED-WINDOW`. Architect: "比 LONG retained-window 更贴代码". + +**Branch cleanup**: architect green-lit deletion of merged-content branches (`g7-redo/pin-floor`, `wiring`, `recycle-pin-hookup`, `retry-fix-pin-under-retention`). Convention: feat branches deleted post-merge. Tags in `archive/g7-phase15-attempt-*` are intentional bisect ground; do NOT touch. + +**Governance (Q5)**: standard — mini-plan → architect → G-1 → code. Round 1 closes the kickoff stage; mini-plan stage opens next. + +### Round 2 — 2026-04-29 (kickoff erratum aligned with mini-plan v0.1) + +Architect re-read after mini-plan v0.1. Two contradictions / easy-to-misread surfaces flagged in v0.2 + corrected here as v0.3: + +**(A) "Receiver code requires zero changes" was overclaim**. §4 paragraph said receiver.go gets zero changes; §7.1 (and mini-plan §2.3) said receiver gets a new `checkMonotonic` branch. Resolved by distinguishing layers: **wire decode/encode** unchanged (frame layout, kind values, decodeWALEntry); **apply path** gets the new monotonic discipline check (non-zero LOC). §4 paragraph rewritten to make this explicit. + +**(B) `lsn == applied` was implicit in `≤ applied`** but ambiguous against §5.2 duplicate-LSN claim semantics. v0.2 §5.1 lumped `≤ applied` under one "backward → Protocol" row; reader could read this as conflicting with §5.2 "duplicates are legitimate at-least-once". Resolved by splitting the row in §5.1: `== applied` (exact wire-level duplicate) is `FailureProtocol` separately from `< applied` (in-stream backward) which is also `FailureProtocol`. §5.2 rewritten to clarify it's substrate-level per-LBA arbitration, NOT wire-frame gating, and explicitly does NOT contradict §5.1. Mini-plan §2.3 `checkMonotonic` implements §5.1 only. + +**Naming consistency**: explicit note added to §3.1 — `pinLSN` (design text) ≡ `fromLSN` (code field). Aligns with mini-plan Q13's `appliedLSN := fromLSN` initializer. + +**Substrate LSN assumption**: §10 OOS adds explicit note — this milestone assumes per-entry write-time LSN from MemoryWAL/WALStore; BlockStore-style synthetic scan-time LSN is out of scope. + +**dual_lane.pcap reference**: softened to "frame-capture artifact (e.g. `dual_lane.pcap`)" since the canonical may produce equivalent capture under a different filename. + +No invariant changes. No question changes. No code changes; this is documentation-only erratum. Mini-plan v0.2 (in companion branch) carries the matching `appliedLSN` initializer fix. diff --git a/sw-block/design/v3-recovery-unified-wal-stream-mini-plan.md b/sw-block/design/v3-recovery-unified-wal-stream-mini-plan.md new file mode 100644 index 000000000..ad2a8798c --- /dev/null +++ b/sw-block/design/v3-recovery-unified-wal-stream-mini-plan.md @@ -0,0 +1,394 @@ +# Recovery — Unified WAL stream / cursor-rewind mini-plan (v0.2) + +**Status**: mini-plan v0.2 — round 1 architect ratification 2026-04-29 logged in §10. All Q10-Q15 ratified-with-defaults. One §2.3 comment fix (Q13 alignment) + cross-reference to kickoff v0.3 §5.1 row split. Stage 2 of governance sequence (`feedback_t4d_governance_sequence.md`) closed; **G-1 stage may begin** per architect. + +**Branch target**: `g7-redo/unified-wal-mini-plan` (this branch carries the doc; implementation branch is a sibling once mini-plan ratifies). + +**Stack base**: `g7-redo/unified-wal-kickoff` (kickoff v0.2 with round 1 ratification logged). + +**Trunk base** (after kickoff merges): `phase-15` tip with the train + kickoff doc landed. + +**Scope contract**: this doc spells out the per-file diff. The implementation branch executes it. Anything not pinned here is out-of-scope for the implementation PR; surfaces back as a Round 2 question. + +--- + +## 1. What this stage owns vs. what stays for G-1 / code + +| Stage | Owns | Defers | +|---|---|---| +| Kickoff (v0.2, ratified) | Framing, naming, INV pin, scope boundary, Q1-Q9 defaults, receiver discipline table | File-level diff, test catalog | +| **Mini-plan (this doc)** | **File-level diff plan, test catalog, LOC budget, G-1 reading list, sequencing** | Actual code, V2 read execution | +| G-1 | V2 read of the gate-deletion site + receiver discipline (per `feedback_g1_pre_code_review.md`) | Actual code | +| Code | Implementation; tests; CI green | — | + +Mini-plan is **diff specifications** — what changes per file, what tests get written, in what order. It is NOT a code review against pseudocode. + +--- + +## 2. File-by-file diff plan + +### 2.1 `core/recovery/sender.go` — gate deletion + rewind-once pump + +**Current**: 470 lines. Five-step `Run()` that streamBacklog → wait closeCh → drainAndSeal → TryAdvanceToSteadyLive → barrier. + +**Diff**: + +| Action | Target | Notes | +|---|---|---| +| **DELETE** | Lines 60-69 (`queueMu`, `liveQueue`, `sealed` fields) | Buffer + atomic-seal disappear | +| **DELETE** | Lines 117-134 (`Close` method + closeOnce) | Caller-side "no more live writes" signal disappears (replaced by idle-window) | +| **DELETE** | Lines 136-156 (`PushLiveWrite` API) | Live writes flow through substrate's growing tail, NOT through this API | +| **DELETE** | Lines 244-258 (Step 5 in `Run()`: `<-closeCh` / `<-ctx.Done()` wait) | Replaced by idle-window exit in pump loop | +| **DELETE** | Lines 260-279 (Step 6 + 7: `drainAndSeal` + `TryAdvanceToSteadyLive`) | Both gone | +| **DELETE** | Lines 384-428 (`streamBacklog` method) | Replaced by `streamUntilHead` | +| **DELETE** | Lines 432-470 (`drainAndSeal` method) | Goes with the queue | +| **DELETE** | Lines 203-209 (defer that sets `sealed=true`) | No `sealed` to set | +| **DELETE** | Field `closeCh chan struct{}` (~line 70 area) + init in `NewSender` | Caller signal removed | +| **MODIFY** | `Run()` step ordering (lines ~211-300) | New flow: SessionStart → streamBase → BaseDone → streamUntilHead → BarrierReq → BarrierResp → CanEmitSessionComplete | +| **ADD** | New method `streamUntilHead()` | Replaces `streamBacklog()`. ~80 LOC. Body sketched in §2.1.1 | +| **ADD** | Field `idleWindow time.Duration` to `Sender` struct | Default `100ms` (mitigates H1; configurable via constructor) | + +**LOC delta**: ~-120 deleted, ~+80 added → net **~-40 LOC** in sender.go. + +#### 2.1.1 `streamUntilHead()` body sketch + +```go +// streamUntilHead pumps WAL entries from the cursor (initialized to +// fromLSN at session start — the one rewind) forward until the cursor +// has caught up with primary's head AND no new appends arrive within +// idleWindow. The Kind byte flips from Backlog to SessionLive once +// (head - cursor < kindFlipEpsilon); never flips back. +// +// Exit conditions: +// 1. ScanLBAs returns substrate failure → wrap as FailureSubstrate. +// 2. ScanLBAs returns ErrWALRecycled (typed) → wrap as FailureWALRecycled. +// 3. Frame write fails → wrap as FailureWire. +// 4. cursor == head AND idleWindow elapsed → return nil (ready to barrier). +// 5. ctx cancelled → wrap as FailureCancelled. +// +// idleWindow is configurable; default 100ms. Tighter values barrier +// prematurely under sustained writes; longer values stall otherwise- +// caught-up sessions. Pinned by TestSender_IdleWindow_NotPrematureBarrier. +func (s *Sender) streamUntilHead(ctx context.Context) error { + cursor := s.fromLSN // the one rewind happens at NewSender; this is the start point + kind := WALKindBacklog + const kindFlipEpsilon uint64 = 8 + + for { + scanErr := s.primaryStore.ScanLBAs(cursor, func(entry storage.RecoveryEntry) error { + // Frame write + if err := s.writeFrame(frameWALEntry, + encodeWALEntry(kind, entry.LBA, entry.LSN, entry.Data)); err != nil { + return err + } + cursor = entry.LSN + // Kind flip: monotonic, one-way + if kind == WALKindBacklog { + _, _, head := s.primaryStore.Boundaries() + if head-cursor < kindFlipEpsilon { + kind = WALKindSessionLive + } + } + return nil + }) + + // Classify substrate vs wire failure + if scanErr != nil { + // ... FailureSubstrate or FailureWALRecycled or FailureWire + return ... + } + + // Scan exhausted at this snapshot. Decide: loop again or barrier? + _, _, head := s.primaryStore.Boundaries() + if cursor == head { + // Truly caught up at this moment; verify idle window passes. + select { + case <-time.After(s.idleWindow): + _, _, headAgain := s.primaryStore.Boundaries() + if cursor == headAgain { + return nil // exit pump; ready to barrier + } + // head moved during idle window; loop again + case <-ctx.Done(): + return newFailure(FailureCancelled, PhaseBacklog, ctx.Err()) + } + } + // cursor < head: more entries appeared mid-scan. Loop immediately. + } +} +``` + +Bikeshed: method name. `streamUntilHead`, `pumpToHead`, `walPumpLoop` — no preference; mini-plan defaults to `streamUntilHead` for symmetry with deleted `streamBacklog`. Implementation PR can rename if reviewer prefers. + +### 2.2 `core/recovery/sender_test.go` — test catalog churn + +**DELETE** (tests that rely on the gate API): + +| Test | Rationale | +|---|---| +| `TestSender_PushLiveWrite_BeforeDrain_AccumulatesQueue` | API gone | +| `TestSender_PushLiveWrite_AfterSeal_Errors` | API gone | +| `TestSender_DrainAndSeal_FlushesQueueInLSNOrder` | drainAndSeal gone | +| `TestSender_Close_TriggersSealAndDrain` | Close gone | +| `TestSender_TryAdvanceToSteadyLive_FiresAtTransition` | Step gone | + +(Exact test names per the current sender_test.go — implementation PR audits in case names differ.) + +**ADD** (new tests pinning §3.2 #3 behavior): + +| Test | Pins | +|---|---| +| `TestSender_RewindOnce_CursorMonotonicForward` | Sender's cursor starts at fromLSN; primary writes during pump; cursor follows monotonically; barrier hits expected `AchievedLSN`. Uses memorywal substrate with concurrent goroutine writing to primary. | +| `TestSender_KindByte_FlipsOnceAtCatchUp` | Frame sequence shows monotonic Backlog→SessionLive transition; never flips back even if primary bursts mid-stream. Uses memorywal + frame capture in test wire. | +| `TestSender_IdleWindow_NotPrematureBarrier` | Head briefly catches cursor mid-burst; pump waits idle window; primary writes again before window elapses; cursor resumes; barrier eventually fires only at true exhaustion. Default `idleWindow=100ms`; test uses tight `5ms` for speed. | +| `TestSender_StreamUntilHead_WALRecycledMidScan` | Primary's substrate trims past cursor mid-pump; sender returns `FailureWALRecycled` — pinning H5-class boundary. | +| `TestSender_StreamUntilHead_SubstrateIOError` | Substrate returns generic IO error mid-scan; sender wraps as `FailureSubstrate`. | +| `TestSender_StreamUntilHead_CtxCancelDuringIdleWindow` | ctx cancelled while pump sleeps in idle-window `time.After`; returns `FailureCancelled`. | + +**KEEP** (unchanged — must continue passing byte-identical): + +`TestE2E_RebuildHappyPath`, `TestE2E_RebuildWithLiveWritesDuringSession`, `TestRebuildSession_*`, all `TestCoordinator_*` except those touching `TryAdvanceToSteadyLive` (see §2.4). + +**LOC delta**: -100 (deletes) +200 (adds) → net **+100 LOC** in sender_test.go. + +### 2.3 `core/recovery/receiver.go` — monotonic-frame audit + new branches + +**Current**: receiver decodes inbound frames, calls `r.session.ApplyWALEntry(kind, lba, data, lsn)` without checking if `lsn` is monotonic with respect to prior applied LSN. RebuildSession is intentionally permissive (`rebuild_session.go:99-102` notes the layer-1 stance). + +**Per kickoff v0.2 §5 (architect round 2)**: receiver MUST enforce gap + backward checks at the wire boundary, not at substrate. + +**Diff**: + +| Action | Target | Notes | +|---|---|---| +| **ADD** | Field `appliedLSN uint64` to `Receiver` struct | Tracks highest LSN successfully passed to `session.ApplyWALEntry` | +| **MODIFY** | `Receiver.Run()` `case frameWALEntry` (~line 130 area) | Insert monotonic check BEFORE calling `session.ApplyWALEntry` | +| **ADD** | Helper method `checkMonotonic(lsn uint64) error` | Returns typed `*Failure` per §5 table | + +The check: + +```go +// Per kickoff v0.3 §5.1: receiver enforces wire-level monotonic LSN. +// Four cases (row split landed in kickoff v0.3 round 2): +// - lsn == applied + 1: normal (the only legal forward step). +// - lsn > applied + 1: GAP — FailureContract; not silent skip. +// Only legal forward step is +1; sender MUST +// emit contiguous LSN. WAL-recycled gaps go +// through FailureWALRecycled at sender side. +// - lsn == applied: EXACT-DUPLICATE on the wire — FailureProtocol. +// TCP delivers in-order; recovery sender writes +// each LSN exactly once. NOT to be confused with +// §5.2 substrate-level per-LBA arbitration +// (INV-DUAL-LANE-WAL-WINS-BASE) — that's a +// different layer. +// - lsn < applied: BACKWARD — FailureProtocol. Only legitimate +// "rewind" is session-level cursor:=pinLSN at +// a NEW SessionStart, never in-stream. +// +// Initialization at SessionStart (per Q13 ratified default): +// `r.appliedLSN := fromLSN` so the first frame's expected LSN is +// `fromLSN + 1` — sender's first emit after rewind. The watermark +// reading: "fromLSN is the already-installed-on-replica boundary". +func (r *Receiver) checkMonotonic(lsn uint64) error { + expected := r.appliedLSN + 1 + if lsn == expected { + return nil + } + if lsn > expected { + return newFailure(FailureContract, PhaseRecvDispatch, + fmt.Errorf("WAL gap: got LSN=%d, expected %d (applied=%d)", + lsn, expected, r.appliedLSN)) + } + // lsn <= applied: covers both exact-duplicate (lsn == applied) + // and in-stream backward (lsn < applied). Both are FailureProtocol + // per kickoff v0.3 §5.1; only the diagnostic message differs. + if lsn == r.appliedLSN { + return newFailure(FailureProtocol, PhaseRecvDispatch, + fmt.Errorf("WAL exact-duplicate: got LSN=%d == applied=%d "+ + "(sender re-emit; never legitimate on the wire)", + lsn, r.appliedLSN)) + } + return newFailure(FailureProtocol, PhaseRecvDispatch, + fmt.Errorf("WAL backward: got LSN=%d < applied=%d", + lsn, r.appliedLSN)) +} +``` + +After successful `ApplyWALEntry`, update `r.appliedLSN = lsn`. + +**LOC delta**: ~+30 LOC in receiver.go. + +### 2.4 `core/recovery/peer_ship_coordinator.go` — Phase enum collapse (Q3 ratified) + +**Current**: 3-value `PeerShipPhase` enum (`Idle`, `DrainingHistorical`, `SteadyLiveAllowed`). `TryAdvanceToSteadyLive` is the explicit transition; `RouteLocalWrite` returns `RouteSessionLane` for both `DrainingHistorical` and `SteadyLiveAllowed`. + +**Q3 ratified default**: collapse to `{Idle, Active}`. `TryAdvanceToSteadyLive` deletes; `RouteLocalWrite` returns `RouteSessionLane` for any non-Idle phase (which is exactly today's behavior — see existing test `TestCoordinator_RouteLocalWrite` line 114 in `peer_ship_coordinator_test.go`: phase=DrainingHistorical → SessionLane; phase=SteadyLiveAllowed → still SessionLane). + +**Diff**: + +| Action | Target | Notes | +|---|---|---| +| **MODIFY** | Enum constants (~lines 40-42) | `PhaseIdle`, `PhaseActive` (was: `PhaseIdle`, `PhaseDrainingHistorical`, `PhaseSteadyLiveAllowed`) | +| **MODIFY** | `String()` method (~lines 45-55) | Two cases instead of three | +| **DELETE** | `TryAdvanceToSteadyLive` method | Q3 ratified | +| **MODIFY** | `MarkBaseDone` | Stays — still useful for Layer-1 `TryComplete` conjunction. Does NOT advance phase. | +| **MODIFY** | `RecordShipped` (existing, advances `BacklogDrained`) | Stays — `BacklogDrained` flag survives because Layer-1 `TryComplete` still uses it. Phase is separate concept. | +| **MODIFY** | `RouteLocalWrite` (~line 200 area) | Two-case switch: `Idle` → `RouteSteadyLive`; otherwise → `RouteSessionLane` | +| **MODIFY** | All callers of `PhaseDrainingHistorical` / `PhaseSteadyLiveAllowed` | Replace with `PhaseActive` | +| **MODIFY** | `peer_ship_coordinator_test.go` | Update `TestCoordinator_PhaseTransitionRequiresDrainAndBaseDone` semantics: CHK-PHASE-NEVER-STEADY-BEFORE-DRAIN re-anchors per kickoff §6. May rename test. | + +Subtle: `BacklogDrained` and `baseDone` flags are NOT removed — they're still input to Layer-1 `TryComplete` (`INV-SESSION-COMPLETE-ON-CONJUNCTION-LAYER1`). Only the **explicit phase transition** based on them disappears. The flags become diagnostic + gate inputs for `CanEmitSessionComplete`. + +Updated CHK semantics (per kickoff §6): +- `CHK-PHASE-NEVER-STEADY-BEFORE-DRAIN` re-anchors as: "session active = SessionLane until EndSession" (no `Steady*` substate in active session). +- `CHK-NO-FAKE-LIVE-DURING-BACKLOG` re-anchors as: same — single rule. + +**LOC delta**: ~-30 LOC in peer_ship_coordinator.go (deletion-heavy). + +### 2.5 `core/transport/rebuild_sender.go` — workaround removal + +**Current** (`rebuild_sender.go:138-167`): `FinishLiveWrites` called immediately after `StartRebuildSession` to match legacy "no live writes during rebuild" semantic. Comment explicitly says this is the §3.2 #3 workaround. + +**Diff**: + +| Action | Target | Notes | +|---|---|---| +| **DELETE** | Lines 138-144 (workaround comment) | No longer needed; can write a one-line "see kickoff" pointer if reviewer prefers | +| **DELETE** | Lines 164-167 (`FinishLiveWrites` call + race-handling) | Lifted: live writes during rebuild are now legal | + +**LOC delta**: ~-15 LOC in rebuild_sender.go. + +### 2.6 Files NOT touched + +- `core/recovery/wire.go` — wire format is invariant per kickoff §4. Zero changes. +- `core/storage/memorywal/store.go`, `core/storage/walstore.go` — substrate untouched. +- `cmd/blockvolume/main.go`, `cmd/blockmaster/...` — wiring layer untouched. +- `core/replication/component/cluster.go` — no changes. + +If the implementation PR touches any file outside this section without an explicit Round 2 question, that's a scope violation and the reviewer asks why. + +--- + +## 3. New invariant pin — `INV-WAL-CURSOR-MONOTONIC-FROM-PINLSN` + +**Where it lands** in `v3-recovery-inv-test-map.md`: + +``` +| `INV-WAL-CURSOR-MONOTONIC-FROM-PINLSN` | Sender's WAL pump rewinds + cursor to fromLSN once at session start; cursor advances + monotonically per ScanLBAs callback; never decreases. Receiver + enforces matching wire-level monotonic discipline (gap = Contract; + backward = Protocol). | Sender side: `TestSender_RewindOnce_ + CursorMonotonicForward`.
Receiver side: `TestReceiver_ + RejectsBackwardLSN_InSession` + `TestReceiver_RejectsGap_InSession` + (both new in implementation PR). | ✅ pinned | +``` + +Goes in Layer 1 section (between INV-DUAL-LANE-WAL-WINS-BASE and INV-SESSION-COMPLETE-ON-CONJUNCTION-LAYER1) since it concerns receiver-side mechanism. + +--- + +## 4. Test catalog summary + +| Action | Sender | Receiver | Coord | Total | +|---|---|---|---|---| +| DELETE | 5 | 0 | 1 (PhaseTransition rename only) | 5-6 | +| ADD | 6 | 2 | 0 | 8 | +| KEEP | ~all others | all | most | — | + +Net test LOC: **+~250** across recovery package. + +Hardware canonical (`v3-recovery-dual-lane-canonical.yaml`) — **unchanged**. Kickoff §4 wire invariance + §6 INV preservation guarantees this. Mini-plan adds NO new canonical phase; existing phases re-run identically. + +--- + +## 5. G-1 audit scope (V2 read) + +Per `feedback_g1_pre_code_review.md`: muscle-path deletions still get a V2 read. The G-1 stage executes against the implementation branch BEFORE merge. + +**G-1 reading list** (compact): + +1. **V2 reference**: `weed/storage/blockvol/wal_shipper.go` and surrounding (the V2 wal_shipper that V3 ports). G-1 audits whether the v3 sender's deletion of `liveQueue + drainAndSeal` regresses any V2 invariant. Specifically — does V2 ever buffer live writes outside the WAL? If yes, document why V3's "no buffer" is correct (substrate carries it). If no, V2 alignment confirmed. + +2. **V2 receiver discipline**: V2's wal-apply path enforces monotonic LSN how? G-1 confirms V3's `Receiver.checkMonotonic` matches or exceeds V2's strictness. (Memo `feedback_recovery_stale_entry_skip.md` — duplicate-stale-skip is per-LBA, not LSN-monotonic. Different axis.) + +3. **Phase enum collapse vs V2**: V2 had no formal phase enum at this layer. G-1 confirms collapse doesn't lose a V2-tested invariant. Suspected: clean — V2's "session active or not" was implicit; V3's `{Idle, Active}` is a faithful port. + +G-1 produces a `proceed-with-minor-patch` or `revise-mini-plan` verdict. If `revise-mini-plan`, this doc gets a v0.2 round. + +--- + +## 6. LOC budget (refined) + +| File | Delete | Add | Net | +|---|---|---|---| +| `core/recovery/sender.go` | ~120 | ~80 | **-40** | +| `core/recovery/sender_test.go` | ~100 | ~200 | **+100** | +| `core/recovery/receiver.go` | 0 | ~30 | **+30** | +| `core/recovery/receiver_test.go` | 0 | ~80 | **+80** | +| `core/recovery/peer_ship_coordinator.go` | ~40 | ~10 | **-30** | +| `core/recovery/peer_ship_coordinator_test.go` | ~20 | ~10 | **-10** | +| `core/transport/rebuild_sender.go` | ~15 | 0 | **-15** | +| `v3-recovery-inv-test-map.md` | 0 | ~5 | **+5** | +| **TOTAL** | **~295** | **~415** | **+120** (production: -55; tests: +175) | + +Net production code: **-55 LOC** (deletion-heavy as predicted in kickoff). Test surface grows because the new invariants need explicit pins. + +--- + +## 7. Risk mitigations spelled out (kickoff §8 references) + +| Hazard | Kickoff ref | Mini-plan mitigation | +|---|---|---| +| H1 — `idleWindow` value | §8 | Default `100ms`; constructor parameter for tests + future tuning. Pinned by `TestSender_IdleWindow_NotPrematureBarrier`. | +| H2 — `ε` for kind-flip | §8 | Constant `kindFlipEpsilon = 8` LSNs per Q1. Inline-constant; not configurable in this milestone. Re-tunable in Option B if observability demands. | +| H3 — `coord.Phase` collapse | §8 | §2.4 spells out the exact diff. Existing tests audit. | +| H4 — Hardware canonical timeout | §8 | No expected change to canonical YAML. If `observe_recycle_gate_active` phase times out, mini-plan v0.2 round bumps timeout from `30s` → `60s`. | +| H5 — Substrate `ScanLBAs` perf | §8 | Mini-plan does NOT add streaming-API change. If implementation PR profiles show >50ms per scan iteration on production substrate, that's a new question (Q-perf in Round 2). | +| H6 — Receiver discipline behavior change | §8 | The new `checkMonotonic` is **defense-in-depth** for current production senders (which do not send backward in-stream). If a production sender turns out to backward-send under some race, that's a real bug surfaced; do NOT silently skip — fail-loud with FailureProtocol per kickoff §5. | + +--- + +## 8. Sequencing (single PR vs split) + +**Default**: single implementation PR off the (post-kickoff-merge) trunk tip. Reasons: +- File count is small (5 production files + 4 test files). +- Net production diff is deletion-heavy; reviews scale well. +- Splitting into "delete gate" + "add receiver discipline" creates an intermediate state where sender no longer buffers but receiver doesn't yet enforce monotonic — a moment when the discipline contract is asymmetric. Avoid. + +**If reviewer prefers split**: cut `core/recovery/receiver.go` + `receiver_test.go` into a separate prefix PR (#NN-1), gate the sender PR (#NN-2) on it. Asymmetric-state risk is small in practice (no production sender backward-sends today), but reviewer is the judge. + +**Branch base**: implementation branch off post-merged trunk (after both kickoff and mini-plan land in `phase-15`). + +--- + +## 9. Open questions for architect at this stage + +| Q | Topic | Default | +|---|-------|---------| +| **Q10** | Method name `streamUntilHead` vs alternatives (`pumpToHead`, `walPumpLoop`)? | `streamUntilHead` for symmetry with deleted `streamBacklog`; reviewer can rename. | +| **Q11** | `idleWindow` default value `100ms` — too tight, too loose, or fine? | `100ms` is the default; pinned by test using tighter `5ms` for speed. Configurable knob exists. | +| **Q12** | `kindFlipEpsilon = 8` ratified as Q1 — keep as inline constant, or expose as field for tests? | Inline constant in this milestone; expose if Option B observability demands. | +| **Q13** | Receiver's `appliedLSN` field name + initial value (0 vs fromLSN-1) — `lsn == applied+1` rule means initial `applied = fromLSN` so first frame's `lsn = fromLSN+1`. Clear? | Initial `applied = fromLSN` (since `fromLSN` itself is the pinLSN already-applied watermark). | +| **Q14** | Single PR or split (gate-deletion + receiver-discipline)? | Single per §8. | +| **Q15** | After implementation merges, is the unified-wal-kickoff doc itself archived (moved to `sw-block/design/archive/`) or kept in `sw-block/design/`? | Kept; doc is the design source-of-truth for the mechanism, not a one-shot kickoff transcript. | + +Q10-Q15 are mini-plan-stage refinements. If architect ratifies-with-defaults, the implementation PR proceeds. + +--- + +## 10. Resolution log + +### Round 1 — 2026-04-29 (architect ratification) + +**Q10-Q15 ratified-with-defaults**: all six refinement questions accepted at their default positions. No mini-plan content change for Q10-Q15 themselves. + +**§2.3 comment fix (the one real edit in v0.2)**: v0.1 `checkMonotonic` comment block said "First WAL entry of session: applied is 0" — this contradicted Q13's ratified default `appliedLSN := fromLSN`. The two cannot coexist except in the trivial case `fromLSN == 0`. v0.2 corrects the comment to describe the watermark form ("`r.appliedLSN := fromLSN` so the first frame's expected LSN is `fromLSN + 1`"). Cross-references the kickoff v0.3 §5.1 row split. + +**§5.1 row split adoption**: kickoff v0.3 round 2 split `lsn ≤ applied` into two distinct rows (`lsn == applied` exact-duplicate; `lsn < applied` backward). v0.2 `checkMonotonic` body updated to the four-case shape; both exact-duplicate and backward map to `FailureProtocol` (different diagnostic messages, same failure class), so the engine-side branching does not change. No additional implementation cost. + +**Implementation may proceed to G-1**. Per architect Round 1: mini-plan v0.2 closes the design surface; G-1 stage opens next on the implementation branch with the §5 V2 reading list as the audit anchor. + +### Round 2 — (pending; only if G-1 surfaces a `revise-mini-plan` verdict) + +Empty unless G-1 finds a V2-alignment regression that requires diff revision. diff --git a/sw-block/design/v3-recovery-wiring-plan.md b/sw-block/design/v3-recovery-wiring-plan.md new file mode 100644 index 000000000..bbbc00e04 --- /dev/null +++ b/sw-block/design/v3-recovery-wiring-plan.md @@ -0,0 +1,184 @@ +# Recovery wiring plan — `core/recovery` into `core/transport` + cmd + +**Status**: design draft, not yet implemented. Targets architect priority **wiring + 2.5** (the natural next milestone after `g7-redo/pin-floor`). +**Branch target**: `g7-redo/wiring`, stacked on `g7-redo/pin-floor`. +**Depends on**: `g7-redo/dual-lane-recovery-poc` + `g7-redo/pin-floor` merged or reviewed first; this PR is meaningless without them. + +This doc fixes the strategy for moving the dual-lane recovery package from a self-contained POC into the production daemon path, alongside (NOT replacing) the existing single-lane `core/transport` path. Architect ruling: "parallel + flag, don't replace existing path until one happy path runs green alongside". + +--- + +## 1. Why parallel-flag, not in-place replacement + +The existing single-lane path (`core/transport/rebuild_sender.go::doRebuild` + `replica.go::handleConn` MsgRebuildBlock/MsgRebuildDone handlers) is what hardware integration tests (G6 §close, prior G5-5* milestones) currently exercise. A direct in-place replacement of `doRebuild` carries the risk of breaking those without giving QA a deterministic A/B comparison. + +Plan: introduce a flag (config + cmd-line + env) that selects between: + +- `--recovery-mode=legacy` (default) — existing single-lane path, no new code on hot path. +- `--recovery-mode=dual-lane` — the new `core/recovery` package drives rebuild sessions. + +Both paths share the same `BlockExecutor` outer surface (`StartRebuild` / `StartRecoverySession` callers do not care which is used). Wire-format compatibility is NOT required between paths — the dual-lane mode opens its own conn / port (TBD; see §4 below), the legacy mode keeps its existing one. + +Until a green hardware run on `--recovery-mode=dual-lane` is observed, the default stays `legacy`. Removal of the legacy path is a separate PR after at least one milestone of dual-lane GREEN observability. + +--- + +## 2. Surface inventory — files that need changes + +### Sender side (primary) + +| File | Change | Notes | +|---|---|---| +| `core/transport/rebuild_sender.go::StartRebuild` | Branch on mode flag. Legacy path unchanged. Dual-lane path delegates to `recovery.PrimaryBridge.StartRebuildSession`. | Outer signature unchanged so all upstream callers (`core/host/volume/peer_command_executor.go`, `core/transport/recovery_session.go::StartRecoverySession`'s `full_extent` branch, etc.) continue working. | +| `core/transport/recovery_session.go::StartRecoverySession` | The `full_extent` branch (line 117 today) calls `StartRebuild`. No change needed if `StartRebuild` itself branches; the dispatch already lives one level up. | Lowest-touch option. | +| `core/transport/executor.go::BlockExecutor` struct | Add optional fields: `dualLaneCoordinator *recovery.PeerShipCoordinator`, `dualLaneBridge *recovery.PrimaryBridge`. Nil ⇒ legacy mode. | Constructor variant `NewBlockExecutorWithDualLane(...)` initializes these. Existing `NewBlockExecutor` stays legacy. | + +### Receiver side (replica) + +| File | Change | Notes | +|---|---|---| +| `core/transport/replica.go::ReplicaListener` | Two listener configurations: legacy listens on existing port for old MsgRebuildBlock dispatch; dual-lane listens on a separate port (or same port + frame-type discriminator). Decision in §4. | Most invasive surface. | +| `core/transport/replica.go::handleConn` | Branch on first frame: if it's `recovery.frameSessionStart` (0x01), hand off to a `recovery.Receiver` for the rest of the conn lifetime. Otherwise use existing dispatch. | Type byte 0x01 in the new wire vs 0x01 (`MsgShipEntry`) in the old wire — collision; see §4. | +| `core/transport/replica.go::liveShipTargetLSNSentinel` + the lane-shim block | Document as deprecated when dual-lane is active (rebuild lineage no longer rejects live ship — that's the whole point of dual-lane). For legacy mode, keep working. | Removal in a later PR after dual-lane is default. | + +### cmd / daemon + +| File | Change | +|---|---| +| `cmd/blockvolume/main.go` (or wherever the executor is constructed) | Add `--recovery-mode` flag. Legacy: build `NewBlockExecutor(...)`. Dual-lane: build coordinator + bridge + `NewBlockExecutorWithDualLane(...)`. | +| `cmd/blockvolume/main.go` | Add `--recovery-pin-floor-recycle` flag (default off) gating priority 2.5 hookup (see §6). | + +### Replication wiring + +| File | Change | +|---|---| +| `core/replication/component/cluster.go` | If integration tests construct executors directly, propagate the mode flag through the test fixture. Otherwise unchanged. | +| `core/host/volume/peer_command_executor.go` | No change — wraps `BlockExecutor`'s `StartRebuild` which itself branches. | + +--- + +## 3. Lifecycle alignment (architect's "不能做第二套相位机") + +**Rule**: there must be exactly one source of truth for "is replica P in a rebuild session right now" — the `recovery.PeerShipCoordinator`. The legacy `BlockExecutor.activeSession` map is internal to the legacy path; the dual-lane path consults `coordinator.Phase()`. Mode is exclusive — at any moment only one tracker is authoritative for a given replica. + +**StartSession ↔ EndSession**: + +- Legacy mode: `BlockExecutor.registerSession` / `finishSession` pair (existing). +- Dual-lane mode: `coord.StartSession` / `coord.EndSession` pair (already implemented in `core/recovery`). The `PrimaryBridge.StartRebuildSession` calls `StartSession` synchronously before returning to its caller; the goroutine spawned underneath handles `EndSession` via `Sender.Run`'s defer. + +**OnSessionStart / OnSessionClose callbacks** (`adapter.CommandExecutor` surface): + +- Legacy: `BlockExecutor.SetOnSessionStart / SetOnSessionClose` already wired. +- Dual-lane: `PrimaryBridge` accepts the same callback shape; `BlockExecutor` constructor in dual-lane mode forwards its `onStart / onClose` down to `PrimaryBridge`. Caller (`HealthyPathExecutor` etc.) cannot tell which path fired the callback — by design. + +--- + +## 4. Wire-format collision + +**Problem**: legacy `MsgShipEntry = 0x01`, dual-lane `frameSessionStart = 0x01`. If both paths share a port, the receiver cannot dispatch on first byte alone. + +**Three resolution options**: + +### Option A — separate port per mode + +`--rebuild-listen=:9221` (legacy) and `--recovery-listen=:9222` (dual-lane). Daemon binds whichever the mode flag selects. **Pro**: zero protocol coupling; **con**: cmd flag surface grows; integration tests need to know which port. + +### Option B — wire-version handshake on connect + +First byte after dial is a wire-version byte (0x00 = legacy, 0x01 = dual-lane). Subsequent bytes follow the corresponding protocol. **Pro**: single port, cleanly versioned; **con**: existing legacy hardware tests don't send a version byte — backward-compat shim required. + +### Option C — re-number dual-lane frames to avoid 0x01 + +Move `frameSessionStart` to e.g. `0x10`, leaving `0x01..0x07` for legacy. **Pro**: same port, no handshake, cheap; **con**: special-cases the frame-type table; collision-free zone shrinks for future frames. + +**Recommend Option A** for this milestone. Cleanest separation, least risk to the legacy hardware path. Reverts to single-port multiplexing later if/when legacy is removed. + +--- + +## 5. Coordinator instance scope + +One `PeerShipCoordinator` per **volume**, NOT per **executor or per replica**. The coordinator's `MinPinAcrossActiveSessions` is meaningful only when it sees ALL replicas of a single volume. Per-replica coordinators would each report their own pin floor with no minimum. + +**Wiring**: cmd constructs a coordinator at volume open; passes it to all per-replica `BlockExecutor` constructors for that volume. The daemon owns the coordinator lifecycle. + +This is also why the WAL recycle hookup (§6 below) lives at the volume level, not the executor level. + +--- + +## 6. Priority 2.5 — WAL recycle path consumes `MinPinAcrossActiveSessions()` + +**Problem**: today `core/storage/walstore.go`'s flusher / recycle path advances `walTail` based on its own retention policy + checkpoint. It does NOT consult any per-peer pin floor. Until the recycle path knows about active sessions' commitments, `pin_floor` advancement is a paper exercise. + +**Hookup point**: `walstore.go::flusher.advanceTail` (or wherever the flusher decides what to recycle). New gate: + +```go +recycleFloor := computeRetentionFloor() // existing logic +if pinSrc != nil { + if pinned, anyActive := pinSrc.MinPinAcrossActiveSessions(); anyActive { + if pinned < recycleFloor { + recycleFloor = pinned // pin holds back recycle + } + } +} +// trim WAL to recycleFloor +``` + +**Interface seam**: `walstore.WALStore` does not know about `core/recovery` (and shouldn't — substrate stays orthogonal to engine). Solution: a small interface in `core/storage`: + +```go +type RecycleFloorSource interface { + MinPinAcrossActiveSessions() (floor uint64, anyActive bool) +} + +func (s *WALStore) SetRecycleFloorSource(src RecycleFloorSource) +``` + +`recovery.PeerShipCoordinator` already implements this signature exactly. cmd wires the coordinator into the volume's WAL store after both are constructed. Default: nil source (legacy behavior). + +**Test**: a new `walstore_recycle_pin_test.go` confirms recycle floor is gated by min pin when source is set; falls back to existing logic when nil. No e2e change required — the unit test is sufficient because the coordinator's contract is already covered. + +**Architect priority 2.5 closes when this hookup lands** AND a brief test confirms the integration works end-to-end (set pin floor on coord, observe recycle path respect it). + +--- + +## 7. Test plan + +Existing tests must continue to pass on `--recovery-mode=legacy` (default). The wiring PR adds: + +| Test | Scope | +|---|---| +| `core/transport/rebuild_sender_dual_lane_test.go` | Dual-lane mode end-to-end with the existing `BlockExecutor` shape: same OnSessionStart/Close callbacks fire, achievedLSN equals primary H. | +| `core/transport/replica_dual_lane_test.go` | Receiver-side dispatch for `frameSessionStart` correctly hands off to a `recovery.Receiver`. | +| `core/storage/walstore_recycle_pin_test.go` | §6 — recycle path consults `MinPinAcrossActiveSessions`. | +| Existing `TestE2E_*` in `core/recovery/` | UNCHANGED — they use `core/recovery` directly, not `core/transport`. Wiring PR doesn't touch them. | + +Hardware acceptance (G7 §2 #2 / #5 / #6): NOT a gate for this PR. Architect ruling: "validate happy-path one round green alongside legacy first, then enable as default in a separate PR". + +--- + +## 8. Rollback / removal plan + +If something is critically wrong with the dual-lane path post-merge: + +- **Default flip back**: change `--recovery-mode` default in cmd from `legacy` to `dual-lane` (or vice versa) is a single-line revert. +- **Code removal**: requires reverting the wiring PR. Because `core/recovery` is independent of `core/transport`, the package itself can stay even if the integration is rolled back. +- **legacy removal**: a future PR removes `core/transport/rebuild_sender.go::doRebuild` + the old wire frames. Should NOT be in this PR. Track as `g7-redo/legacy-removal`. + +--- + +## 9. Open questions for architect — Resolved (architect ACK 2026-04-29) + +1. **Wire collision** (§4): **RESOLVED — Option A (separate ports)**. Daemon binds a second listen address; `--recovery-mode` flag pairs with it. Firewall / Helm registration must follow. Fallback to Option B (version handshake) is permitted only if a platform constraint requires single-port. Option C (re-number frames) is rejected as a long-term solution. +2. **2.5 split**: **RESOLVED — split into `g7-redo/recycle-pin-hookup`**. Wiring PR stays focused on transport + cmd + lifecycle. Recycle hookup lands immediately after wiring merges, NOT held for a long gap (otherwise pin advances in the air while flusher recycles by old policy). +3. **Default mode flip timing**: **RESOLVED — stays `legacy`** until CI + hardware-targeted dual-lane scenarios run GREEN, then a separate single-line PR flips the default. Default flip is a release / ops event, not a wiring implementation detail. +4. **Legacy removal timing**: **RESOLVED — after default flip + at least one release cycle (canary / N weeks)**, NOT just calendar time. Pre-removal checklist: no-flag run green, monitoring / rollback documented. Tracked as `g7-redo/legacy-removal`. + +--- + +## 10. Out of scope (explicit) + +- `§3.2 #3 single-queue real-time interleave` (architect priority #2). Wiring PR ships sequential phases, same as POC. +- `INV-REPL-OVERLAP-HISTORY-NO-REGRESS` bitmap-as-WAL-claim refinement. Separate PR after apply-gate redesign. +- Sparse base + substrate basement-clearing. +- Layer 3 retry budget at engine level (architect priority #3 [retry]). Architect: "可与 wiring 并行 ... 不同目录"; tracked as a separate concurrent PR. +- Removing `core/transport/replica.go::liveShipTargetLSNSentinel` and the lane-shim. Lives until legacy is removed.