Define AbstractAdapter#configure_connection

All the adapters do this, so let's just accept that it's part of the
necessary structure. Doing this helps AbstractAdapter have better
control of the order of operations immediately after a reconnect.

Author: matthewd

activerecord/lib/active_record/connection_adapters/abstract_adapter.rb

@@ -112,6 +112,8 @@ def initialize(connection, logger = nil, config = {}) # :nodoc:
         )
 
         @default_timezone = self.class.validate_default_timezone(config[:default_timezone])
+
+        configure_connection
       end
 
       EXCEPTION_NEVER = { Exception => :never }.freeze # :nodoc:
@@ -550,11 +552,13 @@ def active?
       end
 
       # Disconnects from the database if already connected, and establishes a
-      # new connection with the database. Implementors should call super if they
-      # override the default implementation.
+      # new connection with the database. Implementors should call super
+      # immediately after establishing the new connection (and while still
+      # holding @lock).
       def reconnect!
         clear_cache!(new_connection: true)
         reset_transaction
+        configure_connection
       end
 
       # Disconnects from the database if already connected. Otherwise, this
@@ -584,10 +588,14 @@ def discard!
       # transactions and other connection-related server-side state. Usually a
       # database-dependent operation.
       #
-      # The default implementation does nothing; the implementation should be
-      # overridden by concrete adapters.
+      # If a database driver or protocol does not support such a feature,
+      # implementors may alias this to #reconnect!. Otherwise, implementors
+      # should call super immediately after resetting the connection (and while
+      # still holding @lock).
       def reset!
-        # this should be overridden by concrete adapters
+        clear_cache!(new_connection: true)
+        reset_transaction
+        configure_connection
       end
 
       # Removes the connection from the pool and disconnect it.
@@ -883,6 +891,16 @@ def build_statement_pool
         def build_result(columns:, rows:, column_types: {})
           ActiveRecord::Result.new(columns, rows, column_types)
         end
+
+        # Perform any necessary initialization upon the newly-established
+        # @raw_connection -- this is the place to modify the adapter's
+        # connection settings, run queries to configure any application-global
+        # "session" variables, etc.
+        #
+        # Implementations may assume this method will only be called while
+        # holding @lock (or from #initialize).
+        def configure_connection
+        end
     end
   end
 end

activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb

@@ -59,7 +59,6 @@ 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)
-        configure_connection
       end
 
       def self.database_exists?(config)
@@ -125,9 +124,11 @@ def active?
       end
 
       def reconnect!
-        super
-        disconnect!
-        connect
+        @lock.synchronize do
+          disconnect!
+          connect
+          super
+        end
       end
       alias :reset! :reconnect!
 
@@ -155,7 +156,6 @@ def check_prepared_statements_deprecation(config)
 
         def connect
           @raw_connection = self.class.new_client(@config)
-          configure_connection
         end
 
         def configure_connection

activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb

@@ -283,21 +283,13 @@ def connection_active?
 
       # Initializes and connects a PostgreSQL adapter.
       def initialize(connection, logger, connection_parameters, config)
-        super(connection, logger, config)
-
         @connection_parameters = connection_parameters || {}
 
-        # @local_tz is initialized as nil to avoid warnings when connect tries to use it
-        @local_tz = nil
         @max_identifier_length = nil
+        @type_map = nil
 
-        configure_connection
-        add_pg_encoders
-        add_pg_decoders
+        super(connection, logger, config)
 
-        @type_map = Type::HashLookupTypeMap.new
-        initialize_type_map
-        @local_tz = execute("SHOW TIME ZONE", "SCHEMA").first["TimeZone"]
         @use_insert_returning = @config.key?(:insert_returning) ? self.class.type_cast_config_to_boolean(@config[:insert_returning]) : true
       end
 
@@ -318,31 +310,38 @@ def active?
       end
 
       def reload_type_map # :nodoc:
-        type_map.clear
-        initialize_type_map
+        @lock.synchronize do
+          if @type_map
+            type_map.clear
+          else
+            @type_map = Type::HashLookupTypeMap.new
+          end
+
+          initialize_type_map
+        end
       end
 
       # Close then reopen the connection.
       def reconnect!
         @lock.synchronize do
-          @raw_connection.reset
+          begin
+            @raw_connection.reset
+          rescue PG::ConnectionBad
+            connect
+          end
+
           super
-          configure_connection
-          reload_type_map
-        rescue PG::ConnectionBad
-          connect
         end
       end
 
       def reset!
         @lock.synchronize do
-          reset_transaction
           unless @raw_connection.transaction_status == ::PG::PQTRANS_IDLE
             @raw_connection.query "ROLLBACK"
           end
           @raw_connection.query "DISCARD ALL"
-          clear_cache!(new_connection: true)
-          configure_connection
+
+          super
         end
       end
 
@@ -853,9 +852,6 @@ def prepare_statement(sql, binds)
         # connected server's characteristics.
         def connect
           @raw_connection = self.class.new_client(@connection_parameters)
-          configure_connection
-          add_pg_encoders
-          add_pg_decoders
         end
 
         # Configures the encoding, verbosity, schema search path, and time zone of the connection.
@@ -872,16 +868,6 @@ def configure_connection
 
           variables = @config.fetch(:variables, {}).stringify_keys
 
-          # If using Active Record's time zone support configure the connection to return
-          # TIMESTAMP WITH ZONE types in UTC.
-          unless variables["timezone"]
-            if default_timezone == :utc
-              variables["timezone"] = "UTC"
-            elsif @local_tz
-              variables["timezone"] = @local_tz
-            end
-          end
-
           # Set interval output format to ISO 8601 for ease of parsing by ActiveSupport::Duration.parse
           execute("SET intervalstyle = iso_8601", "SCHEMA")
 
@@ -895,6 +881,28 @@ def configure_connection
               execute("SET SESSION #{k} TO #{quote(v)}", "SCHEMA")
             end
           end
+
+          add_pg_encoders
+          add_pg_decoders
+
+          reload_type_map
+        end
+
+        def reconfigure_connection_timezone
+          variables = @config.fetch(:variables, {}).stringify_keys
+
+          # If it's been directly configured as a connection variable, we don't
+          # need to do anything here; it will be set up by configure_connection
+          # and then never changed.
+          return if variables["timezone"]
+
+          # If using Active Record's time zone support configure the connection
+          # to return TIMESTAMP WITH ZONE types in UTC.
+          if default_timezone == :utc
+            execute("SET SESSION timezone TO 'UTC'", "SCHEMA")
+          else
+            execute("SET SESSION timezone TO DEFAULT", "SCHEMA")
+          end
         end
 
         # Returns the list of a table's column names, data types, and default values.
@@ -987,7 +995,7 @@ def update_typemap_for_default_timezone
 
             # if default timezone has changed, we need to reconfigure the connection
             # (specifically, the session time zone)
-            configure_connection
+            reconfigure_connection_timezone
           end
         end
 

activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb

@@ -88,7 +88,6 @@ def dealloc(stmt)
       def initialize(connection, logger, connection_options, config)
         @memory_database = config[:database] == ":memory:"
         super(connection, logger, config)
-        configure_connection
       end
 
       def self.database_exists?(config)
@@ -165,12 +164,17 @@ def active?
       end
 
       def reconnect!
-        unless @raw_connection.closed?
-          @raw_connection.rollback rescue nil
+        @lock.synchronize do
+          if active?
+            @raw_connection.rollback rescue nil
+          else
+            connect
+          end
+
+          super
         end
-        super
-        connect if @raw_connection.closed?
       end
+      alias :reset! :reconnect!
 
       # Disconnects from the database if already connected. Otherwise, this
       # method does nothing.
@@ -608,7 +612,6 @@ def connect
             @config[:database].to_s,
             @config.merge(results_as_hash: true)
           )
-          configure_connection
         end
 
         def configure_connection