fix(daemon): Retry a busy store instead of failing live sessions - #126
Merged
Merged
Conversation
The event loop persisted each event in a deferred BEGIN that read before it wrote. A commit from another connection in between made the upgrade fail at once with SQLITE_BUSY_SNAPSHOT, which busy_timeout never retries. The catch then marked the session failed and left the loop while the harness kept running, so later events and usage were lost. Persist with BEGIN IMMEDIATE, and on any SQLITE_BUSY retry the same event with capped backoff instead of ending the loop. Other persistence errors still fail the session. classifyFailure now maps "database is locked" to STORE_BUSY with infra blame instead of UNKNOWN harness. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R6o1wJTTJWcHm9mZMGvDTn
|
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
2 participants
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.
Problem
attachDriverEventspersisted each event in a deferredBEGIN, andevents.appendreads (thesource_keydedupe andMAX(sequence)) before it inserts. If another connection commits in between, the read-to-write upgrade fails at once withSQLITE_BUSY_SNAPSHOT(errcode 517), whichbusy_timeoutnever retries.The
catchtreated that as a harness failure. It wrotesession.failed(UNKNOWN, harness blame), marked the rowfailedand left the loop, while the harness kept running. Its later events and usage were never persisted, andwaitreturned a false terminal state.#125 removed the main source of concurrent writers (stray daemons). This PR makes the loop safe against any other writer, such as
usage backfillor a lock held for longer than 5s.Change
persistDriverEventcommits the event, its cursor and the status update withBEGIN IMMEDIATE. The write lock is taken before the read, sobusy_timeoutapplies again.SQLITE_BUSY(base errcode 5, 517 included) it rolls back and retries the same event with capped backoff (25ms doubling, max 2s) instead of ending the loop.daemon.log.isStoreBusy()and aSTORE_BUSYfailure code:classifyFailure("database is locked")now returns infra blame, notUNKNOWNwith harness blame..specs/features/store-busy-healing/spec.md.Validation
tests/daemon-store-busy.test.ts(4 tests), written first and seen red:DatabaseSynccommits between the loop's read and write, the real interleaving. Before the fix, the event log held onlysession.failed. After it, the session endscompletedwith every event stored.session.failedis written.Full suite in a Node 22 container: 1634 pass, 1 fail. The failure (
open-claude-probes"probes again when the binary size changes") also fails onmain.Follow-up
A reconciler that reattaches sessions already marked
failedby a store error while their pid is alive, from the persisted offsets.🤖 Generated with Claude Code
https://claude.ai/code/session_01R6o1wJTTJWcHm9mZMGvDTn
Generated by Claude Code