Skip to content

fix(persistence): retry sidecar open on the rollback→WAL conversion BUSY race - #179

Merged
zzet merged 1 commit into
mainfrom
fix/sidecar-concurrent-open-busy
Jun 27, 2026
Merged

fix(persistence): retry sidecar open on the rollback→WAL conversion BUSY race#179
zzet merged 1 commit into
mainfrom
fix/sidecar-concurrent-open-busy

Conversation

@zzet

@zzet zzet commented Jun 27, 2026

Copy link
Copy Markdown
Owner

Problem

Concurrent first opens of a stale (rollback-journal) sidecar raced and one process failed:

persistence: sidecar schema: begin: database is locked (5) (SQLITE_BUSY)

TestOpenSidecarConcurrentProcesses reproduces it (6 processes opening a pre-WAL DB at once).

Root cause

The sidecar DSN sets journal_mode=WAL, so the first openers convert the file rollback→WAL, which takes a brief EXCLUSIVE lock. SQLite does not consult the busy handler for a journal-mode change, so busy_timeout(5000) is bypassed and the loser gets an immediate SQLITE_BUSY — observed failing in ~0.02s, far under the 5s timeout. The existing _txlock=immediate + busy_timeout only serialize the post-conversion BEGIN IMMEDIATE write lock, not the conversion itself.

Fix

internal/persistence/sidecar_migrate.go + sidecar_sqlite.go:

  • isSidecarBusyErr — typed detection via modernc's *sqlite.Error, masking Code() & 0xff to match base SQLITE_BUSY (5) / SQLITE_LOCKED (6) and their extended codes.
  • withSidecarBusyRetry — bounded exponential backoff (40 attempts, 5ms→250ms).
  • OpenSidecar wraps both runBaseSchema and runMigrations in it. Both are idempotent (CREATE … IF NOT EXISTS / user_version-gated), so retry is safe.
  • Corrected the now-misleading DSN comment that claimed busy_timeout covers the conversion.

Verification

  • Load-bearing proof: stashing just these two files reproduces the exact reported error; restoring them fixes it.
  • Passes under -race (5× and 3×), a 30-iteration / 180-concurrent-open stress, and the full internal/persistence package under -race. go vet clean.

…BUSY race

Concurrent first opens of a stale (rollback-journal) sidecar raced and one
failed with "sidecar schema: begin: database is locked (5) (SQLITE_BUSY)".

The DSN sets journal_mode=WAL, so the first openers convert the file from
rollback to WAL, which takes a brief EXCLUSIVE lock. SQLite does not consult
the busy handler for a journal-mode change, so busy_timeout is bypassed and
the loser gets an immediate SQLITE_BUSY (observed in ~0.02s, far under the
5s timeout). _txlock=immediate + busy_timeout only serialise the
post-conversion BEGIN IMMEDIATE write lock, not the conversion itself.

Add a bounded application-level BUSY/LOCKED retry (withSidecarBusyRetry,
typed detection via the modernc *sqlite.Error code) and wrap runBaseSchema
and runMigrations in OpenSidecar with it. Both are idempotent
(CREATE ... IF NOT EXISTS / user_version-gated), so retry is safe.

Verified load-bearing: TestOpenSidecarConcurrentProcesses fails reliably
without the change and passes under -race (and a 180-open stress) with it.
@zzet
zzet merged commit 51e5bf8 into main Jun 27, 2026
9 checks passed
@zzet
zzet deleted the fix/sidecar-concurrent-open-busy branch June 27, 2026 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant