fix(wal): stop snapshot WAL reads at the advertised end offset - #1491
Open
yasyf wants to merge 1 commit into
Open
fix(wal): stop snapshot WAL reads at the advertised end offset#1491yasyf wants to merge 1 commit into
yasyf wants to merge 1 commit into
Conversation
The snapshot WAL scan bounded pageMap with a byte budget that only breaks on commit frames, so a transaction straddling the advertised WAL end offset was read through to its commit frame past the bound. snapshotReader then rejected the result with 'snapshot wal read exceeded bound' and level-9 snapshot compaction failed permanently. Add a hard end-offset bound to pageMap that stops before reading any frame past the bound, discarding a partial transaction instead of completing it, and use it for snapshot reads. The sync path keeps the existing soft byte budget so a transaction larger than the budget is still read through to its commit and sync always makes progress. Fixes benbjohnson#1490
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.
Problem
Level-9 snapshot compaction can enter a permanent failure state:
Once triggered, every subsequent compaction attempt for that database fails identically, so the snapshot chain never advances and cold restores must replay an ever-growing LTX chain. Observed on v0.5.16 across many databases in a multi-database deployment; the bug is unchanged on
main. Full analysis in #1490.Root cause
snapshotReaderbounds the WAL scan withpageMap(ctx, maxBytes)wheremaxBytes = pos.walEndOffset - WALHeaderSize, butpageMap's bound check only fires on commit frames. A transaction that straddles the advertised WAL end is therefore read through to its commit frame past the bound, and the pages land in the returned map.snapshotReaderthen hard-fails on its absolute checkmaxOffset > pos.walEndOffset.The overshoot in the error above is exactly the tail of the straddling transaction (
24752 − 16512 = 2 × 4120), and the regression test below reproduces the identical error on unpatchedmain.While investigating we confirmed the advertised end offset can land mid-transaction:
snapshotWALEndOffset'slastSyncedWALOffsetfast path returns the stored offset without the WAL-salt validation its own LTX fallback performs, so after a WAL restart the stored offset (commit-aligned in the old WAL) can point inside a transaction of the new WAL. The sync paths themselves always record commit-aligned offsets within a generation (finalOffset = info.offset + szwithszderived frompageMap's committed-frame end), so the mid-transaction case is a bound from a previous WAL generation, and discarding the partial transaction — rather than widening the read past the bound — is the correct response. It preserves the #1281 invariant that a snapshot includes only WAL content synced through the advertised position.Fix
pageMapgains a second bound,endOffset, that is hard: the scan stops before reading any frame that would end past it, so a transaction whose commit frame lies past the bound is discarded entirely (pages are only transferred to the map at commit frames, so no restructuring is needed).snapshotReadernow passespos.walEndOffsetas the hard bound (and no byte budget). The existingmaxOffset > pos.walEndOffsetcheck is kept as an invariant guard.maxSyncWALBytes) unchanged: a transaction larger than the budget must still be read through to its commit so sync always makes progress and L0 files end on commit boundaries.In scope: the snapshot read bound in
pageMap/snapshotReader, plus regression tests.Not in scope: salt-validating the
lastSyncedWALOffsetfast path insnapshotWALEndOffset(a possible follow-up hardening; with this fix a stale bound now degrades to a slightly older, still-consistent snapshot instead of a permanent failure), and any change to sync batching semantics.Tests
TestWALReaderPageMapEndOffsetStopsBeforeStraddlingTransaction— unit test on the existingtestdata/wal-reader/ok/walfixture: a bound inside the first transaction discards it (MidTransaction), a bound exactly on a commit-frame end includes it (AtCommitBoundary).TestDB_SnapshotReaderWALEndOffsetMidTransaction— end-to-end repro: a WAL ending in a multi-frame transaction with the advertised end offset rewound one frame into it. On unpatchedmainthis fails with the exact production error (snapshot wal read exceeded bound: max offset 41232 > end offset 37112); with the fix the snapshot succeeds, decodes, and its header WAL range stays within the advertised bound.Full suite passes (destination integration tests that need cloud credentials skip as usual).
Fixes #1490