Repository navigation
fix: make on-disk header and WAL creation crash-safe under SIGKILL (#308) - #349
Merged
Merged
Conversation
) A SIGKILL landing mid-checkpoint had a ~26% chance of permanently corrupting the database with no recovery path. Root cause: FileBackend::write_page rewrote the header page on every single fact/index page append (to bump page_count) but left header_checksum stale, so a kill before the real commit in PersistentFactStorage::save left an on-disk header whose checksum didn't match its own content. Fix: recompute header_checksum every time write_page persists a page_count bump, so the header is self-consistent at every write, not just at the deliberate final commit. The new SIGKILL fuzz test (tests/crash_kill_test.rs) also caught a second, related bug: WalWriter::open_or_create creates the .wal file and writes its header in two separate steps, so a kill in between left a too-short WAL that validate_wal_header's read_exact choked on, equally blocking reopen. Since no entry can be appended before a WalWriter fully exists, a WAL shorter than its own header is always safe to treat as empty; WalReader::open and open_or_create now do so instead of erroring. Test coverage: a small round count runs unconditionally in the per-PR suite; .github/workflows/crash-kill.yml runs 100 rounds nightly for statistical confidence, following the existing nfs-lock.yml/ llvm-cov.yml pattern (including filing/commenting on a tracking issue on failure). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nsh3ZnmoK6PTaUbp4HcqHY
test_write_page_keeps_disk_header_checksum_consistent_on_page_count_bump opened the file a second time via std::fs::read() while `backend` still held it locked. Unix tolerates a second open from the same process; Windows does not, failing CI with "another process has locked a portion of the file". Read page 0 back through backend.read_page() instead, which uses the already-open, already-locked handle. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nsh3ZnmoK6PTaUbp4HcqHY
#308's root cause was ordering of our own synchronous write calls, not an OS-specific durability boundary -- it affected Windows and macOS exactly as much as Linux. The per-PR smoke run already covers all three (tests/crash_kill_test.rs isn't OS-gated, and rust.yml's build matrix already runs it there), but the nightly 100-round confidence run was Linux-only, copied from nfs-lock.yml/llvm-cov.yml whose Linux-only scope is for real infra reasons (NFS server, coverage tooling) that don't apply here. Matrix it across ubuntu/windows/macos instead, with a per-OS tracking issue title to avoid races between concurrently failing legs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nsh3ZnmoK6PTaUbp4HcqHY
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SIGKILLlanding mid-checkpoint had a ~26% chance of permanently corrupting the database with no recovery path.FileBackend::write_pagerewrote the header page on every single fact/index page append (to bumppage_count) but leftheader_checksumstale, so a kill before the real commit inPersistentFactStorage::saveleft an on-disk header whose checksum didn't match its own content. Fix: recomputeheader_checksumevery timewrite_pagepersists apage_countbump, so the header is self-consistent at every write, not just at the deliberate final commit.WalWriter::open_or_createcreates the.walfile and writes its header in two separate steps, so a kill in between left a too-short WAL thatvalidate_wal_header'sread_exactchoked on, equally blocking reopen. Since no entry can be appended before aWalWriterfully exists, a WAL shorter than its own header is always safe to treat as empty;WalReader::openandopen_or_createnow do so instead of erroring.tests/crash_kill_test.rs: spawns a child that loopstransact+checkpoint()on a fresh.graphfile and kills it with a realSIGKILLat a random point mid-loop, then confirms a fresh process can still open the file. A small round count (5) runs unconditionally in the per-PR suite;.github/workflows/crash-kill.ymlruns 100 rounds nightly for statistical confidence (following the existingnfs-lock.yml/llvm-cov.ymlpattern, including filing/commenting on a tracking issue on failure).src/wal.rsand 1 insrc/storage/backend/file.rslocking in each invariant without relying on timing.Test plan
cargo test— full suite green (694 lib tests + all integration suites)cargo fmt --check,cargo clippy --all-features -- -D warnings— cleanfile.rsfix and confirmedcrash_kill_test.rsreproduces the exact original failure (Header checksum mismatch: possible file corruption)MINIGRAF_CRASH_KILL_ROUNDS=200+ against the fix, run 5 times (1000+ total rounds), zero corruptionCo-Authored-By: Claude Sonnet 5 noreply@anthropic.com
https://claude.ai/code/session_01Nsh3ZnmoK6PTaUbp4HcqHY