Skip to content

test(persistence): fix crash_recovery_disk_offload_no_aof floor-vs-ground-truth mismatch (task #44) #328

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pilotspacex-byte merged 1 commit into from
Jul 14, 2026
Merged

test(persistence): fix crash_recovery_disk_offload_no_aof floor-vs-ground-truth mismatch (task #44) #328

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,46 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed — `crash_recovery_disk_offload_no_aof` harness assumed eviction-throughput durability the write path no longer provides (task #44)

`cold_keys_recover_after_crash_without_aof` forced LRU eviction with a
16,000-key pipelined `SET` burst and asserted `post >= 65% of PROBE_COUNT`,
assuming most evicted probes land durably in a `heap-*.mpf` file. That
predates `disk_offload_spill_inert()` / PR #273 (policy-aware eviction
fail-close): under `--appendonly no`, the per-connection write-path eviction
gate (`run_write_eviction_gate`, `src/server/conn/handler_monoio/mod.rs`) —
the path every ordinary `SET`/`HSET` past `maxmemory` actually takes — has no
`ShardManifest` handle and PLAIN-DROPS victims (documented Redis "pure cache,
no durability" semantics; see `docs/PRODUCTION-CONTRACT.md`'s CRASH-02 note
on task #57, which made this drop-not-OOM behavior the intended fix). Only
the periodic memory-pressure tick (`shard/persistence_tick.rs`) durably
spills under `appendonly no`, and a busy connection's synchronous per-write
eviction always wins the race before that tick observes a sustained
over-budget shard — so the pipelined-`SET` burst durably spilled 0-1/200
probes (ground-truth-verified via direct manifest/`heap-*.mpf` inspection),
Comment on lines +24 to +25

@coderabbitai coderabbitai Bot Jul 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Correct the reported spill-rate range.

The supplied test rationale reports 1-51/200 durably spilled probes across runs, but this changelog says 0-1/200 for the same pipelined-SET burst. Align the number with the recorded evidence or state that it was a particular run.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@CHANGELOG.md` around lines 24 - 25, Update the pipelined-SET burst spill-rate
statement in CHANGELOG.md from “0-1/200” to the evidence-supported “1-51/200”
range, or explicitly label the existing value as a particular run rather than a
general result.

making the 65% floor permanently unreachable. Not a data-loss regression:
the dropped keys were never claimed durable (the boot-time WARN says so
explicitly for this exact config).

Two changes, no `src/` change: (1) `write_filler` now sends the filler as
ONE `MSET` instead of 16,000 pipelined `SET`s — `MSET`'s handler applies all
pairs with no per-key eviction check, and the write-path gate that does run
checks memory once, pre-`MSET`, so a shard can end up far over budget with no
further write pending to re-trigger the plain-drop path; the periodic tick is
then the only thing left to reclaim it, durably spilling the LRU-oldest keys
(the probes) via the manifest. (2) The fixed `RECOVERY_FLOOR` is replaced by
`count_durable_probe_entries`, which reads each shard's manifest + `Active`
`KvLeaf` DataFiles directly (the same way `recover_shard_v3_pitr` does at
boot) right before the kill to establish ground truth, and asserts recovery
returns AT LEAST that many probes — the actual #22 regression signal,
decoupled from eviction throughput. Verified the rewritten test still
red-flags a reintroduced #22 regression (manually reverted the
`persistence_dir.is_some() || disk_offload_base.is_some()` gate in
`main.rs`, confirmed RED, reverted back). Green 10×+ consecutive on macOS
(monoio + tokio runtimes); `crash_recovery_cold_del_resurrection` and the
`crash_matrix_cross_plane` no-durability-contract spot check show no
regression.

### Fixed — `crash_recovery_graph_durability` g1/g2/g3 harness polled the deleted WAL v2 flat file (task #31)

The G1–G3 legacy-mode graph crash tests never actually ran their kill -9
Expand Down
3 changes: 2 additions & 1 deletion docs/PRODUCTION-CONTRACT.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ per the CI gate · Blocking `—` = tracked but never blocks a tag (see Out of S
| Status | ID | Item | Evidence | Blocking |
|---|---|---|---|---|
| ✅ | WAL-01 | WAL v3 unified log, per-shard group commit, checksum-guarded records | `src/persistence/page.rs`, `kv_page.rs`, `clog.rs`, `manifest.rs`; `fuzz/fuzz_targets/wal_v3_record.rs` | GA |
| ✅ | CRASH-01 | Scripted crash-injection matrix (per-shard AOF, cold-tier DEL resurrection, disk-offload, graph, vacuum, vector durability) | `tests/crash_matrix_per_shard_aof.rs`, `crash_matrix_per_shard_bgrewriteaof.rs`, `crash_recovery_cold_del_resurrection.rs`, `crash_recovery_disk_offload_no_aof.rs`, `crash_recovery_graph_durability.rs`, `crash_recovery_vacuum.rs`, `crash_recovery_vector_durability.rs`; run by `.github/workflows/integration-tests.yml` jobs `Durability Tests` + `Crash Matrix (per-shard AOF)`. **2026-07-14 audit caveats:** the graph-durability suite's g1–g3 never actually ran until PRs #322/#324 (harness polled the deleted WAL v2 flat file AND the legacy-mode replay it guards was a silent no-op — both fixed, g1–g5 green 3× macOS / 5× Linux); `crash_recovery_disk_offload_no_aof` is currently red on main (task #44, open). | GA |
| ✅ | CRASH-01 | Scripted crash-injection matrix (per-shard AOF, cold-tier DEL resurrection, disk-offload, graph, vacuum, vector durability) | `tests/crash_matrix_per_shard_aof.rs`, `crash_matrix_per_shard_bgrewriteaof.rs`, `crash_recovery_cold_del_resurrection.rs`, `crash_recovery_disk_offload_no_aof.rs`, `crash_recovery_graph_durability.rs`, `crash_recovery_vacuum.rs`, `crash_recovery_vector_durability.rs`; run by `.github/workflows/integration-tests.yml` jobs `Durability Tests` + `Crash Matrix (per-shard AOF)`. **2026-07-14 audit caveats:** the graph-durability suite's g1–g3 never actually ran until PRs #322/#324 (harness polled the deleted WAL v2 flat file AND the legacy-mode replay it guards was a silent no-op — both fixed, g1–g5 green 3× macOS / 5× Linux); `crash_recovery_disk_offload_no_aof` was harness rot, not a data-loss regression — its 65%-eviction-throughput floor predated PR #273's intentional plain-drop write path and was structurally unreachable; fixed (task #44) by asserting against ground truth read directly from the manifest instead, green 10×+ macOS (monoio + tokio). | GA |

@coderabbitai coderabbitai Bot Jul 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Document the complete ground-truth source.

The test derives durable probes from each shard’s manifest and active KvLeaf heap-*.mpf files, not from the manifest alone. Update this evidence description to match the implementation and the more precise wording already used in the revision history.

🧰 Tools
🪛 LanguageTool

[uncategorized] ~111-~111: The official name of this software platform is spelled with a capital “H”.
Context: ..._recovery_vector_durability.rs; run by .github/workflows/integration-tests.ymljobs...

(GITHUB)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/PRODUCTION-CONTRACT.md` at line 111, Update the CRASH-01 evidence
description in the production contract to state that durable probes come from
each shard’s manifest and active KvLeaf heap-*.mpf files, rather than claiming
the manifest alone is the ground-truth source. Use the same precise terminology
established in the revision history while preserving the existing test and audit
details.

| ✅ | CRASH-02 | Cross-plane kill-9 crash matrix — every plane pair (KV/vector/graph/FTS/WS/MQ/temporal/txn) survives kill -9 with zero synced-write loss, both legacy and disk-offload modes | `tests/crash_matrix_cross_plane/` (37 cells, PR #298, v0.8 storage-kernel G1 gate). The matrix's 4 RED root-cause groups were each fixed with the cell as regression proof: cross-store MULTI/EXEC graph leg (task #52), checkpoint-Finalize graph total-loss window (task #53), eviction plain-drop under write pressure (task #57), legacy graph WAL replay no-op (task #60, PR #322). | GA |
| ✅ | MEM-10X-01 | 10× RAM datasets under disk-offload: bounded resident set, startup readiness not O(spilled-keys) | v0.8 storage-kernel G2 acceptance run (2026-07-13): all 3 criteria PASS — restart readiness 157s → 3.7s at 236K spilled files (PR #319 crash-orphan sweep off the critical path), unified resident-bytes accounting + elastic budget (PR #297), allocator-overhead + PageCache observability (PR #320). Known follow-up: spill-file batching (tracked, non-blocking). | GA |
| ✅ | JEPSEN-01 | Jepsen-lite linearizability suite | `tests/jepsen_lite.rs`, run by `integration-tests.yml` job `Durability Tests` | GA |
Expand Down Expand Up @@ -212,3 +212,4 @@ Never blocks any tag (deliberate, standing non-goals — see ROADMAP §7):
| 2026-04-08 | Initial publication — provisional SLO numbers from v0.1.2 benchmark memory; full checklist structure locked (Phase 87) |
| 2026-07-10 | Full refresh to v0.6.0 reality (ROADMAP `H-5`): converted to an evidence-linked checked ledger, dropped the defunct Phase-87..100 numbering in favor of the v0.6.1→v1.0 release train, added `scripts/check-production-contract.sh` as a CI gate (soft-report pre-v1.0, hard-fail at `v1.0*` tags). Verification pass found two undocumented defects: `SUPPLY-01` (`deny.toml` claims a CI job that does not exist) and a broken 12th fuzz target (`graph_props_record`, `FUZZ-01`) — both left unticked rather than assumed. |
| 2026-07-14 | v0.7.0-prep reconciliation: re-ticked `FUZZ-01` (12th target restored), `ACL-REG-01` (PR #258), `COLD-TTL-01` (`ColdIndex::sweep_expired`), `SEC-07` (release-agnostic policy); updated `FT-PARITY-01` (AGGREGATE shipped, ALTER open). Added honesty caveats to `CRASH-01` (g1–g3 harness never ran pre-#322/#324; task #44 residual red). New rows for shipped-but-previously-untracked guarantees: `CRASH-02` (37-cell cross-plane kill-9 matrix, v0.8 G1), `MEM-10X-01` (10× RAM acceptance, v0.8 G2), `REPL-PLANES-01` (all-plane replication, Waves A/B), and the pending `REPL-SOAK-01` gate for the v0.7.0 tag. |
| 2026-07-14 | task #44 closed: `crash_recovery_disk_offload_no_aof` was harness rot — its fixed 65%-of-`PROBE_COUNT` eviction-throughput floor predated PR #273's policy-aware plain-drop write path and was structurally unreachable (ground-truth-verified 0-1/200 durably spilled under the old pipelined-`SET` burst), not a data-loss regression (the boot-time `disk_offload_spill_inert` WARN already documents this exact config as non-durable on the write path). Fixed test-only: `write_filler` now drives the periodic manifest-backed memory-pressure tick via a single `MSET` instead of racing it with per-write plain-drop, and the assertion reads ground truth directly from each shard's manifest + `heap-*.mpf` files instead of a fixed floor. Verified the rewrite still catches a manually-reintroduced #22 regression; `CRASH-01` caveat updated. |
Loading
Loading