SQLite configuration is not ready for a concurrent worker #87
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Severity: MEDIUM (blocking for #2)
The problem
backend/app/db/session.py:13-27sets onlyjournal_mode=WALandforeign_keys=ON. Probedagainst the installed stack: pool is
QueuePool, busy timeout is pysqlite's implicit 5000 ms, andsynchronous=FULL.Three gaps once
app/workers/becomes a real process writing concurrently with the API:synchronous=FULLfsyncs the WAL on every commit. Every decision writes a decision row, aprojection update, and an audit row; each ingest commits individually.
NORMALis the standardWAL pairing and loses no durability except on power failure.
BEGIN. A read-then-write transaction — exactly the shape of_get_photo_or_404→_conflict_check→ write — that races a worker commit fails withSQLITE_BUSY_SNAPSHOTimmediately. The busy handler cannot rescue a lock upgrade.than 5 s — say an AI handler that writes early then calls a slow API inside the transaction —
turns reviewer saves into 500s.
Fix
PRAGMA synchronous=NORMALand an explicitPRAGMA busy_timeoutto the connect listener.BEGIN IMMEDIATEfor write transactions (viaevent.listens_for(engine, "begin")), atminimum in the worker's claim loop and the photo write paths.
Done when
database is lockederrorsReferences
backend/app/db/session.py:13-27Blocks: #2 (worker runtime). Related: #48.
Done in
4db9ad9.One correction to the diagnosis. The issue says a read-then-write transaction racing a worker commit fails immediately with
SQLITE_BUSY_SNAPSHOT. Probed against the installed stack, that is not what happens today — because the read is not in a transaction at all. pysqlite emitsBEGINbeforeINSERT/UPDATE/DELETEand never before aSELECT, so after aSELECTthrough aSessionthe driver reportsin_transaction = False. The actual defect was worse in a different way: two reads in one request could see two different databases, and the read behind a check-then-write was not covered by the write that followed. (review_versionsurvived that only because #77 had already moved its check into theUPDATEitself.)Fixing that is what makes the issue's diagnosis true.
isolation_level = Noneplus our ownBEGINmakes reads transactional — and then a deferred read-then-write does fail instantly with a stale snapshot, which no busy timeout can retry. So the two halves had to land together; doing only the pragmas would have been a no-op, and doing onlyisolation_levelwould have introduced the failure the issue describes.TestDeferredWritesAreTheHazardproduces that failure directly, so the reason is an executable fact rather than a comment.What landed
configure_sqlite()inapp/db/session.pysetsjournal_mode=WAL,foreign_keys=ON,synchronous=NORMAL, an explicitbusy_timeout, and installs thebeginlistener. Shared by the app engine, the test harness and any tooling, so tests cannot pass against transaction behaviour production does not have.BEGIN IMMEDIATE. Intent is decided by HTTP method inget_db, so a route cannot forget it; reads stay deferred, which in WAL neither waits for the write lock nor holds it. Session renewal is the only write in the codebase on aGETand asks explicitly viabegin_write(). Workers and CLI usewrite_session().DeferredWriteErrorat the first statement instead of becoming a race that only appears under load.CIRCA_SQLITE_STRICT_WRITE_INTENToverrides.CIRCA_SQLITE_SYNCHRONOUS(aLiteral, so a typo is a startup error rather than a string interpolated into a PRAGMA),CIRCA_SQLITE_BUSY_TIMEOUT_MS(10 s).Done when
database is lockederrors — three threads × 40 read-modify-writes, zero errors, and all 120 increments land, so nothing was lost to a race or to a retry papering over oneapp/db/session.pymodule docstring and the README: never hold a write transaction across a network call; do the slow work first, then open a short transaction to record the resultMeasured, not assumed: a writer blocked by a 1.0 s holder waits 1.06 s and then succeeds, where the deferred form fails instantly.
tests/test_sqlite_concurrency.py, 24 tests. Full suite 515 passed.Two knock-on effects worth recording for #2.
POST /api/ingestthat spans the sandboxed image parse. It is bounded and the busy timeout covers it, but it is the reason the handler rule above is a rule and not a suggestion — and #86 (ingest blocks the event loop) is now also a lock-holding issue, not only a latency one.TestGenuineRaceintest_review_version.pycould no longer stage two overlapping open write transactions, because there is no longer any such thing. It now stages the shape that does still occur — an object outliving the transaction that loaded it, which is every ORM object handed to a route — and still proves the mapper-level guard from #77.