feat: Run SQLite in WAL mode - #98
Conversation
|
✅ no critical SBOM vulnerabilities detected This PR previously reported critical SBOM vulnerabilities, but the latest |
|
From my understanding, the flakyness of the scheduler tests is not limited to the sqlite backend + WAL seems to enable concurrent reading and writing, that blocking behavior is what we relied on for the blocking updates on sqlite |
|
I am not super confortable with the details here, so I would need to dig before approving. If we are confident the issue is only caused by our test setup and cannot realistically hit the live service, then I would suggest we amend our test sessions/connections to workaround this issue and leave the live code untouched. e.g. with what I suggested here: https://pasqalworkspace.slack.com/archives/C0ALCCEJT2T/p1788200519081809?thread_ts=1787750111.052089&cid=C0ALCCEJT2T Let me know if you disagree |
4dd1402 to
fd3e49a
Compare
Under SQLite's default rollback journal, a connection holding an
unfinished read keeps the shared lock, and any concurrent COMMIT then
fails immediately with SQLITE_BUSY ("database is locked"). SQLite does
not consult the busy handler for that conflict, so busy_timeout does
not help - only WAL does.
This is reachable in the scheduler: `job_update_commiter` commits job
updates while other tasks read the same file, and it swallows the
failure, so a losing COMMIT silently drops a job update - including the
backend_id that resuming a job relies on.
It also made the sqlite test job flaky. Cancelling a task inside a
query - which every scheduler test does on teardown - abandons its
statement half-read, so that connection holds the shared lock until it
is closed. A sweep of 40 cancel timings failed 15 times before this
change and 0 times after.
Engine creation moves into `build_engine` so all callers get the
pragma, and it applies `echo` from the config instead of each caller
repeating it.
Two test fixtures also dropped `engine.dispose()` whenever their
`drop_all` teardown raised. Every backend shares one database across
the session, so that leaked the failing test's connections - and their
locks - into every later test, which is how one flake failed the two
following tests too. Both now dispose in a `finally`.
a4e4952 to
c161a2f
Compare
07f8094 to
ec4d910
Compare
3 participants
Some tests errors surfaced that the way we're using sqlalchemy was problematic when canceling a session while some operations were underway.
Some exploration with Claude yielded the following PR.
-- Below is AI generated --
Under SQLite's default rollback journal, a connection holding an unfinished read keeps the shared lock, and any concurrent COMMIT then fails immediately with SQLITE_BUSY ("database is locked"). SQLite does not consult the busy handler for that conflict, so busy_timeout does not help - only WAL does.
This is reachable in the scheduler:
job_update_commitercommits job updates while other tasks read the same file, and it swallows the failure, so a losing COMMIT silently drops a job update - including the backend_id that resuming a job relies on.It also made the sqlite test job flaky. Cancelling a task inside a query - which every scheduler test does on teardown - abandons its statement half-read, so that connection holds the shared lock until it is closed. A sweep of 40 cancel timings failed 15 times before this change and 0 times after.
Engine creation moves into
build_engineso all callers get the pragma, and it appliesechofrom the config instead of each caller repeating it.Two test fixtures also dropped
engine.dispose()whenever theirdrop_allteardown raised. Every backend shares one database across the session, so that leaked the failing test's connections - and their locks - into every later test, which is how one flake failed the two following tests too. Both now dispose in afinally.