Repository navigation
Use BEGIN IMMEDIATE for SQLite write transactions - #10
Merged
DavidMStraub merged 3 commits intoSep 29, 2026
Merged
Conversation
test_write_waits_for_concurrent_writer[delete|delete_all]: another connection holds the write lock for 300 ms; delete()/delete_all() must wait for it (the connection has a 5 s busy timeout) instead of raising "database is locked" immediately. Fails on v1.3.1 because conn() uses a deferred BEGIN and preparing the first statement on the documents_fts FTS5 table performs a read, after which SQLite refuses to invoke the busy handler for the write-lock upgrade. test_embeddings_computed_outside_write_transaction: the embedding function must not run while add() holds the write lock. Fails on v1.3.1 because _add() calls it inside the transaction. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CollectionSQLite.conn() opened every transaction with a deferred BEGIN. delete() and delete_all() start with a statement on the documents_fts FTS5 virtual table; preparing that statement makes FTS5 run `SELECT k, v FROM documents_fts_config` (loading its config), which under a deferred BEGIN opens a read transaction before the write lock is requested. When another process holds the write lock at that moment, SQLite returns SQLITE_BUSY immediately without invoking the busy handler (its deadlock-avoidance rule for read->write lock upgrades), so the connection's busy timeout never applies and the caller gets `sqlite3.OperationalError: database is locked` even though the other writer only held the lock for milliseconds. add() was unaffected only by accident: its first statement hits the plain `documents` table, so the write lock is requested before any read and the busy handler is honored. Fix: conn() takes a `write` flag and issues BEGIN IMMEDIATE for write transactions, acquiring the write lock up front (honoring the busy timeout) before any implicit read can happen. create_tables() stays deferred: it is read-only once the schema exists and must keep working against read-only database files. The PostgreSQL backend accepts the flag and ignores it. Observed in gramps-web-api, where a synchronous search-index delete in the request thread races the Celery reindex task from the previous request and returns HTTP 500. A two-process stress run against the same index file went from ~30-70% failed deletes to zero. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CollectionSQLite._add() called the embedding function inside the write transaction, so the write lock was held for the whole embedding computation - model inference, or a remote HTTP call in some callers. Every other writer to the same database file waits for that long, and fails once the connection's busy timeout (5 s by default) expires. Compute the embeddings first and only then open the transaction; the lock is now held only for the actual SQL work. The PostgreSQL backend is unchanged: psycopg2 starts the transaction lazily at the first statement, so no lock is held there during the embedding call. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
|
Thanks! |
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
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.
Fixes #9.
CollectionSQLite.delete()anddelete_all()raisesqlite3.OperationalError: database is lockedimmediately whenever another connection or process holds the write lock, even briefly — the connection's 5 s busy timeout never applies.conn()opens write transactions with a deferredBEGIN; preparing the first statement on thedocuments_ftsFTS5 table makes FTS5 read its_configshadow table, which opens a read transaction before the write lock is requested, and SQLite then declines to invoke the busy handler for the read→write upgrade (its deadlock-avoidance rule).add()only escapes because its first statement is a plain-tableINSERT. Full trace, scenario table and repro in #9.Changes
conn()takes awriteflag; write transactions useBEGIN IMMEDIATE(_add,delete,delete_all). The write lock is acquired up front, so the busy handler is honored and any implicit read happens with the lock already held.create_tables()stays deferred: once the schema exists it is read-only (CREATE ... IF NOT EXISTSjust verifies the schema), and it keeps working against read-only database files.count,query,get) stay deferred.CollectionPostgreSQL.conn()accepts the flag and ignores it._add()computes embeddings before opening the transaction. Previously the embedding function ran while holding the write lock, so slow embeddings (model inference, remote APIs) blocked every other writer for that long. Postgres backend unchanged — psycopg2 begins the transaction lazily, so nothing was held there.Tests
Three regression tests in
tests/sifts/test_sqlite.py, committed first so the failure is visible in the history:test_write_waits_for_concurrent_writer[delete|delete_all]— another connection holds the write lock for 300 ms via athreading.Timer; the write must wait and succeed.test_embeddings_computed_outside_write_transaction— the embedding function probes withBEGIN IMMEDIATE(timeout=0) and must not find the lock held.On
v1.3.1: 3 failed, 87 passed. With this branch: 90 passed. Verified with a two-processadd()-vs-delete()stress run: 1010/3585 deletes failed before, 0 after (same on Linux/SQLite 3.46.1 and macOS/SQLite 3.53.4, rollback and WAL journal modes).🤖 Generated with Claude Code