CI failures look legit:

Error:
DefaultScopingTest#test_default_scope_with_conditions_hash:
RuntimeError: Wrapped undumpable exception for: ActiveRecord::StatementInvalid: SQLite3::BusyException: database is locked
    /usr/local/bundle/gems/sqlite3-1.6.9-x86_64-linux/lib/sqlite3/resultset.rb:162:in `step'
    /usr/local/bundle/gems/sqlite3-1.6.9-x86_64-linux/lib/sqlite3/resultset.rb:162:in `next_hash'
    /usr/local/bundle/gems/sqlite3-1.6.9-x86_64-linux/lib/sqlite3/resultset.rb:97:in `next'
    /usr/local/bundle/gems/sqlite3-1.6.9-x86_64-linux/lib/sqlite3/resultset.rb:125:in `each'

@byroot: Interesting. I cannot reproduce locally.

$ bundle exec rake test:sqlite3
Using sqlite3
Run options: --seed 41270

Finished in 106.020541s, 81.4182 runs/s, 276.5973 assertions/s.
8632 runs, 29325 assertions, 0 failures, 0 errors, 33 skips

I'm guessing Buildkite runs tests in parallel? How could I best mimic Buildkite's test setup on my local machine? I need to reproduce to start hunting down the best way to get tests passing on Buildkite.

I'm guessing Buildkite runs tests in parallel?

Not as far as I know. However it runs test on Linux, perhaps it's not linking to the same version of sqlite3?

My first experiment failed, but I have honed in on where the issue arises, but without getting to the point of understanding precisely what the issue is.

The issue has to do with setting up fixtures. My bet is that the setup_shared_connection_pool method is where things get setup in a way that doesn’t like immediate transactions. At the theoretical level, it would seem to the case that Rails is opening transactions for fixtures that never make a write. So, when transactions were deferred, those transactions never needed the db lock and thus ran fine. In immediate mode, they ask for the lock and doesn’t receive it in the timeout window. Locally, it must be the case that my laptop is faster than Buildkite’s machines and so my connections can complete their linear tasks in less than the timeout.

I can see that we open transactions in 4 connections, 3 of which are for the same database file (fixtures.sqlite3). And the lock exception is thrown on the last connection. I'm not immediately certain why we need to have multiple connections to the same database file. But, regardless, I haven't yet found why trying to open an immediate transaction on the final connection throws a "database locked" exception. Will keep debugging.

Making progress. I'm down to a few sporadic failures where the tests simply take longer than the busy_timeout and thus the busy exception is thrown.

What is the default busy timeout? Should it be higher?

What is the default busy timeout? Should it be higher?

For tests, Rails sets up connections for the fixtures with a 5 second timeout (see here). This matches the 5 second default that Rails generates for a SQLite database.yml file.

I'll try bumping the test timeout and seeing if that resolves the tests, tho it will be flaky of course. I can also keep working on trying to shore up the underlying logic for handling fixtures in tests. If we don't keep so many connections to the same database in our tests, this problem goes away more resiliently.

Still failing. It is a pretty consistent, and small, collection of test cases that are failing. My current hypothesis is that these tests have connections that aren't covered by the config.yml setup that sets the transaction mode for fixtures to deferred. Since the fixture setup starts a transaction for every connection present at the start of the test, if any connection is open that hasn't been configured to use the deferred transaction mode, the fixture setup will open an immediate transaction. Trying to open a second transaction to the same database after an immediate transaction has been opened will immediately throw a busy exception.

I think that we may need to pair this PR with a refactor of the fixture_setup to always open deferred transactions for dealing with fixtures.

@dhh: Unrelated ujs and app:update test failures aside, this PR is green. Needed to ensure that fixture transactions were deferred. Instead of introducing a new option to the transaction method felt improper, as transaction modes are not meaningful to other DBMS. So, I isolated the change to the SQLite3 adapter and fixtures. Then, I needed one other fix for a transaction test.

Looks like there are still busy failures on the CI?

Ah, I pushed what I thought was an irrelevant change (removing the default_transaction_mode: deferred from the test config), and it was clearly very relevant. There are a number of tests it seems that open multiple transactions to the same database. With the default busy_timeout blocking GVL, these tests take 5 seconds and then fail. It seems that this PR needs the non-blocking busy_handler_timeout that I am working to get into the SQLite3 repo (sparklemotion/sqlite3-ruby#443) after another PR discussion (#50370) ended with the decision to move this logic from Rails down to the SQLite3 repo. I will inject that fix locally and try to isolate those tests that require running deferred transactions and fix those.

However, it seems that this PR is blocked for now.

An alternative thought I just had:

We could go another direction altogether and only have the automatically created transactions used to wrap ActiveRecord's write operations run as immediate, while keeping the default behavior of deferred transactions for manually created transactions. I will see if I can make that happen.

@dhh: I found the entry point for ActiveRecord write operation transactions. I have refactored the entire PR to only make these operations INMEDIATE transactions. So, the default of deferred transactions remains the same. This was a long road, as I naively presumed that both automatic and manual transactions would basically always have a write operation. After slogging thru tests, I realize that a more isolated change is probably a more solid and resilient change for Rails. There are the flaky app:update and ujs test failures, but the ActiveRecord tests are all green (sorry for the false promise last time; double checked this time 😉)

Other option is allowing the adapters to be able to take over that transaction method.

Other option is allowing the adapters to be able to take over that transaction method.

True. I can try that out and push it up and you can consider that as a wholistic diff. Will do that shortly.

@dhh: ActiveRecord::Base#with_transaction_returning_status now calls Adapter#transaction_returning_status instead of Adapter#transaction directly. By default, that method is simply as alias, but the SQLite3 adapter implements the transaction_returning_status method to ensure that the immediate transaction mode is used. Transaction mode setting is done via the use_*_transaction_mode! methods added to the SQLite3 adapter, which the test_fixtures.rb module uses as well to ensure that fixture transactions always use deferred transactions.

This implementation doesn't leak the concept of "transaction modes" to any other adapters. WDYT?

(as always, rake test:ujs tests are the only thing making this build red)

@matthewd @byroot: I resolved conflicts on the CHANGELOG, and I will squash commits once we agree on a direction. I'd greatly appreciate your thoughts here. I updated the initial post to match the current state of affairs, and I'm happy to answer any questions you might have. This really is a big pain for production SQLite applications, and I'd love to find a path forward here that works for everyone.

@byroot @matthewd: Another CHANGELOG resolution. I would love to get some eyes on this.

@fractaledmind - we've been running ONCE Campfire (https://once.com/campfire) with this setting since it was released and it's worked well so, +1 for this. With deferred transactions, busy exceptions were a constant problem. With immediate transactions plus a GVL releasing timeout they were virtually non-existent.

I guess if you had an app that heavily relied on read only transactions, deferred mode might be preferable for those. In that case I think you'd still want this setting as the default so that transactions created internally for writes by ActiveRecord are not deferred.

So in that case it would be nice to be able to set the mode per-transaction though that would leak sqlite specific settings into the transaction method. Maybe something to revisit later if it is a real issue.

I modified your PR so it's enabled in the adapter by default rather than in the default generated config, as to have coverage on CI.

But it's causing issues with transactional fixtures, so I'll need to dig more into this when I got time.

I modified your PR so it's enabled in the adapter by default rather than in the default generated config, as to have coverage on CI.

But it's causing issues with transactional fixtures, so I'll need to dig more into this when I got time.

@byroot: Sorry for the delayed response, been on vacation. I had originally enabled this in the adapter as well and ran into the same issues. Basically, Rails' transaction fixtures require DEFERRED transactions. I began fixing this by introducing a keyword option to the transaction method that allowed one to specify which kind of transaction needed, but DHH didn't love the idea of a SQLite-specific concept leaking into the global ActiveRecord transaction method signature. I then implemented #use_deferred_transactions! and #use_immediate_transactions! methods in the SQLite3 adapter, and called them from within the transaction fixtures, but changes to the fixtures logic in the interim broke that logic.

Eventually, I landed on trying to simply push this out of Rails and into the application. I need to add back a mechanism for specifying a specific transaction mode (either an additional kwarg in transaction or the #user_*_transactions! methods), but I pushed up that first change to ensure that CI would pass (which it did). By pushing this change out of Rails, I ensure that transactional fixtures in Rails' CI always work. But, I wasn't able to test, while on vacation, if transaction fixtures in the application's CI would then start failing with such a change.

If you'd like, I can write up a more detailed explanation of the difficulty of patching transactional fixtures to use DEFERRED transactions, while having the rest of AR default to IMMEDIATE transactions, given the current state of that code (TL;DR the abstraction doesn't provide a clean place to make the change for only transactional fixtures, as the transaction itself has been moved up into AR now).

Finding some solution for this is very important for Rails 8 tho, so whatever it takes, I'm here to do whatever I can.

The fixtures transaction uses joinable: false, you can probably just use that to decide on defered VS immediate.

@byroot: Interesting thought! This helped me finalize a comprehensive approach. I just pushed a new commit which handles 3 details:

1. All SQLite3 connection default to using IMMEDIATE transactions
2. Any joinable transaction issues a DEFERRED transaction
3. Users can manually specify that a transaction be either IMMEDIATE or DEFERRED, but only for the SQLite3 adapter

This provides the full suite of tools to ensure applications can be built precisely as needed, but with productive defaults.

A couple of notes:

• I decided against allowing the default transaction mode to be overridden in the database.yml, as I can't see a case where a Rails app would need the opposite default. Most transactions will be write operations, which should be immediate transactions. However, I'm open to your opinion here
• Strictly speaking, I could remove the mode: option from the SQLite3Adapter#transaction method signature, and simply rely on the !joinable? == :deferred logic added to give users control over transaction mode. I added the new kwarg for increased clarity and to avoid overloading the meaning of joinable. Again, though, I am open to your opinion.