perf(state): stop taking the state.db write lock to open a database that needs no writes
Every read-write SessionDB open issued three writes that usually change nothing, and a write statement takes the database write lock even when it matches no rows: - _ensure_db_file_generation's INSERT OR IGNORE into state_meta. The stamp is minted once per FILE, so every open after the first inserted nothing. - the NULL-`active` heal, UPDATE messages SET active = 1 WHERE active IS NULL, which matches nothing on a healthy database. - the fts_storage_version stamp, which re-wrote the same value on every open of an already-optimized database. The connection is opened with timeout=1.0, so each blocked write costs a full busy timeout while a sibling process holds the write lock, and the open path's patience loop can ultimately give up and raise. Gate all three on a read. Measured on a real 99 MB state.db (239 sessions, 6470 messages) with a sibling holding the write lock: 2117-2136 ms -> 3.6-7.1 ms. On an already-optimized database the unpatched open does not merely stall, it raises `database is locked`; patched it completes in 3.7-6.8 ms. With a sibling running 200 ms write transactions in a loop (n=20 opens): p50 894.6 -> 9.6 ms, p90 1094.7 -> 13.9 ms. A settled database now issues zero main-database write statements to open. The reads cost nothing measurable: the state_meta probe is a primary-key seek (2.0 us), the messages probe is 1.7 us on the modern NOT NULL column (unsatisfiable constraint, short-circuited) and 0.4-0.6 us at 300k rows on a legacy default-less column via the partial index that already exists for exactly this predicate. An uncontended open is unchanged. Semantics are preserved. INSERT OR IGNORE still resolves the first-opener race inside SQLite and racers still converge on the winner's token via the re-read; the application_id gate and the PASSIVE-only checkpoint are untouched; the heal is still considered on every startup, as #60108 deliberately made it, with only the write now conditional on a read proving there is something to repair. Read-first also fixes a correctness bug. Under contention the generation block was abandoned by its `except sqlite3.Error` handler, so a process ended up with no generation token at all even though the value was already on disk and a plain read would have returned it -- and that token feeds the deleted-WAL and replaced-file guards added by #101221. The heal's `except OperationalError: pass` likewise skipped the repair silently, so the unconditional form did not even deliver the unconditional repair it advertised whenever it mattered most. The probe deliberately does not use INDEXED BY: that hint raises OperationalError("no query solution") against the modern NOT NULL column, and the existing handler would swallow it, disabling the repair forever.
This commit is contained in:
committed by
Teknium
parent
dbc5d7c60b
commit
ef8d682200
+13
-2
@@ -889,8 +889,13 @@ class SessionSchemaMixin:
|
||||
# Heal NULL ``active`` rows on every startup: older reconciler builds added ``active``
|
||||
# without NOT NULL DEFAULT 1, so ``WHERE active = 1`` loaders hid whole histories. A
|
||||
# ``current_version < 12`` gate never re-ran for already-v12+ databases.
|
||||
# Read before writing: an UPDATE takes the write lock even when it matches no rows, so
|
||||
# the unconditional form blocked every open behind a sibling's write transaction. The
|
||||
# probe is short-circuited on the modern NOT NULL column and index-served on legacy
|
||||
# ones. Deliberately not INDEXED BY (raises "no query solution" on NOT NULL columns).
|
||||
with contextlib.suppress(sqlite3.OperationalError):
|
||||
cursor.execute("UPDATE messages SET active = 1 WHERE active IS NULL")
|
||||
if cursor.execute("SELECT 1 FROM messages WHERE active IS NULL LIMIT 1").fetchone() is not None:
|
||||
cursor.execute("UPDATE messages SET active = 1 WHERE active IS NULL")
|
||||
|
||||
fts5_available = self._sqlite_supports_fts5(cursor)
|
||||
stale_row = cursor.execute("SELECT 1 FROM state_meta WHERE key = ? LIMIT 1", (FTS_STALE_KEY,)).fetchone()
|
||||
@@ -1012,7 +1017,13 @@ class SessionSchemaMixin:
|
||||
and not self._has_fts_trash(cursor)
|
||||
and not self._fts_external_index_empty_with_messages(cursor)
|
||||
):
|
||||
self.set_meta("fts_storage_version", str(FTS_STORAGE_VERSION), cursor=cursor)
|
||||
# Stamp only when it would change something: on a settled DB every condition above
|
||||
# already holds, and re-writing the same value takes the write lock on every open.
|
||||
if cursor.execute(
|
||||
"SELECT 1 FROM state_meta WHERE key = 'fts_storage_version' AND value = ? LIMIT 1",
|
||||
(str(FTS_STORAGE_VERSION),),
|
||||
).fetchone() is None:
|
||||
self.set_meta("fts_storage_version", str(FTS_STORAGE_VERSION), cursor=cursor)
|
||||
|
||||
# Advance schema_version — deliberately NOT gated on the FTS opt-in (that would block
|
||||
# every future migration for a user who never optimizes). FTS5 unavailable is the
|
||||
|
||||
Reference in New Issue
Block a user