Skip to content

Begin SQLite read-then-write transactions immediate - #1597

Open
csquared wants to merge 2 commits into
riverqueue:masterfrom
csquared:sqlite-begin-immediate
Open

Begin SQLite read-then-write transactions immediate#1597
csquared wants to merge 2 commits into
riverqueue:masterfrom
csquared:sqlite-begin-immediate

Conversation

@csquared

@csquared csquared commented Nov 11, 2026 •
edited
Loading

Copy link
Copy Markdown

With the SQLite driver on a pool of more than one connection, the job scheduler can fail intermittently with database is locked (5) (SQLITE_BUSY), even with busy_timeout set.

database/sql begins SQLite transactions deferred. Under WAL, a deferred transaction that reads and then writes can't upgrade to a write transaction once another connection has committed in between, and SQLite returns SQLITE_BUSY at once instead of waiting for busy_timeout. The scheduler's transaction reads eligible jobs before updating them, so it hits exactly this.

Change

  • BeginReadThenWrite on the executor, for a transaction that reads before it writes. The SQLite driver begins it with BEGIN IMMEDIATE, taking the write lock up front where busy_timeout applies. The Postgres drivers treat it as Begin.
  • The job scheduler begins its transaction with it, and JobSchedule uses it when it begins its own.
  • Every other transaction stays deferred, since those write first and an immediate transaction would hold the write lock across their reads.
  • database/sql can't choose the kind of BEGIN, and _txlock is a DSN option of only some drivers (Turso doesn't support it), so the immediate transaction replaces the deferred one on River's dedicated connection.

A pool of one connection is still the recommended configuration; this makes larger pools wait for the write lock instead of failing.

Tests

  • New shared driver tests: concurrent read-then-write transactions and concurrent job scheduling against inserts (8 workers × 20 iterations each), plus subtransaction cases. ConcurrentReadThenWrite first checks the pool really allows two open transactions, so it can't pass without contention.
  • SQLite, libSQL and Turso driver tests run on a pool of four connections with a busy timeout.
  • With BeginReadThenWrite changed to begin deferred, 135 of 160 and 43 of 160 of those operations fail with SQLITE_BUSY. With it immediate, all pass.
  • TestJobScheduler/BeginsReadThenWrite runs the scheduler on an executor whose Begin fails, so the scheduler must begin with BeginReadThenWrite.
  • TestExecutorBegin/FailedBeginImmediateDiscardsConnection covers a failed ROLLBACK and a failed BEGIN IMMEDIATE: the connection is discarded instead of hanging.
  • The second commit came from mutation testing the first one's changed lines (25 mutants). Each of the gaps above was a surviving mutant: reverting the scheduler to Begin, dropping either Rollback in beginImmediate, and a one-connection test pool all passed before and fail now.
  • The test pool's busy_timeout is now 30s instead of 5s. Under deliberate load (one -race process, CPUs oversubscribed, load ~24), ConcurrentReadThenWrite failed 6 of 100 runs at 5s, each time with one of 160 operations waiting past the timeout, since SQLite doesn't serve waiters in order. It failed 0 of 100 at 30s.
  • All run with -race: the SQLite, libSQL and Turso driver suites, riversqlite, internal/maintenance, and the pgx and database/sql driver suites against Postgres 17.

🤖 Generated with Claude Code

mrpink @claude mrblue
[go] Begin SQLite read-then-write transactions immediate
1b5fb09 
database/sql begins SQLite transactions deferred. Under WAL, a deferred
transaction that reads and then writes can't upgrade to a write
transaction once another connection has committed, and SQLite returns
SQLITE_BUSY at once without waiting for `busy_timeout`. The job
scheduler's transaction reads eligible jobs before updating them, so on
a pool of more than one connection it can fail intermittently.

Add `BeginReadThenWrite` to the executor. The SQLite driver begins it
with `BEGIN IMMEDIATE`, taking the write lock up front where
`busy_timeout` applies; the Postgres drivers treat it as `Begin`. The
job scheduler begins its transaction with it, and `JobSchedule` uses it
when it begins its own. River's other transactions write first and stay
deferred, so they don't hold the write lock across reads.

database/sql can't choose the kind of `BEGIN`, and `_txlock` is a DSN
option of some drivers that Turso doesn't support, so the immediate
transaction replaces the deferred one on River's dedicated connection.

Add concurrent read-then-write and job scheduling tests to the shared
driver tests and run SQLite, libSQL, and Turso tests on a pool of four
connections with a busy timeout. With `BeginReadThenWrite` deferred,
135 of 160 and 43 of 160 of their operations fail with SQLITE_BUSY. A
pool of one connection remains the recommended configuration.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@csquared
csquared force-pushed the sqlite-begin-immediate branch from fe7c9d2 to 1b5fb09 Compare November 11, 2026 15:33
@brandur brandur self-assigned this Nov 11, 2026
@brandur

brandur commented Nov 11, 2026

Copy link
Copy Markdown
Contributor

TY Chris! Would you mind signing our CLA please: https://github.com/riverqueue/rivercla

mrpink @claude mrblue
[go] Test the scheduler's read-then-write transaction and its failure…
c746975 
… paths

Mutation testing the previous commit's changed lines found gaps the
existing tests let through:

- Changing the job scheduler back to `Begin` passed every test. Add
  `TestJobScheduler/BeginsReadThenWrite`, which runs the scheduler on an
  executor whose `Begin` fails, so it must use `BeginReadThenWrite`.
- Removing either `tx.Rollback()` in `beginImmediate` after a failed
  `ROLLBACK` or `BEGIN IMMEDIATE` hung the connection's discard forever.
  Add `TestExecutorBegin/FailedBeginImmediateDiscardsConnection` for both.
- Setting the test pool back to one connection passed the concurrency
  tests without any contention. `ConcurrentReadThenWrite` now first
  checks that two transactions can be open at once.
- Add `InSubtransactionBeginsSubtransaction`.

`ConcurrentReadThenWrite` also flaked on a loaded host: with eight workers
queued for the write lock, SQLite doesn't serve waiters in order, and one
in 160 operations could wait past the test pool's 5s `busy_timeout`. Under
the same load (one -race process, CPUs oversubscribed, load ~24) it failed
6 of 100 runs at 5000ms and 0 of 100 at 30000ms, so the test pool's
`busy_timeout` is now 30s. It bounds how long a waiter may wait, not how
long the tests take when nothing contends.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@csquared

csquared commented Nov 11, 2026

Copy link
Copy Markdown
Author

Added a second commit with tests from mutation testing the first one's changed lines: the scheduler's use of BeginReadThenWrite and beginImmediate's failure paths are now covered (each was a surviving mutant), and the test pool's busy_timeout goes from 5s to 30s after ConcurrentReadThenWrite failed 6 of 100 runs on a loaded host at 5s and 0 of 100 at 30s. Details in the description's Tests section.

🤖 Generated with Claude Code

@csquared

csquared commented Nov 11, 2026 via email

Copy link
Copy Markdown
Author

@brandur

brandur commented Nov 13, 2026

Copy link
Copy Markdown
Contributor

@csquared Awesome. Just lemme know when you're finished up (and sign that CLA) and I'll take a look!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@brandur brandur

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@csquared @brandur