Skip corrupted WAL records during recovery instead of failing startup#115
Skip corrupted WAL records during recovery instead of failing startup#115mweiden wants to merge 1 commit into
Conversation
A single damaged byte anywhere in the commit log (torn write, bad base64, invalid UTF-8 key) made parse_entries return an error, which Database::new propagated, leaving the node unable to start at all. Recovery now drops malformed records with a warning and replays the rest, matching the usual LSM commit-log recovery contract: a torn tail must not take down the node or discard the valid prefix of the log. https://claude.ai/code/session_01CJXWjB9ERGgV6B9mRqrdM9
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3994391722
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let path = path.into(); | ||
| let buf = storage.get(&path).await.unwrap_or_default(); | ||
| let entries = parse_entries(&buf)?; | ||
| let entries = parse_entries(&buf); |
There was a problem hiding this comment.
When startup recovers from a torn WAL whose last record has no trailing newline, this call now skips the bad record but the WalState is still initialized with the original buf and flushed = buf.len(). The next insert therefore gets appended directly after the corrupt tail bytes (see Wal::append extending state.data), so the on-disk line becomes something like k2\tdjEk3\t...\n; on the following restart that whole merged line is skipped and the successfully acknowledged k3 write is lost. Recovery needs to rewrite/truncate the WAL to the last valid record boundary (or otherwise insert a delimiter) before marking the existing bytes flushed.
Useful? React with 👍 / 👎.
2 participants
Problem
parse_entriesreturned an error on the first malformed WAL record (invalid UTF-8 key, bad base64 value), andDatabase::newpropagated it — so a single damaged byte anywhere in the commit log (e.g. a torn write from a crash mid-append) left the node unable to start at all, discarding the valid log prefix along with it.Fix
Recovery now drops malformed records (missing separator, bad UTF-8, bad base64) with a
tracing::warn!that reports how many were skipped and how many were recovered, and replays the rest. This matches the usual LSM commit-log recovery contract: a torn tail must not take down the node.Tests
database_recovers_past_corrupted_wal_records: a WAL containing invalid base64, a separator-less line, and an invalid-UTF-8 key followed by a valid record — startup succeeds, the valid record is replayed, the corrupt ones are dropped (replaces the old test that asserted the fail-hard behavior).wal_recovery_survives_torn_tail: simulates a crash mid-append (partial record, truncated base64, no trailing newline) and verifies the earlier record survives restart.Found by codebase audit (high severity: single corrupt byte bricks node startup).
https://claude.ai/code/session_01CJXWjB9ERGgV6B9mRqrdM9
Generated by Claude Code