fix(sync): fix SnapshotReader goroutine race and fsync ordering

[corylanou]

📍 Stack map: part of a stacked series — see the canonical PR map for review order, scope, and current blockers.

Summary

Fixes the issue #1164 corruption hazards in a small stacked PR:

• applyLTXFile() now calls f.Sync() before f.Truncate(), so restored page writes are durable before the database file is shortened to the LTX commit size.
• Adds TestDB_SnapshotReaderConsistentDuringConcurrentCheckpoints, which pins the atomicity between SnapshotReader's position capture and its chkMu read lock. The lock is acquired while the executor semaphore is still held (the arrangement introduced in fix(db): harden checkpoint snapshots #1292), so no checkpoint can run between reading the position and encoding the snapshot. The test reproduces the Database corrupted when restoring from backup #1164 corruption class on the first iteration if that handoff is ever broken.
• Adds sequential sync/restore integrity coverage (TestSyncRestoreIntegrity, TestSyncRestoreIntegrity_WithCheckpoints) and apply-LTX restore tests.

Note: an earlier revision of this PR moved chkMu.RLock() into the snapshot goroutine. That change was authored against main (where the lock was released when SnapshotReader returned, before the encode finished) but on this stack #1292 already holds the lock across the full encode via the snapshotReadPosition handoff — re-applying it here reopened the checkpoint window it was meant to close. The commit was dropped; the new concurrency test now guards that invariant instead.

This PR stacks on #1289 through #1292 via issue-1280-checkpoint-snapshot-hardening.

Ref #1164
Ref #1199
Ref #1281

Scope

In scope:

• Product fix in replica.go (fsync-before-truncate)
• Snapshot-vs-checkpoint consistency regression test in db_internal_test.go
• Focused internal/unit coverage for apply-LTX restore behavior and sync/restore integrity

Not in scope:

• Full integration soak coverage
• tests/integration/soak_replicate_restore_test.go, which moved to test(integration): add replicate restore soak test #1302

Test Plan

go build -o bin/litestream ./cmd/litestream
go test -race ./... -run "SnapshotReader|ApplyLTX|Snapshot|SyncRestoreIntegrity"
go test -race -count=1 ./...

Verified TestDB_SnapshotReaderConsistentDuringConcurrentCheckpoints fails immediately ("snapshot at txid N contains v=X, want Y: content inconsistent with advertised position") when the dropped lock-restructure commit is re-applied, and passes on this branch.

[github-actions]

PR Build Metrics

✅ All clear — no issues detected

Check	Status	Summary
Binary size	✅	36.98 MB (0.0 KB / 0.00%)
Dependencies	✅	No changes
Vulnerabilities	✅	None detected
Go toolchain	✅	1.25.12 (latest)
Module graph	✅	1240 edges (0)

Binary Size

	Size	Change
Base (d26cb54)	36.98 MB
PR (e583f68)	36.98 MB	0.0 KB (0.00%)

Dependency Changes

No dependency changes.

govulncheck Output

=== Symbol Results ===

No vulnerabilities found.

Your code is affected by 0 vulnerabilities.
This scan also found 0 vulnerabilities in packages you import and 1
vulnerability in modules you require, but your code doesn't appear to call these
vulnerabilities.
Use '-show verbose' for more details.

Build Info

Metric	Value
Build time	45s
Go version	go1.25.12
Commit	e583f68

History (10 previous)

Commit	Updated	Status	Summary
2647dc2	2026-07-16 15:06 UTC	✅	36.98 MB (0.0 KB / 0.00%)
3c24520	2026-07-08 20:43 UTC	✅	36.93 MB (0.0 KB / 0.00%)
7bef87c	2026-07-08 20:40 UTC	✅	36.93 MB (0.0 KB / 0.00%)
58a3583	2026-07-08 18:52 UTC	✅	36.93 MB (0.0 KB / 0.00%)
8d8419d	2026-07-08 18:50 UTC	✅	36.93 MB (0.0 KB / 0.00%)
2e89938	2026-07-02 03:42 UTC	✅	36.93 MB (0.0 KB / 0.00%)
c65d55c	2026-06-26 15:05 UTC	✅	36.92 MB (0.0 KB / 0.00%)
82d0f00	2026-06-09 19:59 UTC	✅	36.92 MB (0.0 KB / 0.00%)
f0ba521	2026-06-09 14:00 UTC	✅	36.92 MB (0.0 KB / 0.00%)
e8ac2e9	2026-06-08 21:55 UTC	✅	35.82 MB (0.0 KB / 0.00%)

---

_{🤖 Updated on each push.}

[corylanou]

Testing against the reproduction repo

We used the schema and access patterns from @fuchstim's reproduction repo to build a deterministic test (TestSync_WALRaceCondition) that mechanically demonstrates the race condition.

The reproduction repo creates a table with 5 indexes and 6 triggers, then runs 100 ops/sec (inserts, updates, soft-deletes, outbox cleanup). Each transaction touches many WAL pages across different B-tree structures — exactly the scenario that maximizes the checkpoint race window.

What the test does

1. Creates a DB using the same schema (table data with _uid, _resource_version, _deleted_at, name, data_json, is_active columns + all the indexes from the repro)
2. Inserts 200 rows to generate many WAL pages across table and index B-trees
3. Reads the WAL using both approaches:
   • OLD: Records page offsets (map[uint32]int64) — what the code did before this PR
   • NEW: Buffers page data (map[uint32][]byte) — what the code does now
4. Overwrites all WAL frames with garbage (simulating a PASSIVE checkpoint rewriting the WAL)
5. Compares results

Test output

buffered PageMap: 10 pages, commit=13, maxOffset=1709832
old offset map: 10 page offsets recorded
corrupted 416 WAL frames to simulate checkpoint

OLD offset-based approach: 10/10 pages corrupted by checkpoint simulation
NEW buffered approach: 0/10 pages corrupted

LTX file valid: 10 pages encoded and decoded successfully

The old approach produces 10/10 corrupted pages when the WAL is modified — this is what was happening in production. The new approach produces 0/10 corrupted pages because the data was already captured during the initial read.

Thanks again to @fuchstim for the detailed trace logs, reproduction repo, and all the debugging — it made pinpointing this race condition much faster.

[emschwartz]

I separately ran into this issue.

I'm not a Go or Litestream expert so I had Claude Code try to debug it. After confirming that the issue was with the data in the object store, not a problem with the production database or the restoration process, CC identified that this PR's work would have fixed the problem.

I've copied CC's analysis below, which includes a new test that could be included in this PR.

---

Analysis and test by Claude Code, from a production incident; verify independently.

SnapshotReader releases db.chkMu (deferred RUnlock fires on outer-function return) before its goroutine reads DB/WAL pages as the snapshot is drained, so a concurrent checkpoint can corrupt the snapshot. This is the race f8beb02 fixes, still present on main.

Production isolation:

• Restore to the snapshot's max TXID (1-file plan: L9 snapshot, no deltas, no merge) fails integrity_check: freelist corruption, btreeInitPage error 11, 2nd reference to page, rowid out of order across many trees. The snapshot object is malformed at rest.
• The live source DB passes quick_check, so the corruption was introduced during snapshot build, not pre-existing.
• Scheduled snapshots use this path: Store.CompactDB shortcuts SnapshotLevel to db.Snapshot() → SnapshotReader().

Deterministic regression test (no MinIO, no timing dependence). RED on main, GREEN with f8beb02:

TestSnapshotReader_ConcurrentCheckpointDoesNotCorruptSnapshot

// Reproduces the SnapshotReader checkpoint race (#1164).
//
// On main, SnapshotReader's deferred chkMu.RUnlock() fires when the outer
// function returns, before its goroutine reads DB/WAL pages as the snapshot is
// drained. A checkpoint during the drain mutates the WAL out from under the
// reader. RED on main; GREEN with this PR (lock held for the read duration).
func TestSnapshotReader_ConcurrentCheckpointDoesNotCorruptSnapshot(t *testing.T) {
	ctx := context.Background()
	dbPath := filepath.Join(t.TempDir(), "db")

	db := NewDB(dbPath)
	db.MonitorInterval = 0
	db.Replica = NewReplica(db)
	db.Replica.Client = &testReplicaClient{dir: t.TempDir()}
	db.Replica.MonitorEnabled = false
	if err := db.Open(); err != nil {
		t.Fatal(err)
	}
	defer db.Close(ctx)

	sqldb, err := sql.Open("sqlite", dbPath)
	if err != nil {
		t.Fatal(err)
	}
	defer sqldb.Close()
	if _, err := sqldb.Exec(`PRAGMA journal_mode = wal;`); err != nil {
		t.Fatal(err)
	}

	// Many pages so the snapshot stream is large (the goroutine cannot finish
	// before we drain) and the WAL is non-empty (pages resolve via the WAL page
	// map, the read most sensitive to a concurrent checkpoint).
	if _, err := sqldb.Exec(`CREATE TABLE t (id INTEGER PRIMARY KEY, data TEXT)`); err != nil {
		t.Fatal(err)
	}
	for i := 0; i < 500; i++ {
		if _, err := sqldb.Exec(`INSERT INTO t (data) VALUES (?)`, fmt.Sprintf("%0500d", i)); err != nil {
			t.Fatal(err)
		}
	}
	if err := db.Sync(ctx); err != nil {
		t.Fatal(err)
	}

	// Begin a snapshot read and consume only the LTX header, so the page-reading
	// goroutine is mid-flight (page map built; about to read pages as we drain).
	_, r, err := db.SnapshotReader(ctx)
	if err != nil {
		t.Fatalf("SnapshotReader: %v", err)
	}
	if _, err := io.ReadFull(r, make([]byte, ltx.HeaderSize)); err != nil {
		t.Fatalf("read snapshot header: %v", err)
	}

	// The checkpoint lock must be held while the snapshot is being read.
	if db.chkMu.TryLock() {
		db.chkMu.Unlock()
		t.Errorf("chkMu was free mid-snapshot-read: SnapshotReader released the checkpoint lock before its page-reading goroutine finished; a concurrent checkpoint can corrupt the snapshot")
	}

	// A checkpoint that runs during the read must be blocked (no-op) so the
	// snapshot completes cleanly. With the race, TRUNCATE empties the WAL out
	// from under the reader.
	if err := db.Checkpoint(ctx, CheckpointModeTruncate); err != nil {
		t.Logf("checkpoint returned: %v", err)
	}

	if _, drainErr := io.Copy(io.Discard, r); drainErr != nil {
		t.Errorf("snapshot stream broke because a concurrent checkpoint mutated the WAL mid-read: %v", drainErr)
	}
}

The test uses TRUNCATE, so the stale WAL read returns EOF. In production the WAL is reset and reused, so the stale offsets return valid-but-wrong bytes: the corrupt snapshot is written and uploaded with HeaderFlagNoChecksum, so nothing flags it.

[corylanou]

Why this PR was rebased onto the sync-stability series

This PR's CI was red on TestWALGrowth, failing with nonsequential page numbers in snapshot transaction. That is not caused by the changes here — it is the pre-existing snapshot-on-checkpoint correctness bug tracked in #1199 / #1281, which is still present on main. The fix for it lives in #1292 (fix(db): harden checkpoint snapshots).

So rather than leave this PR red against main, it was rebased onto the tip of the sync-stability series (issue-1280-checkpoint-snapshot-hardening), stacking it behind #1289 → #1290 → #1291 → #1292. With #1292's fix present, the failure is resolved and the dependency is now explicit.

Verified green on HEAD 4b681109a2e037ce5b2ff3b21c69c84ca9d9b192:

• Local: TestWALGrowth now passes — --- PASS: TestWALGrowth (173.42s).
• CI: Quick Integration Tests passes (16m49s), all other checks green.

Scope was also reduced to honor small, reviewable increments: this PR is now just the two product fixes (db.go SnapshotReader lock duration, replica.go fsync-before-truncate) plus focused unit tests. The integration soak test moved to its own stacked PR, #1302.

[corylanou]

Status note for reviewers — this PR has been rescoped since the earlier review.

The changes-requested on this PR dates to 2026-02-27 and was against the original approach (reading the entire WAL into memory). That approach is gone, and both of those review threads are now resolved/outdated.

The current PR is scoped to two #1164 corruption fixes only:

• SnapshotReader() takes chkMu.RLock() (and calls f.Stat()) inside the io.Pipe goroutine, so the read lock covers the full snapshot encode.
• applyLTXFile() calls f.Sync() before f.Truncate(), so restored pages are durable before the file is shortened.

It is stacked behind #1289 → #1292 (base issue-1280-checkpoint-snapshot-hardening); the previously-red TestWALGrowth passes once the #1292 snapshot fix is included, and CI on this branch is green. The integration soak test was split out to #1302 to keep this PR small.

Since the prior verdict predates the rescope, a fresh review would be appreciated. Full stack map: https://gist.github.com/corylanou/f2709d9868cad1943c63096bf978e881