Roll back transactions leaked by killed job threads in tests - #773
Merged
Conversation
Killing the job threads leaked by in-process workers can interrupt a thread in the middle of a database write, skipping the rollback in the transaction's ensure block while still returning the connection to the pool. The open transaction then locks SQLite for every other writer until the pool reaper flushes the idle connection minutes later, cascading "database is locked" errors through the rest of the suite. Wait for the killed threads to die and drop all pool connections, rolling back anything they left open. Transactional tests don't need it: they pin every thread to the test's own connection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Since #766, tests that stop in-process workers kill the job threads those workers leak. That kill can interrupt a thread in the middle of a database write, skipping the rollback in the transaction's ensure block while still returning the connection to the pool — with the write transaction still open.
On SQLite that open transaction locks out every other writer. Nothing clears it until the connection pool's reaper flushes the idle connection at its default 300-second
idle_timeout, so a single unlucky kill turns into a ~5-minute cascade ofSQLite3::BusyException: database is lockederrors through whatever tests run next, after which the suite mysteriously heals.That's been the signature of the recent flaky sqlite legs on CI: dozens of errors, always starting with
SolidQueue::JobTest#test_try_to_discard_claimed_jobfailing in its own teardown ~10 seconds after killing a job thread (it's the only non-transactional caller of the helper), always healing ~300 seconds later. For example, this run and this one, both on main.The fix: after killing the threads, wait for them to die, then drop all pool connections, which rolls back anything they left open. Transactional tests don't need it — they pin every thread to the test's own connection, so a killed thread can't strand a lock on a connection of its own.
The leaked state and the fix are easy to see in isolation: a thread killed while holding an open transaction leaves the pool with
raw_connection.transaction_active? == trueand the next writer times out; afterdisconnect!, the write goes through instantly.🤖 Generated with Claude Code