Repository navigation
fix(db): tolerate a concurrent WAL switch on a fresh SQLite file - #722
Merged
Merged
Conversation
Two connections opening a fresh file at once raced on PRAGMA journal_mode=WAL, which SQLite fails with 'database is locked' without waiting out busy_timeout. Skip the switch when the file is already WAL and retry a lost switch until busy_timeout runs out. Flaked test_concurrent_callers_keep_tenants_and_pools_separate. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Deploying simple-module-python with
|
| Latest commit: |
99cf8c6
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://bfeeebc8.simple-module-python.pages.dev |
| Branch Preview URL: | https://fix-sqlite-wal-switch-race.simple-module-python.pages.dev |
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
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.
Fixes a CI flake found while merging Dependabot PRs:
test_concurrent_callers_keep_tenants_and_pools_separate(from #720) failed on #676's Postgres job withsqlite3.OperationalError: database is locked.Cause. Every new SQLite connection ran
PRAGMA journal_mode=WAL. Switching a rollback-journal file to WAL needs an exclusive lock, and SQLite refuses it at once, without waiting outbusy_timeout, when another connection holds a lock. Two connections opening a fresh file together (two worker threads in that test; concurrent workers on a first boot in real use) could fail.Fix (
framework/db/simple_module_db/session.py,_ensure_wal):busy_timeoutruns out, then re-raise. Abusy_timeoutof 0 keeps the old fail-at-once behaviour.Tests (
framework/db/tests/test_db_sqlite_wal_switch.py):_ensure_walsucceeds once that connection finishes.Checks:
make lintpasses;PYTEST_WORKERS=8 make test-py: 6686 passed. A Sonnet review found no bugs; both minor points it raised are addressed (the event-loop note and the real-race test).https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4