fix(db): guard SQLite store init against concurrent opens - #686
Merged
Merged
Conversation
brettdavies
force-pushed
the
fix/sqlite-busy-timeout
branch
from
May 31, 2026 06:53
330fa74 to
158238a
Compare
…ing SQLITE_BUSY Running multiple `qmd query` invocations against the same index in parallel (e.g. an agent fanning out searches) caused N-1 of N processes to fail immediately with `SQLiteError: database is locked` from `initializeDatabase`. The first DDL statement (`DROP TRIGGER IF EXISTS documents_ai`) hits the write lock; with `busy_timeout = 0` (the default for both `bun:sqlite` and `better-sqlite3`), the loser throws on contact instead of waiting. WAL improves read-while-write concurrency but does not serialise concurrent writers. Only the timeout does that. Setting `PRAGMA busy_timeout = 5000` in `openDatabase` makes any connection wait up to 5s for the write lock before failing. Initialization runs in <100ms, so the worst-case wait for typical agent fan-out (5-10 processes) is ~1s and every process eventually succeeds. Three tests in test/db.test.ts cover the PRAGMA round-trip on a single and multiple connections, plus a behavioural check that SQLite honours the configured timeout under real lock contention.
brettdavies
force-pushed
the
fix/sqlite-busy-timeout
branch
from
June 4, 2026 05:00
158238a to
553f607
Compare
…Y_TIMEOUT The initial 5s ceiling was sized for init-time DDL contention (sub-100ms work). On multi-GB indexes, a single `embed` batch commit can outlast 5s, so an `update` or `query` that races a long-running `embed` still hits `SQLITE_BUSY` on the first write. Raise the default to 120000 ms, which outlasts the worst-case batch commit observed on multi-GB / multi-tens-of-thousands-of-doc indexes. Add `QMD_SQLITE_BUSY_TIMEOUT` (milliseconds) as an operator escape hatch. Unset, empty, or unparseable values fall back to the default; `0` restores upstream fail-fast behaviour for environments that prefer surfacing contention as an error. Tests expand from 3 to 6 cases: default value, multi-connection default, env-override honored, env=0 fail-fast, garbage value falls back to default, and the existing real-lock-contention timing check (unchanged). Verified under both `bun:sqlite` and `better-sqlite3` runtimes; `tsc` clean.
Concurrent processes opening the same index could crash during store initialization with `trigger documents_ai already exists` or `database is locked`, even with busy_timeout set. The FTS sync triggers were dropped and recreated on every open as separate autocommit statements, so two connections interleaved between the DROP and the CREATE (A drops, B drops, A creates, B creates -> "already exists"); busy_timeout serialises individual statements but not the DROP/CREATE pair. Separately, `PRAGMA journal_mode = WAL` needs a brief exclusive lock to migrate a cold database and does not invoke the busy handler, so concurrent first opens threw SQLITE_BUSY. Gate the trigger rebuild behind PRAGMA user_version inside one IMMEDIATE transaction with a double-checked read, so the DROP/CREATE pair is atomic across connections and runs once per schema version. Move WAL setup into openDatabase with a bounded retry within the busy-timeout budget, alongside busy_timeout. Add a multi-process regression test (cold and existing database) that fails before and passes after.
brettdavies
force-pushed
the
fix/sqlite-busy-timeout
branch
from
June 23, 2026 05:39
77eb395 to
5496df3
Compare
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 three crashes that hit when multiple
qmdprocesses open the same index at once: the failure mode in #710 (anupdateorqueryracing a longembed, an agent fanning out searches, or a first-open migration racing a routine command).Root cause
Three independent faults on the shared
createStore→initializeDatabasepath, all triggered by concurrent opens:SQLITE_BUSY: database is lockedon any contended write.bun:sqliteandbetter-sqlite3both defaultbusy_timeoutto 0, so a writer that loses the lock throws on contact instead of waiting. WAL lets readers and one writer coexist but does not serialise writers.trigger documents_ai already exists. The FTS sync triggers were dropped and recreated on every open as separate autocommit statements, so two connections interleave between theDROPand theCREATE(A drops, B drops, A creates, B creates, throw).busy_timeoutserialises individual statements but not theDROP/CREATEpair.database is lockedwhile migrating a cold database to WAL.PRAGMA journal_mode = WALneeds a brief exclusive lock and does not invoke the busy handler, so concurrent first-ever opens throw immediately regardless ofbusy_timeout.Fix
src/db.ts:openDatabasesetsPRAGMA busy_timeout(default 120000, overrideQMD_SQLITE_BUSY_TIMEOUT;0restores fail-fast) and enables WAL with a bounded retry within the same budget, so the cold-database journal migration survives concurrent opens. Connection-level pragmas now live in one place.src/store.ts: the FTS trigger rebuild is gated behindPRAGMA user_versionand applied inside oneIMMEDIATEtransaction with a double-checked read. TheDROP/CREATEpair is atomic across connections and runs once per schema version instead of on every open. BumpSTORE_SCHEMA_VERSIONto reissue changed trigger bodies to existing databases.Tests
test/db.test.ts: busy_timeout default, per-connection application, env override,=0fail-fast, garbage-falls-back-to-default, and a real-lock-contention timing check.test/store-concurrency.test.ts(new): spawns N processes that open the same database at once (cold and existing), asserting noalready exists/database is lockedthrow and that the triggers, FTS table, anduser_versionsurvive. Fails against the pre-fix code, passes after.tsc -p tsconfig.build.json --noEmitclean. Node and Bun suites pass; the LLM-pipeline integration tests fail in this environment on VRAM pressure (same baseline noted in feat(serve): qmd serve shared model server + RemoteQMD client (supersedes #511) #663) and reproduce identically against baremain.Closes #710.