fix(store): remove destination WAL/SHM sidecars before installing a fresh DB#1016
Merged
Conversation
…resh DB Three code paths install a brand-new DB file where a previous generation lived without removing the destination's -wal/-shm: the pipeline page writer (cbm_writer_open), artifact import (which cleaned the TMP file's sidecars but not the destination's), and cbm_store_dump_to_file (whose comment wrongly asserted stale sidecars 'become stale and get recreated'). SQLite validates a WAL against the sidecar's own header/checksums — not the main file — so a crashed session's leftover WAL is replayed ON TOP of the freshly installed file at the next open, splicing old-generation pages into it. New shared helper cbm_remove_db_sidecars (compat_fs, wide-path safe on Windows) called at all three install points. Reproduce-first: dump_install_ignores_stale_wal_sidecar follows the issue's recipe (hot-copy a live WAL, clean close, restore the copy, install generation 2, reopen). RED on the unguarded code — the replay clobbered the fresh generation entirely (fresh row unqueryable); GREEN with the cleanup. Non-vacuous: asserts the hot WAL is non-empty before the copy. Closes #897 Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The new stale-sidecar guard exposed a pre-existing Windows gap in the same code path: rename() fails with EEXIST when the destination exists, so cbm_store_dump_to_file could never install over a previous DB generation on Windows. New cbm_rename_replace uses MoveFileExW(MOVEFILE_REPLACE_EXISTING) with wide paths (raw MoveFileExA would re-mangle non-ASCII cache paths); POSIX keeps plain atomic rename. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
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
1 participant
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.
Closes #897.
All three install paths from the report now clear the destination's
-wal/-shmfirst via a sharedcbm_remove_db_sidecarshelper (compat_fs,_wunlink-backed on Windows): the pipeline page writer, artifact import (it cleaned the tmp file's sidecars, not the destination's), andcbm_store_dump_to_file— whose 'sidecars become stale and get recreated' comment the report correctly called out as wrong.Reproduce-first (the issue's own recipe): hot-copy a live WAL aside, clean close, restore it as the crashed-session leftover, install generation 2 via
dump_to_file, reopen. On unguarded code the replay clobbered the fresh generation entirely (the fresh row was unqueryable) — even nastier than the resurrected-stale-rows the report predicted. GREEN with the cleanup; the test asserts the hot WAL is non-empty so it can never pass vacuously.Full suite 5,990/0, lint-ci clean. Related: #790 (stale sidecars making a project unqueryable after restart) is the same family but a different scenario (same-generation crash recovery, not fresh-install) — kept separate.
Thanks @kriswill for an unusually precise report — the repro recipe went into the test almost verbatim.