Repository navigation
fix(core): opening the ledger while another program holds it waits instead of failing busy - #54
Merged
Merged
Conversation
SQLite answers SQLITE_BUSY at once, without consulting the busy timeout, when a connection that already holds a read lock asks for the write lock while another connection holds it. Switching an old ledger to WAL does exactly that, so two programs opening one in the same instant can fail an open that a short wait would have let through (the Windows upgrade-matrix round that fails with "database is locked"). A migration refused that way is also swallowed today and reads as already applied. Red state: busy.rs is a stub that never retries and open_pool is the old connect-then-init sequence. The unit tests and the six ledger tests fail on it.
…ver read a busy migration as applied SQLite answers SQLITE_BUSY at once, without the busy timeout, when a connection that already holds a read lock asks for the write lock while another holds it. Switching a ledger from the rollback journal to WAL does that, so two programs opening one ledger in the same instant could hear "database is locked" from an open a short wait would have let through. That is the Windows CI failure in the upgrade matrix's concurrent rounds. Every opener now retries a busy open for up to two seconds: the shared pool open (open_pool, used by the daemon and the app), init_schema, and the rusqlite open_db. A migration refused as busy used to be swallowed and left undone; it is now an error, and the next open runs it again. The silent-fallback allowances for those two swallows are gone. Callers get their own tests: the app's connect and the real daemon binary, each started while a writer holds the ledger.
…a writer can refuse sqlx leaves the journal mode alone unless asked (its source says so: changing into WAL needs an exclusive lock that the busy timeout cannot wait on), so a plain connect never meets a writer. The switch is the DDL's own pragma, and init_schema retries that. A probe shows it: with a writer inside a rollback-journal ledger the connect takes 2 ms and succeeds, and the DDL is the statement that is refused at once.
…ation does, on both openers The backfill UPDATE takes the write lock on every open, even on a ledger already in WAL, so a writer that holds the ledger past the busy timeout made an open fail where it used to go on. Two tests fail on this commit for that (pool and rusqlite). The other new ones pin what is already right: a busy column migration is an error on the rusqlite opener as well as the pool opener, and the retry pauses between tries (it was only checked to try at least three times).
…en that fails The one row rewrite among the migrations takes the write lock on every open, so on a ledger a writer held past the busy timeout the open now returned the busy error where it used to go on, and a hook lost its row. It moves to BACKFILLS, which both openers run after the migrations and read as before: not an error, and done by the next open. Schema migrations stay strict. open_pool closes its pool when the schema step fails; the MIGRATIONS doc says what the openers now do with a busy statement.
Owner
Author
|
Evidence, for the record. It is not a proof.
|
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
1 participant
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.
What
A ledger still in rollback-journal mode (a first run, or one last written by an old build) is switched to WAL by whichever program opens it first. While another connection holds the write lock, SQLite refuses
PRAGMA journal_mode=WALwith SQLITE_BUSY at once, without calling the busy handler, sobusy_timeoutdoes not help. That pragma is the first statement of the shared DDL, run byinit_schema(daemon, stats) andopen_db(hooks, MCP server, CLI, app). Two programs opening the same old ledger together could fail with "database is locked".Seen on
Test (windows-latest)for main dfe4947 (run 38235651076) and again for #55 (run 38254452918):concurrent_upgrade::v1_1_4andv1_5_0, "Daemon failed beside the others: error returned from database: (code: 5) database is locked" (crates/lumen-daemon/tests/upgrade_matrix/main.rs:302). It did not fail in 150 macOS loops before this change, so only the Windows job on this PR can show the improvement.A second defect: a migration statement that failed BUSY was swallowed (
let _ =) and counted as applied, so a column or an index could stay missing for good.Change
lumen_core::busy:retry_busy(async) andretry_busy_blocking. Budget 2 s in all, pauses 5/10/20/25 ms. Matches SQLITE_BUSY and its extended codes (261, 517, 773). An attempt that itself used up the budget is not repeated. When the budget runs out the busy error is returned.init_schemaandopen_dbrun the DDL through it. A schema migration that fails busy is now an error, so the next open runs it again. Other migration errors are still read as applied (duplicate column, DROP COLUMN idempotency), by design.UPDATE turns SET is_subagent = 1 ..., moves to a newBACKFILLSlist that both openers run after the migrations. On a WAL ledger it is the only statement that takes the write lock, and every open runs it, so a writer that holds the ledger past the busy timeout (5 s) has always refused it, and the open has always gone on without it. That stays: a busy backfill is skipped and the next open redoes it. Making it an error would fail opens that succeed today.schema::open_pool(url)is the one connect-and-init, used by the daemon and by lumen-stats (connect). It closes the pool when the init fails. sqlx-sqlite 0.9 does not switch the journal mode on connect, so only the DDL takes a lock a writer can refuse.lumen-core/Cargo.toml: tokio withtimeonly, for the async sleep. It is already in the resolved tree: the packages and features are the same before and after for lumen-core, -cli, -daemon, -mcp and -stats (one new edge, lumen-core to tokio), and Cargo.lock is unchanged. Nothing new on the hook path.silent_fallbacks.rs: twoallowentries removed (the migration loops no longer have that shape), two added for the backfill loops, each with its reason: a row rewrite, idempotent, redone by the next open.Tests (the test commit comes before the fix)
busy.rs: unit tests for the retry, blocking and async: runs again until the call works, does not repeat an error that is not busy, gives the busy error back when the budget is spent (and tries at most 30 times in 120 ms, so a retry with no pause fails), does not repeat a call that used the budget by itself.lumen-core/tests/busy_ledger.rs, 10 tests, with a rusqliteBEGIN IMMEDIATEwriter holding the ledger. On a rollback-journal ledger,open_dbandopen_pooleach wait and succeed once the writer lets go, and return the busy error when it never does. Through the pool opener: a column the ledger was busy to add is added, not skipped; a ledger that stays busy is an error, not an applied migration; a busy backfill is redone within the open. Both openers: a backfill that stays busy does not fail the open, and the next open does it. The rusqlite opener waits on its own 5 s busy timeout before the retry, so a missing retry there cannot be seen by a test.lumen-daemon/tests/busy_start.rs: the real daemon binary, started while a writer holds the ledger, serves once the writer lets go.lumen-stats/tests/busy_connect.rs:connectwaits for a writer. Both go red when a caller runs the DDL alone with no retry (checked before the backfill change).concurrent_upgrade) is unchanged and as strict as before.open_db/init_schemaswallow a busy schema migrationinit_schema: backfill not retriedinit_schema: migrations not retriedopen_db/init_schemaGate
Local gate green on
7f2a36e(432 s): fmt, clippy-D warnings, workspace tests withRUSTFLAGS=-D warnings, frontend build and tests with thresholds, e2e, Tauri crate tests, hook e2e, MSRV 1.94.0 build, install verify and the debug app build. Run withLUMEN_RANKED_TIME_BUDGET_MS=60000: the lumen-core ranked-outline budget test is wall-clock bound and flakes on a loaded machine; nothing here touches it.CRLF clone (
core.autocrlf=true): thelumen-core,lumen-daemonandlumen-statstests, 524 passed, none failed, includingbusy_ledger,busy_start,busy_connect, the upgrade matrix and the silent-fallback guard.