Skip to content

fix(storage): retry contended writes across the whole Turn path - #319

Open
zch0227 wants to merge 1 commit into
mainfrom
fix/sqlite-turn-write-contention
Open

zch0227 wants to merge 1 commit into
mainfrom
fix/sqlite-turn-write-contention

Conversation

@zch0227

@zch0227 zch0227 commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator

Problem

Every mcode process opens the same runtime-state.sqlite in WAL mode, which
allows a single writer at a time. A concurrent process holding that write lock
longer than the five-second native busy_timeout surfaced SQLITE_BUSY
("database is locked") straight into the Turn, aborting it.

#287 added bounded-stall retry admission, but scoped it to the v2 message-upsert
path and stated that other SQLite writers keep their existing policy. The paths
named in #282 — turn admission, the event/session writer and the goal repo —
were still unprotected.

Reproduction

test/sqlite-turn-contention.test.ts forks a second process that holds
BEGIN IMMEDIATE on the real database for 8 seconds, then drives the real
DatabaseClient. On main, turn admission, session agent-state projection and
turn settlement all rejected with SQLITE_BUSY; only message upsert survived.

Change

Move the retry policy into @mavis/shared/sqlite-write-retry and apply it to
every turn-critical write:

  • v2 turn repository: admission, settlement, lease acquire/renew/release,
    deletion, steering receipts, restart recovery
  • v2 session repository: create, update, agent state, upsert, delete, reparent
  • v2 message repository: user commit, rewind, replacement batches, fork origin
  • v2 queue repository: both transaction chokepoints
  • v1 Goal store: all 13 writes, via runInImmediateTransaction

Each attempt takes only a 50ms native stall; the rest of the ten-second budget
is asynchronous backoff. better-sqlite3 blocks the Node event loop for the whole
native wait, so raising busy_timeout instead would only move the TUI freeze
from five seconds to thirty.

Only lock acquisition is replayed. A callback that already started rolls back
and propagates its own error unchanged, which keeps the fail-closed guarantees
#287 established for cancellation. A genuinely abandoned wait still reports as
WriteLockWaitAbortedError, now via a dedicated marker so a callback error that
merely happens to equal the signal's abort reason is not reclassified.

Reads keep their synchronous path: WAL readers are not blocked by a foreign
writer. Cron orchestration is also unchanged — its scheduler handler is
synchronous end to end, and a failed persist already reports CRON_PERSIST_FAILED.

Validation

  • pnpm verify: 13 of 14 gates pass on the committed tree (macOS arm64,
    Node v26.7.0), including typecheck, build, standalone boundary, source
    inventory, export preview and test:byok.
  • test:sandbox fails, but it fails identically on the base commit 44b13d3
    (3 failures there vs 2 here): the rm probes are routed through the desktop
    recoverable-delete launcher, which the sandbox denies. Unrelated to this change.
  • New: 4 turn-path contention tests and 2 Goal-store contention tests, all
    driving real better-sqlite3 against a foreign lock holder.
  • fix(storage): retry contended message writes and preserve cancellation #287's existing 12 contention tests remain green, including the cancellation
    and no-replay cases.

Fixes #282


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Every mcode process opens the same runtime-state.sqlite in WAL mode, so a
concurrent process holding the single write lock made a foreign writer's
SQLITE_BUSY surface as "database is locked" and aborted the Turn. PR #287
covered only the v2 message-upsert path; turn admission, session state
projection, settlement, queue writes and the v1 Goal store still failed after
the five-second native busy timeout.

Move the retry policy into @mavis/shared/sqlite-write-retry and apply it to
every turn-critical write. Each attempt takes only a 50ms native stall and the
rest of the ten-second budget is asynchronous backoff, so a foreign writer can
no longer block the event loop for the whole budget. Only lock acquisition is
replayed: a callback that already started rolls back and propagates unchanged.

Reads keep their synchronous path because WAL readers are not blocked by a
foreign writer. Cron orchestration stays on the old policy - its scheduler
handler is synchronous end to end and a failed persist is already reported as
CRON_PERSIST_FAILED.

Related to #282.
@hetaoBackend hetaoBackend added bug Something isn't working cli Standalone mcode: TUI, headless, ACP and source builds/tooling perf:full Run the full performance suite; required for performance-sensitive changes labels Sep 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working cli Standalone mcode: TUI, headless, ACP and source builds/tooling perf:full Run the full performance suite; required for performance-sensitive changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Concurrent mcode processes kill each other's turns: SQLITE_BUSY on shared runtime-state.sqlite

2 participants