Skip to content

Make DB writes rollback-safe and self-heal poisoned connections#168

Merged
pufit merged 1 commit into
mainfrom
pufit/db-txn-rollback-poison-guard
Jul 6, 2026
Merged

Make DB writes rollback-safe and self-heal poisoned connections#168
pufit merged 1 commit into
mainfrom
pufit/db-txn-rollback-poison-guard

Conversation

@pufit

@pufit pufit commented Jul 6, 2026

Copy link
Copy Markdown
Member

Incident

On 2026-07-06 04:04 UTC a production daemon's write path wedged for 10 hours: every nerve.db write failed instantly with database is locked (43K+ errors) while reads kept working, no file lock was actually held, and busy_timeout=10000 never engaged. Only a daemon restart recovered it.

Root cause

The shared aiosqlite connection was left inside an abandoned open transaction (a write task errored or was cancelled between the implicit BEGIN and commit()). That pinned a WAL read snapshot. The moment a second connection (CLI / helper script / backup) committed, the snapshot went stale and every write-upgrade on the shared connection returned SQLITE_BUSY_SNAPSHOT — reported as database is locked, instant (the busy handler is deliberately not invoked for snapshot conflicts), and permanent until ROLLBACK. Nothing in the codebase called rollback(), so the state could never self-clear. Reads silently served the frozen snapshot the whole time.

Pre-existing related hazards (same missing-rollback root): _atomic() half-commits on error (partial writes flushed by the next commit), and ~50 single-statement writers committed outside _write_lock, able to prematurely flush someone else's in-flight transaction.

Fix

  • _atomic() is now a real transaction: commit on success, rollback on any exception including asyncio.CancelledError; commit/rollback are cancellation-shielded so they always run to completion.
  • _write(sql, params) — new helper for single-statement writes with the same guarantees, serialized under _write_lock. Returns a WriteResult(lastrowid, rowcount).
  • _heal_leaked_txn() — belt-and-suspenders guard on every write entry: if the connection is unexpectedly in_transaction (any future code path that leaks), roll back loudly instead of wedging forever. This alone converts a repeat of the incident into a one-line error log.
  • All 52 unlocked execute()+commit() sites across the 16 store mixins migrated to the helpers; checkpoint()/vacuum() no longer issue bare unlocked commits; the startup FTS reseed runs inside _atomic().
  • update_session_metadata no longer nests a locked call inside its transaction (extracted a shared SQL builder).

Tests

tests/test_db_atomicity.py (11 tests): full incident reproduction (leaked txn + external commit → write self-heals; raw path shown to fail without the guard), rollback on body error / failed statement / mid-transaction cancellation, no-premature-commit serialization regression, 50-writer concurrency smoke, and WriteResult plumbing. One existing test adjusted — its fixture relied on the old half-commit behavior (raw uncommitted execute flushed by the next method's commit).

Full suite: 1400 passed, 2 skipped.

Generated by Nerve

A write task abandoned mid-transaction leaves the shared aiosqlite
connection pinned to a stale WAL snapshot; after any external commit,
every write fails instantly with SQLITE_BUSY_SNAPSHOT ('database is
locked', busy_timeout not consulted) until ROLLBACK - which nothing
ever called. Wedged a production daemon for 10h on 2026-07-06.

- _atomic(): commit on success, rollback on any error incl. cancellation
- _write(): single-statement writes serialized under the write lock
- _heal_leaked_txn(): recover from leaked transactions on any write
- migrate all 52 unlocked execute+commit sites; fix bare commits in
  checkpoint()/vacuum()
- regression tests incl. the poisoned-connection reproduction
@pufit pufit merged commit f1b0cff into main Jul 6, 2026
2 checks passed
@pufit pufit deleted the pufit/db-txn-rollback-poison-guard branch July 6, 2026 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant