Read the write-ahead log - #1
Open
fmguerreiro wants to merge 5 commits into
Open
Conversation
A database in WAL mode keeps recent pages in a separate "-wal" file until a checkpoint folds them in, so reading the main file alone returns the database as of the last checkpoint. For a file belonging to a running application that is arbitrarily stale: a row written seconds ago can be missing, and so can a newer page 1, which leaves the header (user_version among it) out of date too. Open, and OpenFrom when the reader exposes Name(), now index the log beside the database and serve those pages from it. Frames are accepted while their salts match the header and their checksums verify, and only frames up to the last commit frame are applied, so a torn tail or an in-flight transaction is dropped rather than read. A log SQLite would ignore (bad header checksum, mismatched page size, no commit frame) is ignored here too, leaving the main file readable on its own. testdata/wal.sqlite was checkpointed after its first row: one row and user_version 7 on its own, three rows and user_version 16 with its log applied.
The pager held the index and the file it points into as two fields set together, cleared together, and with one guarded on the other to be dereferenced. Nothing enforced that pairing. The file now lives on walIndex, which drops a return value from openWAL and a branch from pager.Delete. Cleanup on a failed attachWAL moves to the call site, matching the init path directly below it, and DbFile.Close now returns the error from closing the log instead of discarding it. In the tests, copyDB owns the temporary directory it copies into and hands back a cleanup func, and assertRows replaces the open-and-compare block that five of them repeated verbatim.
Open panics on a log whose page 1 is zeroed. The magic check in OpenFrom runs against the main file's page 1, never against the log's replacement for it, so a page size of zero reaches the pager and page.Kind() indexes an empty buffer. Re-check the magic and the page size after the decode. Three frame fields went unvalidated: - a page number of zero, which wal.c rejects outright - a format version other than 3007000, which would parse a future format with today's frame rules - a commit page count above MaxInt32, which became a negative NumPage A checkpoint can also restart the log between the scan and a later page read, leaving an indexed offset pointing into another generation's frame. Re-read the frame header on each page and check that the page number and salt still match, so that errors instead of returning a wrong page. Structural, no behaviour change: build the index before the pager so the page count derives once, put the offsets and the file handle behind walIndex methods, and pass the checksum word order as a binary.ByteOrder. Each rejection has a test through readWALIndex. The new wal-grow fixture covers a log that grows the database past the end of the main file, which is the ordinary state of a database with a live writer.
Reading the page count from the log before building the pager, as the previous commit does, also made it attacker-controlled at the point of allocation: newPager hints its cache map with npages, so a 1KB crafted -wal committing at MaxInt32 pages reaches make(map[int]page, 2147483647). Open thrashes for 19s and is killed. Drop the hint. The cache only ever holds the pages actually read, and the main file's header could already reach the same call. Tests: page 1 with the wrong magic, and page 1 changing the page size, were each masked by whichever check fired first, and the log's fall-through to the main file was never exercised at all, since every fixture happens to log every page it commits. The three copies of the checksum chain collapse to one helper taking the word order, which keeps the property that a bug in walChecksum cannot cancel itself out.
A checkpoint restarts the log either by overwriting it under a new salt or, for wal_checkpoint(TRUNCATE) and journal_size_limit, by emptying it. Only the first was recognised. The second leaves every indexed offset past the end of the file, so the read came back as a bare io.EOF and read like a disk problem rather than the restart it is. A short read at an offset the scan already reached can only mean the log shrank, so map it onto the same error as a salt mismatch. Also stop the pager cache comment implying a bound it does not have: lru is appended to and never read, so nothing evicts.
fmguerreiro
marked this pull request as ready for review
July 29, 2026 00:23
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.
Problem
This package never opens the
-walfile, so a database in WAL mode reads as of its last checkpoint. Anything written since is invisible.I hit it as a missing GitHub cookie:
gh imagesaid "no session token found" for a Firefox that was logged in, because the cookie sat in a 754KBcookies.sqlite-wal. Chrome, Firefox and Epiphany all go throughOpenFrom, so one fix covers all three.What it does
Open, andOpenFromwhen the reader has aName() string, read committed pages out of the log beside the database.Which frames count follows the spec: the salt matches the header, the checksum verifies, nothing past the last commit frame. Two more rejects: a page number of zero, which wal.c also rejects, and a page count too big for an int32.
Decisions worth your attention:
OpenFromfinds the log by assertingName() string, not by taking a path. It only gets anio.ReadSeeker, so there is no path otherwise, and the assertion is what lets kooky pick this up unchanged, since it opens the real file in place. An explicitOpenFromPath(io.ReadSeeker, string)would avoid folding a live log into a deliberate snapshot copy, but nothing copies these files today, so I took the smaller change. Say the word if you want the explicit form.-walclaiming MaxInt32 pages gotOpenOOM-killed.Behaviour change
Open, andOpenFromwith an*os.File, now return a different database image, with no opt-out. That's the point of the change, but it may want a release note.Tests
testdata/wal.sqliteis a real database left mid-WAL: one row in the main file, three once the log applies. Opening it with the sqlite3 CLI checkpoints the log away and the fixture stops testing anything.TestOpenWithoutWALcatches that if it happens. Every other log is built in memory.Both pre-existing on master, left alone here. Happy to file separately:
1andPageSize()returns it as-is, soOpenreads 1-byte pages.OpenFrom.