Merge pull request rails#50371 from fractaledmind/ar-sqlite-immediate-transactions

Ensure SQLite transaction default to IMMEDIATE mode

Author: byroot

activerecord/CHANGELOG.md

@@ -1,3 +1,9 @@
+*   Use SQLite `IMMEDIATE` transactions when possible.
+
+    Transactions run against the SQLite3 adapter default to IMMEDIATE mode to improve concurrency support and avoid busy exceptions.
+
+    *Stephen Margheim*
+
 *   Raise specific exception when a connection is not defined.
 
      The new `ConnectionNotDefined` exception provides connection name, shard and role accessors indicating the details of the connection that was requested.

activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb

@@ -411,6 +411,14 @@ def add_transaction_record(record, ensure_finalize = true)
       # Begins the transaction (and turns off auto-committing).
       def begin_db_transaction()    end
 
+      def begin_deferred_transaction(isolation_level = nil) # :nodoc:
+        if isolation_level
+          begin_isolated_db_transaction(isolation_level)
+        else
+          begin_db_transaction
+        end
+      end
+
       def transaction_isolation_levels
         {
           read_uncommitted: "READ UNCOMMITTED",

activerecord/lib/active_record/connection_adapters/abstract/transaction.rb

@@ -448,10 +448,14 @@ def full_rollback?; false; end
     # = Active Record Real \Transaction
     class RealTransaction < Transaction
       def materialize!
-        if isolation_level
-          connection.begin_isolated_db_transaction(isolation_level)
+        if joinable?
+          if isolation_level
+            connection.begin_isolated_db_transaction(isolation_level)
+          else
+            connection.begin_db_transaction
+          end
         else
-          connection.begin_db_transaction
+          connection.begin_deferred_transaction(isolation_level)
         end
 
         super

activerecord/lib/active_record/connection_adapters/sqlite3/database_statements.rb

@@ -65,25 +65,16 @@ def exec_delete(sql, name = "SQL", binds = []) # :nodoc:
         end
         alias :exec_update :exec_delete
 
-        def begin_isolated_db_transaction(isolation) # :nodoc:
-          raise TransactionIsolationError, "SQLite3 only supports the `read_uncommitted` transaction isolation level" if isolation != :read_uncommitted
-          raise StandardError, "You need to enable the shared-cache mode in SQLite mode before attempting to change the transaction isolation level" unless shared_cache?
+        def begin_deferred_transaction(isolation = nil) # :nodoc:
+          internal_begin_transaction(:deferred, isolation)
+        end
 
-          with_raw_connection(allow_retry: true, materialize_transactions: false) do |conn|
-            ActiveSupport::IsolatedExecutionState[:active_record_read_uncommitted] = conn.get_first_value("PRAGMA read_uncommitted")
-            conn.read_uncommitted = true
-            begin_db_transaction
-          end
+        def begin_isolated_db_transaction(isolation) # :nodoc:
+          internal_begin_transaction(:deferred, isolation)
         end
 
         def begin_db_transaction # :nodoc:
-          log("begin transaction", "TRANSACTION") do
-            with_raw_connection(allow_retry: true, materialize_transactions: false) do |conn|
-              result = conn.transaction
-              verified!
-              result
-            end
-          end
+          internal_begin_transaction(:immediate, nil)
         end
 
         def commit_db_transaction # :nodoc:
@@ -114,6 +105,25 @@ def high_precision_current_timestamp
         end
 
         private
+          def internal_begin_transaction(mode, isolation)
+            if isolation
+              raise TransactionIsolationError, "SQLite3 only supports the `read_uncommitted` transaction isolation level" if isolation != :read_uncommitted
+              raise StandardError, "You need to enable the shared-cache mode in SQLite mode before attempting to change the transaction isolation level" unless shared_cache?
+            end
+
+            log("begin #{mode} transaction", "TRANSACTION") do
+              with_raw_connection(allow_retry: true, materialize_transactions: false) do |conn|
+                if isolation
+                  ActiveSupport::IsolatedExecutionState[:active_record_read_uncommitted] = conn.get_first_value("PRAGMA read_uncommitted")
+                  conn.read_uncommitted = true
+                end
+                result = conn.transaction(mode)
+                verified!
+                result
+              end
+            end
+          end
+
           def raw_execute(sql, name, async: false, allow_retry: false, materialize_transactions: false)
             log(sql, name, async: async) do |notification_payload|
               with_raw_connection(allow_retry: allow_retry, materialize_transactions: materialize_transactions) do |conn|

activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb

@@ -120,7 +120,11 @@ def initialize(...)
         end
 
         @config[:strict] = ConnectionAdapters::SQLite3Adapter.strict_strings_by_default unless @config.key?(:strict)
-        @connection_parameters = @config.merge(database: @config[:database].to_s, results_as_hash: true)
+        @connection_parameters = @config.merge(
+          database: @config[:database].to_s,
+          results_as_hash: true,
+          default_transaction_mode: :immediate,
+        )
         @use_insert_returning = @config.key?(:insert_returning) ? self.class.type_cast_config_to_boolean(@config[:insert_returning]) : true
       end
 

activerecord/test/cases/locking_test.rb

@@ -798,7 +798,7 @@ def duel(&block)
 
         a = Thread.new do
           t0 = Time.now
-          Person.transaction do
+          Person.transaction(joinable: false) do
             yield
             b_wakeup.set
             a_wakeup.wait

activerecord/test/cases/transactions_test.rb

@@ -1386,6 +1386,12 @@ def test_sqlite_add_column_in_transaction
         Topic.reset_column_information
       end
     end
+
+    def test_sqlite_default_transaction_mode_is_immediate
+      assert_queries_match(/BEGIN IMMEDIATE TRANSACTION/i, include_schema: false) do
+        Topic.transaction { Topic.lease_connection.materialize_transactions }
+      end
+    end
   end
 
   def test_transactions_state_from_rollback