[frahlg]

Two internal/state tests that failed CI on PRs that did not touch them. One shows a small real bug in backup error reporting. The other is a timing assumption in the test, not a data-safety bug.

1. Backup copy loses the deadline (real bug, small)

Run 39759536215 (master):

--- FAIL: TestLiveBackupPastItsLimitNamesTheLimitAndTable (1.17s)
    backup_failure_test.go:36: backup past its limit = live backup reached its time limit of 1s: backup history: copy history_hot: sql: statement is closed

Cause. copyVerifiedTable inserts rows through a statement prepared on a transaction that uses the backup context. When the deadline lands inside a batch, database/sql rolls the transaction back on its own goroutine and closes its statements. The next row's ExecContext then fails with sql: statement is closed, not the context's error. The reason still named the limit (the outer wrapper adds it from context.Cause), but errors.Is(err, context.DeadlineExceeded) was false. A user's cancel in the same spot came out as "statement is closed", with no "canceled".

Fix. Once the context has ended, the copy puts the context's error in front of the database error: copy history_hot: context deadline exceeded: sql: statement is closed. The final commit's error gets the same treatment and the table name. Patch changeset.

Evidence. TestBackupCopyCancelledInsideABatchKeepsTheContextError cancels after the first row, waits until database/sql has rolled back and returned the connection, then sends the second row. On master it fails every time with copy t: sql: statement is closed, the CI message. It passes on this branch.

2. Maintenance rollback test: timing, not an ordering bug

Run 40185497314 (PR #1807), race job:

--- FAIL: TestHistoryMaintenanceWaitsForCancelledRollback/backfill (0.50s)
    history_rollback_test.go:251: maintenance released the writer mutex before SQLite rollback finished

Cause. The test cancels with a 50 ms timeout and holds the first rollback the driver sees. backfillSeriesHoursSnapshot reads in a deferred transaction first and takes historyWriteMu only before it writes. Under CI load the 50 ms can run out during those reads. The rollback the test then holds belongs to a read-only transaction that never took the mutex, so the free mutex looks like an early release.

Why it is not a data-safety bug. When a write is under way, the deferred cleanup runs tx.Rollback(), then conn.Close(), then historyWriteMu.Unlock(). A transaction begun from a sql.Conn holds that connection's close lock until database/sql's own rollback has returned, so conn.Close() waits for SQLite's rollback before the unlock. That is the property the test checks, and it holds. A read cancelled before the write holds no writer slot in SQLite (WAL, deferred transaction), so not holding the mutex there is correct.

Fix (test only). The test now cancels when the driver reports that the write has started, instead of on a timer. It accepts either context error, because the archive write also has its own 1 s limit.

Evidence. On master, 60 ms of delay in the test driver's reads (a local probe, not committed) reproduces the CI message in the backfill case. The archive and completion cases instead fail with maintenance ended before rollback. With the same probe this branch passes all three: the mutex stays held until the rollback finishes.

Review

An independent review (Sonnet) checked the ordering claim against database/sql in Go 1.26.9 (CI) and 1.27.1. It found no bug: all three paths unlock after conn.Close(), and conn.Close() waits for the rollback. It also mutated the code: dropping conn.Close() from the backfill cleanup, or unlocking before rollback and close, makes the changed test fail with the original message, so the test still guards the property. The second commit applies its two notes: accept either context error, and reword the changeset.

It also found a separate, older risk, which this PR does not change. history_plain.go:337, store_ts.go:396 and history_migration.go:302 hold historyWriteMu around a transaction begun on the *sql.DB, not on a sql.Conn. If that context is cancelled, tx.Rollback() can return before SQLite has rolled back, and the mutex is released early. SQLite's own writer lock still protects the data, so the next writer waits or gets BUSY; nothing is lost. store_ts.go:528 already uses a Conn for this reason. I have not checked whether those paths run with a context that can be cancelled.

Runs

• Race detector, 6 parallel copies × -count=34 of the four rollback and backup tests (204 runs each): all pass. Master also passes this local load; both flakes need CI's load, which is why the evidence above uses deterministic reproductions.
• Whole internal/state package under the race detector: passes (270 s).
• go vet ./... passes.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-25T11:16:51.183360Z	3f2d931	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.