Merge pull request rails#44591 from rails/defer-db-connect

Simplify adapter construction; defer connect until first use

Author: matthewd

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

@@ -390,7 +390,7 @@ def commit_db_transaction()   end
       def rollback_db_transaction
         exec_rollback_db_transaction
       rescue ActiveRecord::ConnectionNotEstablished, ActiveRecord::ConnectionFailed
-        reconnect!
+        # Connection's gone; that counts as a rollback
       end
 
       def exec_rollback_db_transaction() end # :nodoc:

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

@@ -414,13 +414,6 @@ def materialize_transactions
             @has_unmaterialized_transactions = false
           end
         end
-
-        # As a logical simplification for now, we assume anything that requests
-        # materialization is about to dirty the transaction. Note this is just
-        # an assumption about the caller, not a direct property of this method.
-        # It can go away later when callers are able to handle dirtiness for
-        # themselves.
-        dirty_current_transaction
       end
 
       def commit_transaction
@@ -446,8 +439,12 @@ def commit_transaction
 
       def rollback_transaction(transaction = nil)
         @connection.lock.synchronize do
-          transaction ||= @stack.pop
-          transaction.rollback
+          transaction ||= @stack.last
+          begin
+            transaction.rollback
+          ensure
+            @stack.pop if @stack.last == transaction
+          end
           transaction.rollback_records
         end
       end
@@ -502,6 +499,9 @@ def within_new_transaction(isolation: nil, joinable: true)
 
                 begin
                   commit_transaction
+                rescue ActiveRecord::ConnectionFailed
+                  transaction.state.invalidate! unless transaction.state.completed?
+                  raise
                 rescue Exception
                   rollback_transaction(transaction) unless transaction.state.completed?
                   raise

activerecord/lib/active_record/connection_adapters/abstract_adapter.rb

@@ -89,34 +89,53 @@ def self.quoted_table_names # :nodoc:
         @quoted_table_names ||= {}
       end
 
-      def initialize(connection, logger = nil, config = {}) # :nodoc:
+      def initialize(config_or_deprecated_connection, deprecated_logger = nil, deprecated_connection_options = nil, deprecated_config = nil) # :nodoc:
         super()
 
-        @raw_connection      = connection
-        @owner               = nil
-        @instrumenter        = ActiveSupport::Notifications.instrumenter
-        @logger              = logger
-        @config              = config
-        @pool                = ActiveRecord::ConnectionAdapters::NullPool.new
-        @idle_since          = Process.clock_gettime(Process::CLOCK_MONOTONIC)
+        @raw_connection = nil
+        @unconfigured_connection = nil
+
+        if config_or_deprecated_connection.is_a?(Hash)
+          @config = config_or_deprecated_connection.symbolize_keys
+          @logger = ActiveRecord::Base.logger
+
+          if deprecated_logger || deprecated_connection_options || deprecated_config
+            raise ArgumentError, "when initializing an ActiveRecord adapter with a config hash, that should be the only argument"
+          end
+        else
+          # Soft-deprecated for now; we'll probably warn in future.
+
+          @unconfigured_connection = config_or_deprecated_connection
+          @logger = deprecated_logger || ActiveRecord::Base.logger
+          if deprecated_config
+            @config = (deprecated_config || {}).symbolize_keys
+            @connection_parameters = deprecated_connection_options
+          else
+            @config = (deprecated_connection_options || {}).symbolize_keys
+            @connection_parameters = nil
+          end
+        end
+
+        @owner = nil
+        @instrumenter = ActiveSupport::Notifications.instrumenter
+        @pool = ActiveRecord::ConnectionAdapters::NullPool.new
+        @idle_since = Process.clock_gettime(Process::CLOCK_MONOTONIC)
         @visitor = arel_visitor
         @statements = build_statement_pool
         @lock = ActiveSupport::Concurrency::LoadInterlockAwareMonitor.new
 
         @prepared_statements = self.class.type_cast_config_to_boolean(
-          config.fetch(:prepared_statements, true)
+          @config.fetch(:prepared_statements, true)
         )
 
         @advisory_locks_enabled = self.class.type_cast_config_to_boolean(
-          config.fetch(:advisory_locks, true)
+          @config.fetch(:advisory_locks, true)
         )
 
-        @default_timezone = self.class.validate_default_timezone(config[:default_timezone])
+        @default_timezone = self.class.validate_default_timezone(@config[:default_timezone])
 
         @raw_connection_dirty = false
         @verified = false
-
-        configure_connection
       end
 
       EXCEPTION_NEVER = { Exception => :never }.freeze # :nodoc:
@@ -147,7 +166,7 @@ def use_metadata_table?
       end
 
       def connection_retries
-        (@config[:connection_retries] || 3).to_i
+        (@config[:connection_retries] || 1).to_i
       end
 
       def default_timezone
@@ -314,7 +333,14 @@ def adapter_name
 
       # Does the database for this adapter exist?
       def self.database_exists?(config)
-        raise NotImplementedError
+        new(config).database_exists?
+      end
+
+      def database_exists?
+        connect!
+        true
+      rescue ActiveRecord::NoDatabaseError
+        false
       end
 
       # Does this adapter support DDL rollbacks in transactions? That is, would
@@ -598,6 +624,7 @@ def reconnect!(restore_transactions: false)
       def disconnect!
         clear_cache!(new_connection: true)
         reset_transaction
+        @raw_connection_dirty = false
       end
 
       # Immediately forget this connection ever existed. Unlike disconnect!,
@@ -658,10 +685,30 @@ def requires_reloading?
       # This is done under the hood by calling #active?. If the connection
       # is no longer active, then this method will reconnect to the database.
       def verify!
-        reconnect!(restore_transactions: true) unless active?
+        unless active?
+          if @unconfigured_connection
+            @lock.synchronize do
+              if @unconfigured_connection
+                @raw_connection = @unconfigured_connection
+                @unconfigured_connection = nil
+                configure_connection
+                @verified = true
+                return
+              end
+            end
+          end
+
+          reconnect!(restore_transactions: true)
+        end
+
         @verified = true
       end
 
+      def connect!
+        verify!
+        self
+      end
+
       def clean! # :nodoc:
         @raw_connection_dirty = false
         @verified = nil
@@ -864,6 +911,8 @@ def reconnect_can_restore_state?
         #
         def with_raw_connection(allow_retry: false, uses_transaction: true)
           @lock.synchronize do
+            connect! if @raw_connection.nil? && reconnect_can_restore_state?
+
             materialize_transactions if uses_transaction
 
             retries_available = allow_retry ? connection_retries : 0
@@ -909,6 +958,13 @@ def with_raw_connection(allow_retry: false, uses_transaction: true)
                 end
               end
 
+              unless retryable_query_error?(translated_exception)
+                # Barring a known-retryable error inside the query (regardless of
+                # whether we were in a _position_ to retry it), we should infer that
+                # there's likely a real problem with the connection.
+                @verified = false
+              end
+
               raise translated_exception
             ensure
               dirty_current_transaction if uses_transaction
@@ -942,7 +998,7 @@ def reconnect
         # to both be thread-safe and not rely upon actual server communication.
         # This is useful for e.g. string escaping methods.
         def any_raw_connection
-          @raw_connection
+          @raw_connection || valid_raw_connection
         end
 
         # Similar to any_raw_connection, but ensures it is validated and

activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb

@@ -51,10 +51,6 @@ def dealloc(stmt)
           end
       end
 
-      def initialize(connection, logger, connection_options, config)
-        super(connection, logger, config)
-      end
-
       def get_database_version # :nodoc:
         full_version_string = get_full_version
         version_string = version_string(full_version_string)

activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb

@@ -10,21 +10,7 @@ module ActiveRecord
   module ConnectionHandling # :nodoc:
     # Establishes a connection to the database that's used by all Active Record objects.
     def mysql2_connection(config)
-      config = config.symbolize_keys
-      config[:flags] ||= 0
-
-      if config[:flags].kind_of? Array
-        config[:flags].push "FOUND_ROWS"
-      else
-        config[:flags] |= Mysql2::Client::FOUND_ROWS
-      end
-
-      ConnectionAdapters::Mysql2Adapter.new(
-        ConnectionAdapters::Mysql2Adapter.new_client(config),
-        logger,
-        nil,
-        config,
-      )
+      ConnectionAdapters::Mysql2Adapter.new(config)
     end
   end
 
@@ -55,16 +41,25 @@ def new_client(config)
         end
       end
 
-      def initialize(connection, logger, connection_options, config)
-        check_prepared_statements_deprecation(config)
-        superclass_config = config.reverse_merge(prepared_statements: false)
-        super(connection, logger, connection_options, superclass_config)
-      end
+      def initialize(...)
+        super
+
+        @config[:flags] ||= 0
 
-      def self.database_exists?(config)
-        !!ActiveRecord::Base.mysql2_connection(config)
-      rescue ActiveRecord::NoDatabaseError
-        false
+        if @config[:flags].kind_of? Array
+          @config[:flags].push "FOUND_ROWS"
+        else
+          @config[:flags] |= Mysql2::Client::FOUND_ROWS
+        end
+
+        unless @config.key?(:prepared_statements)
+          ActiveSupport::Deprecation.warn(<<-MSG.squish)
+            The default value of `prepared_statements` for the mysql2 adapter will be changed from +false+ to +true+ in Rails 7.2.
+          MSG
+          @config[:prepared_statements] = false
+        end
+
+        @connection_parameters ||= @config
       end
 
       def supports_json?
@@ -120,7 +115,7 @@ def quote_string(string)
       #++
 
       def active?
-        @raw_connection.ping
+        !!@raw_connection&.ping
       end
 
       alias :reset! :reconnect!
@@ -129,30 +124,23 @@ def active?
       # Otherwise, this method does nothing.
       def disconnect!
         super
-        @raw_connection.close
+        @raw_connection&.close
+        @raw_connection = nil
       end
 
       def discard! # :nodoc:
         super
-        @raw_connection.automatic_close = false
+        @raw_connection&.automatic_close = false
         @raw_connection = nil
       end
 
       private
-        def check_prepared_statements_deprecation(config)
-          if !config.key?(:prepared_statements)
-            ActiveSupport::Deprecation.warn(<<-MSG.squish)
-              The default value of `prepared_statements` for the mysql2 adapter will be changed from +false+ to +true+ in Rails 7.2.
-            MSG
-          end
-        end
-
         def connect
-          @raw_connection = self.class.new_client(@config)
+          @raw_connection = self.class.new_client(@connection_parameters)
         end
 
         def reconnect
-          @raw_connection.close
+          @raw_connection&.close
           connect
         end