fix: bump the file change counter and version-valid-for on commit (#710) - #722
Open
dpsiderius wants to merge 1 commit into
Open
dpsiderius wants to merge 1 commit into
dpsiderius wants to merge 1 commit into
Conversation
Pager::flush preserved header offset 24 (change counter) and offset 92 (version-valid-for) across every commit instead of incrementing them, so a long-lived sqlite3 reader that had already cached page 1 kept serving stale rows after our write with no error and integrity_check ok. flush now bumps both fields exactly once per committed transaction — guarded against double-bumping when a flush is retried after lock contention — wrapping past u32::MAX rather than erroring, matching stock sqlite3. Proven with a persistent sqlite3 session (tests/corpus/oracle.rs's new OracleSession, reusable for #706) that reads before and after our write, plus byte-exact comparison of both header fields against the oracle for matching write sequences and for a freshly created database. spend: matched estimate (small)
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.
What
Header offset 24 (the file change counter) is now incremented, and offset
92 (version-valid-for) mirrored to the same value, on every commit that
writes page 1. Before this, both fields were preserved rather than
zeroed/bumped, so a long-lived stock sqlite3 reader had no signal that
our write happened and kept serving its cached page image — no error,
no corruption, just stale answers.
Fixed a real bug found while implementing this: a retried
flush()afterlock contention would double-bump the counter, because a dirty page 1
survives the failed attempt. Guarded with a
change_counter_bumpedflagreset at every dirty-clear site.
Why this needed a new test harness
Proving it requires a long-lived oracle process spanning our write — every
existing corpus test invokes
sqlite3fresh per assertion, which can'tobserve a cache going stale. Added a reusable persistent-session helper
(
OracleSession) totests/corpus/oracle.rs.Before/after
engine writes a row, the same session reads again: before, it returns
the stale row count; after, it sees the new row.
write sequence, on both a freshly-created database and one we mutate.
Test plan
tests/corpus/change_counter_test.rs— the scenario above, byte-exactheader match vs. oracle, fresh-database match vs. oracle.
make test,make lint,make test-corpus,make check-mvl-limit,make check-mod-files,make assuranceall pass.Refs: #193, #705
Closes #710
spend: matched estimate