Fix COMMIT/ROLLBACK/savepoint retry data-loss; harden MySQL connection-loss detection

[skunkworker]

Summary

Splits the connection-retry correctness fix out of #1216 into a focused PR.

Transaction terminators must not be retried on a dropped backend. On a
networked database, retrying a COMMIT / ROLLBACK / savepoint statement
through with_raw_connection(allow_retry: true) would reconnect, replay an
empty transaction (the original writes died with the old backend), commit it
successfully, and report success — silently losing the transaction's writes.

This routes those statements through
with_raw_connection(allow_retry: false, materialize_transactions: true),
matching ActiveRecord's native adapters, and forwards the retry kwargs through
exec_query so ordinary statements stay retryable.

It also hardens MySQL/MariaDB connection-loss detection: a dropped server
connection (SQLState class 08, vendor "server gone" codes like
CR_SERVER_GONE_ERROR/CR_SERVER_LOST, or a recoverable/non-transient cause)
is now translated to ActiveRecord::ConnectionFailed so the reconnect/retry
machinery engages — mirroring the PostgreSQL adapter's handling of proxy-dropped
connections (e.g. ProxySQL / pgbouncer).

Changes

• abstract/transaction_support.rb — allow_retry: false, materialize_transactions: true for COMMIT/ROLLBACK + all savepoint ops
• abstract/database_statements.rb — forward allow_retry/materialize_transactions through exec_query
• mysql/adapter.rb — connection_lost? detection (SQLStates, vendor error codes, message patterns, recoverable/non-transient causes) + translate_exception hook
• Tests: mysql/commit_no_retry_test.rb, sqlite3/transaction_no_retry_test.rb, mysql/connection_lost_test.rb, postgresql/connection_lost_test.rb

Notes

Part of a larger split of #1216 into reviewable, topic-scoped PRs. Targets 72-stable. No conflicts with the other carved branches.

🤖 Used Claude Opus 4.8

Justification

Existing functionality today:

#commit_db_transaction

https://github.com/rails/rails/blob/7-2-stable/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb#L111-L113

#exec_rollback_db_transaction

https://github.com/rails/rails/blob/7-2-stable/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb#L116-L119

create_savepoint changes

The change in this PR

  # BEFORE  (72-stable base — and still on arjdbc master today)
  def create_savepoint(name = current_savepoint_name)
    log("SAVEPOINT #{name}", 'TRANSACTION') do
      with_raw_connection(allow_retry: true, materialize_transactions: false) do |conn|
        conn.create_savepoint(name)
      end
    end
  end

  # AFTER  (pr/retry-correctness)
  def create_savepoint(name = current_savepoint_name)
    log("SAVEPOINT #{name}", 'TRANSACTION') do
      with_raw_connection(allow_retry: false, materialize_transactions: true) do |conn|
        conn.create_savepoint(name)
      end
    end
  end

Native AR defines the savepoint methods in Savepoints, routing through internal_execute with no explicit flags — so it takes internal_execute's defaults, which
are allow_retry: false, materialize_transactions: true:

  # active_record/connection_adapters/abstract/savepoints.rb
  def create_savepoint(name = current_savepoint_name)
    internal_execute("SAVEPOINT #{name}", "TRANSACTION")   # defaults: allow_retry: false, materialize_transactions: true
  end

• Savepoint methods: https://github.com/rails/rails/blob/7-2-stable/activerecord/lib/active_record/connection_adapters/abstract/savepoints.rb#L11-L21
• internal_execute defaults (allow_retry: false, materialize_transactions: true):
  https://github.com/rails/rails/blob/7-2-stable/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb

So the PR brings ArJdbc's savepoint handling from the unsafe allow_retry: true, materialize_transactions: false into exact parity with native AR's allow_retry:
false, materialize_transactions: true — the same fix and reasoning as the COMMIT/ROLLBACK methods, and master still needs this change applied separately.

Why shouldn't MySQL be broken into it's own PR?

The changes to bring transaction_support.rb logic in line with what is on AR 7.2, would expose Mysql to these situations where the connection could be dropped, an error is raised (currently a non-retryable JDBCError, not handling situations where the connection is need to be retried. And shipping these two separately could open MySQL to data loss.

Having them be both in this MR allows for the proper connection retry logic to bubble up to ActiveRecord, and prevents data loss from occuring.

The sqlite test

SQLITE is just a testing change without any actual logic needed. And will run if PG/MYSQL is not present but ensure the code works as expected.

[headius]

In the interest of moving the critical part of this fix forward (correct handling of restored connections with active tranactions) I approve these changes, but my comments here need to be addressed in another PR:

• More effort to try to align our copies of DB logic with Rails, such as adding the internal_execute method that defaults these retry/materialization kwargs.
• Eliminate likely-duplicate tests added here where they already exist in Rails.

Claude does not have enough context to know that this repository includes copied code from rails/rails nor that we use the tests from rails/rails as our primary acceptance suite.

[headius]

Equivalent code in Rails just falls back on default kwarg parameters for allow_retry: false and materialize_transactions: true in the internal_execute method they have and we do not. We probably want to try to match that relationship in the future.

https://github.com/rails/rails/blob/77237954863d92e09708e7815bfc8e72847f4a0a/activerecord/lib/active_record/connection_adapters/abstract/savepoints.rb#L12

[headius]

Inconsistently, Rails does not fall back in internal_execute defaults here and passes these values explicitly. 🤨

https://github.com/rails/rails/blob/743763b8607a6017d9b549e49273b313692eccda/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb#L118

[headius]

And this one falls back on defaults again.

https://github.com/rails/rails/blob/77237954863d92e09708e7815bfc8e72847f4a0a/activerecord/lib/active_record/connection_adapters/abstract/savepoints.rb#L20

[headius]

I'd have preferred this be a separate PR but I understand they are somewhat interrelated.

Note that this mimics the same changes for PG in #1220 and these changes were suggested and encouraged there.

[headius]

We can merge these generated tests for now, but Claude is not considering the presence of similar tests in Rails. We are very likely duplicating tests and should review and remove where if that is the case.

[headius]

@skunkworker This is still marked as a draft with your "Draft: ..." subject. If you're satisfied with this please update the topic appropriately (and if it's really a draft, use a draft PR in the future).