fix(persistence): legacy-mode graph WAL replay dropped raw RESP payload as cmd name (task #60) by pilotspacex-byte · Pull Request #322 · pilotspace/moon

pilotspacex-byte · 2026-07-13T16:47:02Z

Problem (silent data loss — v0.8 "kill-9-lossless every plane" blocker)

In --disk-offload disable (legacy) mode, GRAPH.* state was silently discarded on every kill-9 restart — GRAPH.CREATE/ADDNODE/… since the last graceful shutdown were lost entirely (ERR graph not found).

Root cause

replay_graph_wal (src/shard/shared_databases.rs) passed the raw RESP-encoded WalRecord::payload directly as the cmd argument to CommandReplayEngine::replay_command, which expects a bare command name + already-parsed &[Frame] args. GraphReplayCollector::is_graph_command compares against literals like b"GRAPH.CREATE", so it never matched the multi-line RESP blob → graph_command_count() stayed 0 → the replay_graph_commands guard never fired. Regression from PR #236 (WAL v2→v3 port); the disk-offload-enabled path (replay_graph_wal_v3) was unaffected because it already parses correctly.

Fix

Parse each WalRecord payload with crate::protocol::parse::parse, take arr[0] as the command name and arr[1..] as args — the exact parse-then-dispatch pattern already proven in the sibling replay_graph_wal_v3 and recovery.rs's KV replay. ~20 lines, one function.

Un-gates the 6 crash-matrix cells that were RED for this reason (red_guard): graph_isolated, txn_isolated_committed, txn_isolated_atomicity, mixed_all_planes_synced, mixed_all_planes_mid_pass_c (legacy s1) + cross_plane_spot_legacy_s4_graph_isolated.

Verification (moon-dev VM, real kill-9)

6/6 previously-RED cells green across 3 soak runs
No regression on disk-offload-enabled graph cells (prod_s1/s4 graph_isolated + graph_drop_survives_repeated_checkpoints)
cargo fmt --check + clippy -D warnings clean on default AND runtime-tokio,jemalloc

…ad as cmd name (task #60) replay_graph_wal (src/shard/shared_databases.rs), the --disk-offload disable path used when persistence_dir is set but disk_offload_base is None, passed the raw RESP-encoded WalRecord::payload directly as the `cmd` argument to CommandReplayEngine::replay_command instead of parsing it into frames first. GraphReplayCollector::is_graph_command compares against literal command names like b"GRAPH.CREATE", so it never matched the multi-line RESP blob -- graph_command_count() stayed 0, the replay_graph_commands guard never fired, and the entire graph plane was silently discarded on every kill-9 restart in legacy mode. Regression from PR #236 (WAL v2 -> v3 port). Fix mirrors the parse-then-dispatch pattern already used by the sibling replay_graph_wal_v3 (disk-offload-enabled path) and recovery.rs's KV Command replay: parse each WalRecord payload with crate::protocol::parse::parse, extract the bare command name from arr[0], and pass arr[1..] as args. Un-gates the 6 crash-matrix cells that were RED for this exact reason (harness::red_guard, MOON_CRASH_MATRIX_RED=1) in tests/crash_matrix_cross_plane/{tests_legacy.rs,tests_spot.rs}: graph_isolated, txn_isolated_committed, txn_isolated_atomicity, mixed_all_planes_synced, mixed_all_planes_mid_pass_c (legacy s1) and cross_plane_spot_legacy_s4_graph_isolated. Verified on moon-dev VM: 6/6 green across 3 soak runs, no regression on the disk-offload-enabled graph cells (prod_s1/s4 graph_isolated + graph_drop_survives_repeated_ checkpoints), cargo fmt --check clean, clippy -D warnings clean on both default and runtime-tokio+jemalloc feature sets. author: Tin Dang

qodo-code-review · 2026-07-13T16:47:07Z

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

coderabbitai · 2026-07-13T16:47:12Z

Warning

Review limit reached

@pilotspacex-byte, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 51 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5fd2a2d7-3b67-4614-9e67-05a5c034b325

📥 Commits

Reviewing files that changed from the base of the PR and between f6518c5 and ce51bb1.

📒 Files selected for processing (4)

CHANGELOG.md
src/shard/shared_databases.rs
tests/crash_matrix_cross_plane/tests_legacy.rs
tests/crash_matrix_cross_plane/tests_spot.rs

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/task-60-legacy-graph-wal-replay

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

…lat-file probe (task #31) (#324) The G1-G3 legacy-mode graph crash tests never reached their kill -9 scenario: wait_for_wal_bytes and G3's replay-idempotency length check still polled the flat <dir>/shard-0.wal WAL v2 layout, deleted in PR #236 one day before this test file was added (#237). The probe timed out after 20s waiting for a file that is never created — on every platform (Linux VM reproduces identically; not a macOS durability gap) — masked because the suite is #[ignore]d and no CI job runs it with --ignored. Both helpers now scan the real <dir>/shard-0/wal-v3/*.wal segment directory. The new wal_dir_total_bytes helper for G3 excludes each segment's fixed 64-byte WAL_V3_HEADER_SIZE so the fresh active segment each boot opens isn't misread as replay-time double-append. The underlying replay_graph_wal RESP-parse bug these tests expose (graph plane silently dropped on legacy-mode restart) was fixed separately in PR #322 (task #60); this harness fix is what lets g1-g3 prove it. Verification: g1-g5 green 3x consecutively on macOS (release-fast, MOON_BIN pinned to fresh binary per the stale-target/release trap); agent run additionally verified 5/5 on the Linux dev VM. fmt clean. Refs: task #31, PR #236, PR #237, PR #322 author: Tin Dang Co-authored-by: Tin Dang <tindang.ht97@gmail.com>

…(v0.7.0 prep) Owner reconciliation 2026-07-14 of docs/PRODUCTION-CONTRACT.md against the tree (main is 67 commits past the v0.6.0 tag, carrying the full v0.6.1 hygiene scope and the v0.7 replication workstreams untagged): Re-ticked with verified evidence: - FUZZ-01: graph_props_record.rs restored, all 12 declared targets exist - ACL-REG-01: TXN/WS/MQ/TEMPORAL/CDC in metadata.rs + monoio intercept reorder (PR #258) - COLD-TTL-01: ColdIndex::sweep_expired + info_reclamation counters - SEC-07: SECURITY.md now states a release-agnostic supported-versions policy (latest minor line pre-1.0, LTS from v1.0.0) Updated honestly: - FT-PARITY-01: FT.AGGREGATE shipped (ft_aggregate.rs); FT.ALTER still absent -- stays unticked - CRASH-01: caveats recorded -- graph-durability g1-g3 never actually ran before PRs #322/#324 (WAL v2 flat-file probe + legacy replay no-op); crash_recovery_disk_offload_no_aof residual red (task #44) - SUPPLY-01: supply-chain.yml workflow in flight (task #63), tick on merge New rows for shipped-but-previously-untracked guarantees: - CRASH-02: 37-cell cross-plane kill-9 matrix (v0.8 G1, PR #298 + the 4 RED-group fixes #52/#53/#57/#60) - MEM-10X-01: 10x RAM datasets G2 acceptance (restart readiness 157s->3.7s, PRs #319/#297/#320) - REPL-PLANES-01: all-plane replication (Waves A/B, PRs #285/#294) - REPL-SOAK-01 (unticked): 24h replication soak gates the v0.7.0 tag Header refreshed (milestone = v0.7.0 Replication GA, tag gated on soak). scripts/check-production-contract.sh parses all new rows; GA-blocking gap is now an honest 17 rows. Refs: tasks #61-#65 author: Tin Dang

…(v0.7.0 prep) (#325) Owner reconciliation 2026-07-14 of docs/PRODUCTION-CONTRACT.md against the tree (main is 67 commits past the v0.6.0 tag, carrying the full v0.6.1 hygiene scope and the v0.7 replication workstreams untagged): Re-ticked with verified evidence: - FUZZ-01: graph_props_record.rs restored, all 12 declared targets exist - ACL-REG-01: TXN/WS/MQ/TEMPORAL/CDC in metadata.rs + monoio intercept reorder (PR #258) - COLD-TTL-01: ColdIndex::sweep_expired + info_reclamation counters - SEC-07: SECURITY.md now states a release-agnostic supported-versions policy (latest minor line pre-1.0, LTS from v1.0.0) Updated honestly: - FT-PARITY-01: FT.AGGREGATE shipped (ft_aggregate.rs); FT.ALTER still absent -- stays unticked - CRASH-01: caveats recorded -- graph-durability g1-g3 never actually ran before PRs #322/#324 (WAL v2 flat-file probe + legacy replay no-op); crash_recovery_disk_offload_no_aof residual red (task #44) - SUPPLY-01: supply-chain.yml workflow in flight (task #63), tick on merge New rows for shipped-but-previously-untracked guarantees: - CRASH-02: 37-cell cross-plane kill-9 matrix (v0.8 G1, PR #298 + the 4 RED-group fixes #52/#53/#57/#60) - MEM-10X-01: 10x RAM datasets G2 acceptance (restart readiness 157s->3.7s, PRs #319/#297/#320) - REPL-PLANES-01: all-plane replication (Waves A/B, PRs #285/#294) - REPL-SOAK-01 (unticked): 24h replication soak gates the v0.7.0 tag Header refreshed (milestone = v0.7.0 Replication GA, tag gated on soak). scripts/check-production-contract.sh parses all new rows; GA-blocking gap is now an honest 17 rows. Refs: tasks #61-#65 author: Tin Dang Co-authored-by: Tin Dang <tindang.ht97@gmail.com>

… item 4) (#352) Adds .github/workflows/crash-matrix.yml, running tests/crash_matrix_cross_plane.rs (kernel M3/G1's 46-cell kill-9 durability suite covering KV/graph/vector/WS/MQ/cross-store-TXN durability across appendonly x disk-offload x shards) on the self-hosted moon-dev runner: - nightly (03:17 UTC): full matrix, default single iteration. - weekly soak (Saturday 04:41 UTC): MOON_CRASH_MATRIX_ITERS=20 on the 10 cells with a probabilistic kill point (mid_checkpoint, graph_drop_survives_repeated_checkpoints, txn_isolated_*) -- the cells kernel M3 stage 2 (task #53) proved need repeated sampling, not a single pass, to catch a timing-window regression (pre-fix baseline failed at iteration 7/20 and 11/20). Both jobs support workflow_dispatch for on-demand runs. Deliberately NOT wired into the per-PR gate: moon-dev is a single self-hosted runner and PR-time is already tight (same rationale as integration-tests.yml's ci-full label gate). Confirmation run on the moon-dev Linux VM (fresh ELF release binary built in-worktree via `cargo build --release --test crash_matrix_cross_plane`, CARGO_TARGET_DIR=target-linux, MOON_BIN pinned, main @ ec08455): - Full 46-cell matrix: 2/2 consecutive green runs (46 passed; 0 failed; ~79s each), including every former RED cell -- the 6 legacy-mode graph cells (task #60 / PR #322) and the MQ generic-DEL resurrection cell (task #46 / PR #301) now run fully ungated (no harness::red_guard call sites remain in the suite). - MOON_CRASH_MATRIX_ITERS=5 soak of the 10 probabilistic-kill-point cells: 10 passed; 0 failed (40.3s) -- same cells whose pre-fix baseline failed mid-soak. CHANGELOG.md [Unreleased] updated with the confirmation-run evidence. author: Tin Dang <tindang.ht97@gmail.com> Co-authored-by: Tin Dang <tindang.ht97@gmail.com>

pilotspacex-byte merged commit 0270e98 into main Jul 13, 2026
8 checks passed

This was referenced Jul 13, 2026

test(persistence): fix crash_recovery_graph_durability g1-g3 WAL v2 flat-file probe (task #31) #324

Merged

docs(contract): reconcile GA Exit Ledger to post-replication-GA main (v0.7.0 prep) #325

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(persistence): legacy-mode graph WAL replay dropped raw RESP payload as cmd name (task #60)#322

fix(persistence): legacy-mode graph WAL replay dropped raw RESP payload as cmd name (task #60)#322
pilotspacex-byte merged 1 commit into
mainfrom
fix/task-60-legacy-graph-wal-replay

pilotspacex-byte commented Jul 13, 2026

Uh oh!

qodo-code-review Bot commented Jul 13, 2026

Uh oh!

coderabbitai Bot commented Jul 13, 2026

Review limit reached

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

pilotspacex-byte commented Jul 13, 2026

Problem (silent data loss — v0.8 "kill-9-lossless every plane" blocker)

Root cause

Fix

Verification (moon-dev VM, real kill-9)

Uh oh!

qodo-code-review Bot commented Jul 13, 2026

Qodo reviews are paused for this user.

Uh oh!

coderabbitai Bot commented Jul 13, 2026

Review limit reached

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants