Skip to content

fix: leave SQLite snapshots in WAL mode so cold-read stops timing a conversion (#96) - #131

Merged
Xof merged 1 commit into
fix/95-criterion-cell-discoveryfrom
fix/96-sqlite-cold-read-journal-mode
Jul 31, 2026
Merged

fix: leave SQLite snapshots in WAL mode so cold-read stops timing a conversion (#96)#131
Xof merged 1 commit into
fix/95-criterion-cell-discoveryfrom
fix/96-sqlite-cold-read-journal-mode

Conversation

@Xof

@Xof Xof commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Closes #96 (BENCH-3, BUG/performance).

Stacked on #130#129main.

The defect

populate_snapshot ends with flush_for_snapshot, whose SQLite impl flips
journal_mode to DELETE so the WAL drains into the main .db and a copy of
that single file is a re-openable database. It then left the file in DELETE.
The comment said that was fine:

Each cell-runner re-enters WAL mode via open_file's existing PRAGMA on open,
so no behavioral change downstream of this call.

True only when the open is in setup. The cold-read row opens the engine
inside the timed routine — by design:

/// Cold-read cell-runner — row 4 only. Engine open is INSIDE the timed
/// routine: cold means "fresh open, no values touched, first read is
/// the timed call" (master spec §5.2).

and SqliteEngine::open_file unconditionally runs PRAGMA journal_mode = WAL.
So every sqlite-strict / sqlite-unsafe cold-read iteration paid for a
DELETE→WAL conversion — exclusive lock, header rewrite, fsync, an
F_FULLFSYNC on macOS under fullfsync=ON — that neither Chisel nor redb
performs, and no real deployment performs on every open. Pure harness artifact
sitting inside the timed region of a cross-engine comparison.

The fix

flush_for_snapshot restores WAL once the DELETE flip has done its job, so the
snapshot rests in the mode every open wants. Nothing is written after the flip,
so the WAL stays empty, the clean close removes the siblings, and the .db
stays self-contained under file-copy — the property the DELETE flip exists to
guarantee is preserved.

Measured

2000×512B snapshot, 30 iterations of copy + open + one read (the cold-read
cell's timed region), this machine:

snapshot mode mean timed open+read
DELETE (before) 1.088 ms
WAL (after) 0.636 ms

~450 µs per iteration was mode conversion. The single read the row claims to
measure is microseconds, so relative to the quantity of interest the
distortion is far larger than that ratio suggests.

A trap worth flagging

The restore uses execute_batch plus a separate read-back, not
query_row("PRAGMA journal_mode = WAL").

That call returns "wal" while leaving the file in DELETE — it stops at the
first row instead of stepping the statement to completion, and entering WAL
needs the exclusive lock held to the end of the statement to rewrite the
header. It also does this inconsistently (it happened to work in one of my
scratch measurements and not in the unit test), which is exactly why the check
now verifies the mode the file actually reports rather than the value the
pragma returned. The DELETE direction tolerates the query_row shape, which is
what makes the asymmetry easy to miss.

Test

flush_for_snapshot_leaves_a_self_contained_wal_mode_file asserts both
properties at once:

  1. no surviving -wal / -shm siblings — why the DELETE flip exists;
  2. a WAL-mode header, read on a bare Connection so open_file's own
    pragma cannot be what makes it pass;

then file-copies the snapshot and reads a value back through open_file.

Property 2 is the one that regressed. Counterfactual — dropping the restore:

assertion `left == right` failed: snapshot left in "delete";
a timed cold-read open would pay for the conversion

Verification

cargo test all green, cargo clippy --all-targets -- -D warnings clean,
cargo fmt --check clean.

…onversion

`populate_snapshot` ends with `flush_for_snapshot`, whose SQLite impl
flips `journal_mode` to DELETE to drain the WAL into the main .db so a
file-copy of that one file is a re-openable database. It then left the
file in DELETE. The old comment said that was harmless — "Each
cell-runner re-enters WAL mode via open_file's existing PRAGMA on open,
so no behavioral change downstream of this call."

That holds only when the open happens in setup. The cold-read row opens
the engine INSIDE the timed routine, by design ("cold means fresh open,
no values touched, first read is the timed call"), and
`SqliteEngine::open_file` unconditionally runs `PRAGMA journal_mode =
WAL`. So every sqlite-strict / sqlite-unsafe cold-read iteration paid
for a DELETE->WAL conversion — exclusive lock, header rewrite, fsync,
an F_FULLFSYNC on macOS under `fullfsync=ON` — that neither Chisel nor
redb performs and that no real deployment would perform on every open.
The row's reported latency was a harness artifact, not a property of
SQLite.

`flush_for_snapshot` now restores WAL after the DELETE flip has done its
work, so the snapshot rests in the mode every open wants. Nothing is
written after the flip, so the WAL stays empty, the clean close removes
the siblings, and the .db stays self-contained under file-copy.

Measured on this machine (2000x512B snapshot, 30 iterations of
copy + open + one read, the cold-read cell's timed region):

  snapshot in DELETE (before): 1.088 ms mean
  snapshot in WAL    (after):  0.636 ms mean

The restore uses `execute_batch` plus a separate read-back, not
`query_row("PRAGMA journal_mode = WAL")`. That call reports "wal" while
leaving the file in DELETE — it stops at the first row instead of
stepping the statement to completion, and entering WAL needs the
exclusive lock held to the end of the statement to rewrite the header.
It also does this inconsistently, which is precisely why the check now
verifies the mode the file actually reports rather than the value the
pragma returned.

The new test asserts both properties the snapshot must have at once:
no surviving -wal/-shm siblings (why the DELETE flip exists), and a
WAL-mode header read on a bare connection so `open_file`'s own pragma
cannot be what makes it pass.

Closes #96.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant