Repository navigation
Rails 8.0+ transaction materialization breaks other tests #1212
Description
Activity
My way of disabling the test:
diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb index 5bd1a06804..e9283264b6 100644 --- a/activerecord/test/cases/adapter_test.rb +++ b/activerecord/test/cases/adapter_test.rb @@ -510,7 +510,7 @@ def reset_fixtures(*fixture_names) end end - class AdapterConnectionTest < ActiveRecord::TestCase + false && class AdapterConnectionTest < ActiveRecord::TestCase unless in_memory_db? self.use_transactional_tests = falseThe MySQL tests run even better than the SQLite tests with this block disabled:
8831 runs, 29971 assertions, 37 failures, 79 errors, 44 skipsI think I've found the root cause. It's not the
materialize_transactionslogic itself — it's Rails'raw_transaction_open?test helper colliding with a legacy method alias on the JDBC raw connection.The collision
The helper probes for an open transaction on SQLite like this (adapter_test.rb):
when "SQLite" begin connection.instance_variable_get(:@raw_connection).transaction { nil } false rescue true end
On MRI,
SQLite3::Database#transactionwith a block isBEGIN→ yield →COMMIT— a net-zero probe that raisescannot start a transaction within a transactionwhen one is already open. Neat.Under AR-JDBC,
@raw_connectionis aSQLite3JdbcConnection, and itstransactionmethod is something else entirely: a legacy alias ofbegindating back to AR 4.0 support (RubyJdbcConnection.java#L335-L344). Since the Java method doesn't take a block, the block is silently ignored. So the probe:- with no transaction open: calls
setAutoCommit(false)— opening a real transaction that nothing ever commits — and returns the "right" answer while leaving a zombie transaction behind; - with a transaction open: hits
if (getAutoCommit())→ no-op, no exception, so the helper reports "no transaction" (this is why the assertions in that block also fail directly).
Reproduction (72-stable, AR 7.2.3.1, jruby-9.4.14.0 — the alias and the helper are identical on master / Rails 8):
raw = ActiveRecord::Base.connection.instance_variable_get(:@raw_connection) java = raw.jdbc_connection(true) java.auto_commit # => true raw.transaction { :ran } # => nil (block ignored; MRI returns the block value) java.auto_commit # => false (zombie transaction is now open) raw.transaction { nil } # => nil (no exception; MRI raises "cannot start a transaction within a transaction") # everything that follows silently lands inside the zombie tx — DDL included: conn.execute("CREATE TABLE poison_check (id integer)") conn.execute("INSERT INTO poison_check (id) VALUES (1)") raw.rollback conn.select_value("SELECT COUNT(*) FROM poison_check") # => SQLiteException: no such table: poison_check (the rollback erased the CREATE TABLE)
That last part is the cascade mechanism: after the probe runs, every subsequent statement on that pooled connection — fixture loads, schema changes, the transactional-test wrapper — runs inside a transaction Rails doesn't know about, and the next stray rollback/commit lands in the wrong place. Hence order-dependent failures in the thousands, and why disabling the whole
AdapterConnectionTestblock stabilizes the suite.Proposed fix
Give the SQLite JDBC connection a real
transactionmethod withSQLite3::Databasesemantics (block: begin → yield → commit, rollback + re-raise on error; raise when already in a transaction), shadowing the legacy alias — same shim approach as #1220 took for the PG branch of this helper (transaction_status/PG::PQTRANS_*). Removing the"transaction"alias from the base class outright would also work but risks breaking external callers, so I'd keep that separate.Happy to prepare a PR if this direction sounds right.
- with no transaction open: calls
@ryudoawaru Note there were some related fixes for transaction management in #1218 from @skunkworker.
I see what you see and yes, the
transactionmethod should be split off and handle a passed block appropriately. Go ahead and make a PR for that.
While working to finalize Rails 8.0 support I discovered a number of tests failing depending on the ordering of the test suite. I eventually narrowed the problems down to a set of tests for "transaction materialization" in the Rails 8.0 suite here:
https://github.com/rails/rails/blob/ff0b257d73b97d9bc5157150f93899aac3797f46/activerecord/test/cases/adapter_test.rb#L513
Several of the tests in this block use
@connection.materialize_transactionsto preserve a transaction across a reconnect. Either this logic or the associated setup/teardown of the spec causes the adapter to leave a transaction open on JRuby's adapter (tested on sqlite), usually causing many subsequent tests to fail until the system resets enough to return to a stable state.The number of failures can reach into the thousands depending on when this block of tests runs.
If I comment out this entire block, the test suite is much more stable, and the total number of failures is in the hundreds, roughly 3-5% of all tests run.
I have seen this as low as 230-ish F/E total.
Something's clearly not right with this test block or with the
materialize_transactionlogic when run with our adapters.