Merge pull request #37253 from Shopify/connection-pool-db-config

Instantiate ConnectionPool with a DatabaseConfig rather than a ConnectionSpecification

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

@@ -372,7 +372,7 @@ module ActiveRecord
       include ConnectionAdapters::AbstractPool
 
       attr_accessor :automatic_reconnect, :checkout_timeout, :schema_cache
-      attr_reader :spec, :size, :reaper
+      attr_reader :db_config, :size, :reaper
 
       # Creates a new ConnectionPool object. +spec+ is a ConnectionSpecification
       # object which describes database connection information (e.g. adapter,
@@ -380,14 +380,14 @@ module ActiveRecord
       # this ConnectionPool.
       #
       # The default ConnectionPool maximum size is 5.
-      def initialize(spec)
+      def initialize(db_config)
         super()
 
-        @spec = spec
+        @db_config = db_config
 
-        @checkout_timeout = spec.db_config.checkout_timeout
-        @idle_timeout = spec.db_config.idle_timeout
-        @size = spec.db_config.pool
+        @checkout_timeout = db_config.checkout_timeout
+        @idle_timeout = db_config.idle_timeout
+        @size = db_config.pool
 
         # This variable tracks the cache of threads mapped to reserved connections, with the
         # sole purpose of speeding up the +connection+ method. It is not the authoritative
@@ -415,7 +415,7 @@ module ActiveRecord
 
         @lock_thread = false
 
-        @reaper = Reaper.new(self, spec.db_config.reaping_frequency)
+        @reaper = Reaper.new(self, db_config.reaping_frequency)
         @reaper.run
       end
 
@@ -891,7 +891,7 @@ module ActiveRecord
         alias_method :release, :remove_connection_from_thread_cache
 
         def new_connection
-          Base.send(spec.db_config.adapter_method, spec.db_config.configuration_hash).tap do |conn|
+          Base.send(db_config.adapter_method, db_config.configuration_hash).tap do |conn|
             conn.check_version
           end
         end
@@ -1049,6 +1049,10 @@ module ActiveRecord
         self.prevent_writes = original
       end
 
+      def connection_pool_names # :nodoc:
+        owner_to_pool.keys
+      end
+
       def connection_pool_list
         owner_to_pool.values.compact
       end
@@ -1057,6 +1061,7 @@ module ActiveRecord
       def establish_connection(config)
         resolver = Resolver.new(Base.configurations)
         spec = resolver.spec(config)
+        db_config = spec.db_config
 
         remove_connection(spec.name)
 
@@ -1066,11 +1071,11 @@ module ActiveRecord
         }
         if spec
           payload[:spec_name] = spec.name
-          payload[:config] = spec.db_config.configuration_hash
+          payload[:config] = db_config.configuration_hash
         end
 
         message_bus.instrument("!connection.active_record", payload) do
-          owner_to_pool[spec.name] = ConnectionAdapters::ConnectionPool.new(spec)
+          owner_to_pool[spec.name] = ConnectionAdapters::ConnectionPool.new(db_config)
         end
 
         owner_to_pool[spec.name]
@@ -1141,7 +1146,7 @@ module ActiveRecord
         if pool = owner_to_pool.delete(spec_name)
           pool.automatic_reconnect = false
           pool.disconnect!
-          pool.spec.db_config.configuration_hash
+          pool.db_config.configuration_hash
         end
       end
 
@@ -1156,7 +1161,7 @@ module ActiveRecord
             # A connection was established in an ancestor process that must have
             # subsequently forked. We can't reuse the connection, but we can copy
             # the specification and establish a new connection with it.
-            establish_connection(ancestor_pool.spec.db_config.configuration_hash.merge(name: spec_name)).tap do |pool|
+            establish_connection(ancestor_pool.db_config.configuration_hash.merge(name: spec_name)).tap do |pool|
               pool.schema_cache = ancestor_pool.schema_cache if ancestor_pool.schema_cache
             end
           else

activerecord/lib/active_record/connection_adapters/abstract_adapter.rb

@@ -126,14 +126,19 @@ module ActiveRecord
       def schema_migration # :nodoc:
         @schema_migration ||= begin
                                 conn = self
-                                spec_name = conn.pool.spec.name
+                                spec_name = conn.pool.db_config.spec_name
                                 name = "#{spec_name}::SchemaMigration"
 
                                 Class.new(ActiveRecord::SchemaMigration) do
                                   define_singleton_method(:name) { name }
                                   define_singleton_method(:to_s) { name }
 
-                                  self.connection_specification_name = spec_name
+                                  connection_handler.connection_pool_names.each do |pool_name|
+                                    if conn.pool == connection_handler.retrieve_connection_pool(pool_name)
+                                      self.connection_specification_name = pool_name
+                                      break
+                                    end
+                                  end
                                 end
                               end
       end

activerecord/lib/active_record/connection_handling.rb

@@ -227,7 +227,7 @@ module ActiveRecord
     #
     # Please use only for reading.
     def connection_config
-      connection_pool.spec.db_config.configuration_hash
+      connection_pool.db_config.configuration_hash
     end
 
     def connection_pool

activerecord/lib/active_record/tasks/database_tasks.rb

@@ -139,7 +139,7 @@ module ActiveRecord
         old_pool = ActiveRecord::Base.connection_handler.retrieve_connection_pool(ActiveRecord::Base.connection_specification_name)
         each_local_configuration { |db_config| create(db_config) }
         if old_pool
-          ActiveRecord::Base.connection_handler.establish_connection(old_pool.spec.db_config)
+          ActiveRecord::Base.connection_handler.establish_connection(old_pool.db_config)
         end
       end
 

activerecord/lib/active_record/test_fixtures.rb

@@ -191,8 +191,7 @@ module ActiveRecord
 
         ActiveRecord::Base.connection_handlers.values.each do |handler|
           if handler != writing_handler
-            handler.connection_pool_list.each do |pool|
-              name = pool.spec.name
+            handler.connection_pool_names.each do |name|
               writing_connection = writing_handler.retrieve_connection_pool(name)
               handler.send(:owner_to_pool)[name] = writing_connection
             end

activerecord/test/cases/adapters/mysql2/schema_test.rb

@@ -11,7 +11,7 @@ module ActiveRecord
 
       def setup
         @connection = ActiveRecord::Base.connection
-        db          = Post.connection_pool.spec.db_config.database
+        db          = Post.connection_pool.db_config.database
         table       = Post.table_name
         @db_name    = db
 

activerecord/test/cases/connection_adapters/adapter_leasing_test.rb

@@ -40,7 +40,7 @@ module ActiveRecord
 
       def test_close
         db_config = ActiveRecord::DatabaseConfigurations::HashConfig.new("test", "primary", {})
-        pool = Pool.new(ConnectionSpecification.new("primary", db_config))
+        pool = Pool.new(db_config)
         pool.insert_connection_for_test! @adapter
         @adapter.pool = pool
 

activerecord/test/cases/connection_adapters/connection_handler_test.rb

@@ -63,13 +63,13 @@ module ActiveRecord
         @handler.establish_connection(:readonly)
 
         assert_not_nil pool = @handler.retrieve_connection_pool("readonly")
-        assert_equal "db/readonly.sqlite3", pool.spec.db_config.database
+        assert_equal "db/readonly.sqlite3", pool.db_config.database
 
         assert_not_nil pool = @handler.retrieve_connection_pool("primary")
-        assert_equal "db/primary.sqlite3", pool.spec.db_config.database
+        assert_equal "db/primary.sqlite3", pool.db_config.database
 
         assert_not_nil pool = @handler.retrieve_connection_pool("common")
-        assert_equal "db/common.sqlite3", pool.spec.db_config.database
+        assert_equal "db/common.sqlite3", pool.db_config.database
       ensure
         ActiveRecord::Base.configurations = @prev_configs
         ENV["RAILS_ENV"] = previous_env
@@ -93,7 +93,7 @@ module ActiveRecord
 
           ActiveRecord::Base.establish_connection
 
-          assert_match "db/primary.sqlite3", ActiveRecord::Base.connection.pool.spec.db_config.database
+          assert_match "db/primary.sqlite3", ActiveRecord::Base.connection.pool.db_config.database
         ensure
           ActiveRecord::Base.configurations = @prev_configs
           ENV["RAILS_ENV"] = previous_env
@@ -116,7 +116,7 @@ module ActiveRecord
 
           ActiveRecord::Base.establish_connection
 
-          assert_match "db/primary.sqlite3", ActiveRecord::Base.connection.pool.spec.db_config.database
+          assert_match "db/primary.sqlite3", ActiveRecord::Base.connection.pool.db_config.database
         ensure
           ActiveRecord::Base.configurations = @prev_configs
           ENV["RAILS_ENV"] = previous_env
@@ -132,7 +132,7 @@ module ActiveRecord
         @handler.establish_connection(:development)
 
         assert_not_nil pool = @handler.retrieve_connection_pool("development")
-        assert_equal "db/primary.sqlite3", pool.spec.db_config.database
+        assert_equal "db/primary.sqlite3", pool.db_config.database
       ensure
         ActiveRecord::Base.configurations = @prev_configs
       end
@@ -147,7 +147,7 @@ module ActiveRecord
         @handler.establish_connection(:development_readonly)
 
         assert_not_nil pool = @handler.retrieve_connection_pool("development_readonly")
-        assert_equal "db/readonly.sqlite3", pool.spec.db_config.database
+        assert_equal "db/readonly.sqlite3", pool.db_config.database
       ensure
         ActiveRecord::Base.configurations = @prev_configs
       end
@@ -211,7 +211,7 @@ module ActiveRecord
 
         assert_same klass2.connection, ActiveRecord::Base.connection
 
-        pool = klass2.establish_connection(ActiveRecord::Base.connection_pool.spec.db_config.configuration_hash)
+        pool = klass2.establish_connection(ActiveRecord::Base.connection_pool.db_config.configuration_hash)
         assert_same klass2.connection, pool.connection
         assert_not_same klass2.connection, ActiveRecord::Base.connection
 
@@ -353,28 +353,6 @@ module ActiveRecord
           end
         end
 
-        def test_retrieve_connection_pool_preserves_connection_specification_name
-          rd, wr = IO.pipe
-          rd.binmode
-          wr.binmode
-
-          @handler.establish_connection ActiveRecord::Base.configurations["arunit"].merge(name: "foo")
-
-          pid = fork {
-            rd.close
-            pool = @handler.retrieve_connection_pool("foo")
-            wr.write pool.spec.name
-            wr.close
-            exit!
-          }
-
-          wr.close
-
-          Process.waitpid pid
-          assert_equal "foo", rd.read
-          rd.close
-        end
-
         def test_retrieve_connection_pool_copies_schema_cache_from_ancestor_pool
           @pool.schema_cache = @pool.connection.schema_cache
           @pool.schema_cache.add("posts")

activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb

@@ -82,10 +82,10 @@ module ActiveRecord
           ActiveRecord::Base.connects_to(database: { writing: :primary, reading: :readonly })
 
           assert_not_nil pool = ActiveRecord::Base.connection_handlers[:writing].retrieve_connection_pool("primary")
-          assert_equal "db/primary.sqlite3", pool.spec.db_config.database
+          assert_equal "db/primary.sqlite3", pool.db_config.database
 
           assert_not_nil pool = ActiveRecord::Base.connection_handlers[:reading].retrieve_connection_pool("primary")
-          assert_equal "db/readonly.sqlite3", pool.spec.db_config.database
+          assert_equal "db/readonly.sqlite3", pool.db_config.database
         ensure
           ActiveRecord::Base.configurations = @prev_configs
           ActiveRecord::Base.establish_connection(:arunit)
@@ -140,10 +140,10 @@ module ActiveRecord
           ActiveRecord::Base.connects_to(database: { default: :primary, readonly: :readonly })
 
           assert_not_nil pool = ActiveRecord::Base.connection_handlers[:default].retrieve_connection_pool("primary")
-          assert_equal "db/primary.sqlite3", pool.spec.db_config.database
+          assert_equal "db/primary.sqlite3", pool.db_config.database
 
           assert_not_nil pool = ActiveRecord::Base.connection_handlers[:readonly].retrieve_connection_pool("primary")
-          assert_equal "db/readonly.sqlite3", pool.spec.db_config.database
+          assert_equal "db/readonly.sqlite3", pool.db_config.database
         ensure
           ActiveRecord::Base.configurations = @prev_configs
           ActiveRecord::Base.establish_connection(:arunit)
@@ -162,7 +162,7 @@ module ActiveRecord
             assert_equal handler, ActiveRecord::Base.connection_handlers[:writing]
 
             assert_not_nil pool = handler.retrieve_connection_pool("primary")
-            assert_equal({ adapter: "postgresql", database: "bar", host: "localhost" }, pool.spec.db_config.configuration_hash)
+            assert_equal({ adapter: "postgresql", database: "bar", host: "localhost" }, pool.db_config.configuration_hash)
           end
         ensure
           ActiveRecord::Base.establish_connection(:arunit)
@@ -182,7 +182,7 @@ module ActiveRecord
             assert_equal handler, ActiveRecord::Base.connection_handlers[:writing]
 
             assert_not_nil pool = handler.retrieve_connection_pool("primary")
-            assert_equal(config, pool.spec.db_config.configuration_hash)
+            assert_equal(config, pool.db_config.configuration_hash)
           end
         ensure
           ActiveRecord::Base.establish_connection(:arunit)
@@ -222,7 +222,7 @@ module ActiveRecord
             assert_equal handler, ActiveRecord::Base.connection_handlers[:writing]
 
             assert_not_nil pool = handler.retrieve_connection_pool("primary")
-            assert_equal(config["default_env"]["animals"], pool.spec.db_config.configuration_hash)
+            assert_equal(config["default_env"]["animals"], pool.db_config.configuration_hash)
           end
         ensure
           ActiveRecord::Base.configurations = @prev_configs
@@ -249,7 +249,7 @@ module ActiveRecord
             assert_equal handler, ActiveRecord::Base.connection_handlers[:writing]
 
             assert_not_nil pool = handler.retrieve_connection_pool("primary")
-            assert_equal(config["default_env"]["primary"], pool.spec.db_config.configuration_hash)
+            assert_equal(config["default_env"]["primary"], pool.db_config.configuration_hash)
           end
         ensure
           ActiveRecord::Base.configurations = @prev_configs
@@ -284,7 +284,7 @@ module ActiveRecord
           ActiveRecord::Base.connects_to database: { writing: :development, reading: :development_readonly }
 
           assert_not_nil pool = ActiveRecord::Base.connection_handlers[:reading].retrieve_connection_pool("primary")
-          assert_equal "db/readonly.sqlite3", pool.spec.db_config.database
+          assert_equal "db/readonly.sqlite3", pool.db_config.database
         ensure
           ActiveRecord::Base.configurations = @prev_configs
           ActiveRecord::Base.establish_connection(:arunit)

activerecord/test/cases/connection_pool_test.rb

@@ -12,7 +12,7 @@ module ActiveRecord
         super
 
         # Keep a duplicate pool so we do not bother others
-        @pool = ConnectionPool.new ActiveRecord::Base.connection_pool.spec
+        @pool = ConnectionPool.new(ActiveRecord::Base.connection_pool.db_config)
 
         if in_memory_db?
           # Separate connections to an in-memory database create an entirely new database,
@@ -198,9 +198,9 @@ module ActiveRecord
 
       def test_idle_timeout_configuration
         @pool.disconnect!
-        spec = ActiveRecord::Base.connection_pool.spec
-        spec.db_config.configuration_hash.merge!(idle_timeout: "0.02")
-        @pool = ConnectionPool.new(spec)
+        db_config = ActiveRecord::Base.connection_pool.db_config
+        db_config.configuration_hash.merge!(idle_timeout: "0.02")
+        @pool = ConnectionPool.new(db_config)
         idle_conn = @pool.checkout
         @pool.checkin(idle_conn)
 
@@ -223,9 +223,9 @@ module ActiveRecord
 
       def test_disable_flush
         @pool.disconnect!
-        spec = ActiveRecord::Base.connection_pool.spec
-        spec.db_config.configuration_hash.merge!(idle_timeout: -5)
-        @pool = ConnectionPool.new(spec)
+        db_config = ActiveRecord::Base.connection_pool.db_config
+        db_config.configuration_hash.merge!(idle_timeout: -5)
+        @pool = ConnectionPool.new(db_config)
         idle_conn = @pool.checkout
         @pool.checkin(idle_conn)
 
@@ -315,7 +315,7 @@ module ActiveRecord
       end
 
       def test_checkout_behaviour
-        pool = ConnectionPool.new ActiveRecord::Base.connection_pool.spec
+        pool = ConnectionPool.new(ActiveRecord::Base.connection_pool.db_config)
         main_connection = pool.connection
         assert_not_nil main_connection
         threads = []
@@ -448,7 +448,7 @@ module ActiveRecord
       end
 
       def test_automatic_reconnect_restores_after_disconnect
-        pool = ConnectionPool.new ActiveRecord::Base.connection_pool.spec
+        pool = ConnectionPool.new(ActiveRecord::Base.connection_pool.db_config)
         assert pool.automatic_reconnect
         assert pool.connection
 
@@ -457,7 +457,7 @@ module ActiveRecord
       end
 
       def test_automatic_reconnect_can_be_disabled
-        pool = ConnectionPool.new ActiveRecord::Base.connection_pool.spec
+        pool = ConnectionPool.new(ActiveRecord::Base.connection_pool.db_config)
         pool.disconnect!
         pool.automatic_reconnect = false
 
@@ -718,13 +718,12 @@ module ActiveRecord
 
       private
         def with_single_connection_pool
-          old_config = ActiveRecord::Base.connection_pool.spec.db_config.configuration_hash
+          old_config = ActiveRecord::Base.connection_pool.db_config.configuration_hash
           db_config = ActiveRecord::DatabaseConfigurations::HashConfig.new("arunit", "primary", old_config.dup)
-          one_conn_spec = ConnectionSpecification.new("primary", db_config)
 
-          one_conn_spec.db_config.configuration_hash[:pool] = 1 # this is safe to do, because .dupped ConnectionSpecification also auto-dups its config
+          db_config.configuration_hash[:pool] = 1 # this is safe to do, because .dupped ConnectionSpecification also auto-dups its config
 
-          yield(pool = ConnectionPool.new(one_conn_spec))
+          yield(pool = ConnectionPool.new(db_config))
         ensure
           pool.disconnect! if pool
         end

activerecord/test/cases/fixtures_test.rb

@@ -1412,7 +1412,7 @@ class MultipleDatabaseFixturesTest < ActiveRecord::TestCase
   private
     def with_temporary_connection_pool
       old_pool = ActiveRecord::Base.connection_handler.retrieve_connection_pool(ActiveRecord::Base.connection_specification_name)
-      new_pool = ActiveRecord::ConnectionAdapters::ConnectionPool.new ActiveRecord::Base.connection_pool.spec
+      new_pool = ActiveRecord::ConnectionAdapters::ConnectionPool.new(ActiveRecord::Base.connection_pool.db_config)
       ActiveRecord::Base.connection_handler.send(:owner_to_pool)["primary"] = new_pool
 
       yield

activerecord/test/cases/helper.rb

@@ -37,7 +37,7 @@ end
 
 def in_memory_db?
   current_adapter?(:SQLite3Adapter) &&
-  ActiveRecord::Base.connection_pool.spec.db_config.database == ":memory:"
+  ActiveRecord::Base.connection_pool.db_config.database == ":memory:"
 end
 
 def subsecond_precision_supported?

activerecord/test/cases/query_cache_test.rb

@@ -556,7 +556,7 @@ class QueryCacheTest < ActiveRecord::TestCase
   private
     def with_temporary_connection_pool
       old_pool = ActiveRecord::Base.connection_handler.retrieve_connection_pool(ActiveRecord::Base.connection_specification_name)
-      new_pool = ActiveRecord::ConnectionAdapters::ConnectionPool.new ActiveRecord::Base.connection_pool.spec
+      new_pool = ActiveRecord::ConnectionAdapters::ConnectionPool.new(ActiveRecord::Base.connection_pool.db_config)
       ActiveRecord::Base.connection_handler.send(:owner_to_pool)["primary"] = new_pool
 
       yield

activerecord/test/cases/reaper_test.rb

@@ -60,7 +60,7 @@ module ActiveRecord
       def test_pool_has_reaper
         config = ActiveRecord::Base.configurations.configs_for(env_name: "arunit", spec_name: "primary")
         spec = ConnectionSpecification.new("primary", config)
-        pool = ConnectionPool.new spec
+        pool = ConnectionPool.new(spec.db_config)
 
         assert pool.reaper
       ensure
@@ -71,7 +71,7 @@ module ActiveRecord
         spec = duplicated_spec
         spec.db_config.configuration_hash[:reaping_frequency] = "10.01"
 
-        pool = ConnectionPool.new spec
+        pool = ConnectionPool.new(spec.db_config)
 
         assert_equal 10.01, pool.reaper.frequency
       ensure
@@ -82,7 +82,7 @@ module ActiveRecord
         spec = duplicated_spec
         spec.db_config.configuration_hash[:reaping_frequency] = "0.0001"
 
-        pool = ConnectionPool.new spec
+        pool = ConnectionPool.new(spec.db_config)
 
         conn, child = new_conn_in_thread(pool)
 
@@ -101,7 +101,7 @@ module ActiveRecord
         spec.db_config.configuration_hash[:reaping_frequency] = "0.0001"
 
         2.times do
-          pool = ConnectionPool.new spec
+          pool = ConnectionPool.new(spec.db_config)
 
           conn, child = new_conn_in_thread(pool)
 
@@ -120,7 +120,7 @@ module ActiveRecord
       # it would take on a discarded pool
       def test_reap_flush_on_discarded_pool
         spec = duplicated_spec
-        pool = ConnectionPool.new spec
+        pool = ConnectionPool.new(spec.db_config)
 
         pool.discard!
         pool.reap
@@ -131,11 +131,11 @@ module ActiveRecord
         spec = duplicated_spec
         spec.db_config.configuration_hash[:reaping_frequency] = "0.0001"
 
-        pool = ConnectionPool.new spec
+        pool = ConnectionPool.new(spec.db_config)
         pool.checkout
 
         pid = fork do
-          pool = ConnectionPool.new spec
+          pool = ConnectionPool.new(spec.db_config)
 
           conn, child = new_conn_in_thread(pool)
           child.terminate
@@ -174,7 +174,7 @@ module ActiveRecord
 
       private
         def duplicated_spec
-          old_config = ActiveRecord::Base.connection_pool.spec.db_config.configuration_hash
+          old_config = ActiveRecord::Base.connection_pool.db_config.configuration_hash
           db_config = ActiveRecord::DatabaseConfigurations::HashConfig.new("arunit", "primary", old_config.dup)
           ConnectionSpecification.new("primary", db_config)
         end

railties/test/application/rake/multi_dbs_test.rb

@@ -359,7 +359,7 @@ module ApplicationTests
         db_migrate_and_schema_dump_and_load "schema"
 
         app_file "db/seeds.rb", <<-RUBY
-          print Book.connection.pool.spec.db_config.database
+          print Book.connection.pool.db_config.database
         RUBY
 
         output = rails("db:seed")