Never connect to the database in define_attribute_methods initializer

Followup: rails#48743

After careful consideration, unless users have a schema cache dump loaded
and `check_schema_cache_dump_version = false`, we have no choice but
to arbitrate between resiliency and performance.

If we define attribute methods during boot, we allow them to be shared
between forked workers, and prevent the first requests to be slower.

However, by doing so we'd trigger a connection to the datase, which
if it's unresponsive could lead to workers not being able to restart
triggering a cascading failure.

Previously Rails used to be in some sort of middle-ground where
it would define attribute methods during boot if for some reason
it was already connected to the database at this point.

But this is now deemed undesirable, as an application initializer
inadvertantly establishing the database connection woudl lead to
a readically different behavior.

activerecord/lib/active_record/railtie.rb

@@ -186,20 +186,18 @@ class Railtie < Rails::Railtie # :nodoc:
         ActiveSupport.on_load(:active_record) do
           # In development and test we shouldn't eagerly define attribute methods because
           # db:test:prepare will trigger later and might change the schema.
-          if app.config.eager_load && !Rails.env.local?
+          #
+          # Additionally if `check_schema_cache_dump_version` is enabled (which is the default),
+          # loading the schema cache dump trigger a database connection to compare the schema
+          # versions.
+          # This means the attribute methods will be lazily defined whent the model is accessed,
+          # likely as part of the first few requests or jobs. This isn't good for performance
+          # but we unfortunately have to arbitrate between resiliency and performance, and chose
+          # resiliency.
+          if !check_schema_cache_dump_version && app.config.eager_load && !Rails.env.local?
             begin
               descendants.each do |model|
-                # If the schema cache doesn't have the columns for this model,
-                # we avoid calling `define_attribute_methods` as it would trigger a query.
-                #
-                # However if we're already connected to the database, it's too late so we might
-                # as well eagerly define the attributes and hope the database timeout is strict enough.
-                #
-                # Additionally if `check_schema_cache_dump_version` is enabled, we have to connect to the
-                # database anyway to load the schema cache dump, so we might as well do it during boot to
-                # save memory in pre-forking setups and avoid slowness during the first requests post deploy.
-                schema_reflection = model.connection_pool.schema_reflection
-                if check_schema_cache_dump_version || schema_reflection.cached?(model.table_name) || model.connected?
+                if model.connection_pool.schema_reflection.cached?(model.table_name)
                   model.define_attribute_methods
                 end
               end

railties/test/application/configuration_test.rb

@@ -406,7 +406,7 @@ class Post < ActiveRecord::Base
       assert_not_includes Post.instance_methods, :title
     end
 
-    test "does not eager load attribute methods in production when the schema cache is empty and the database not connected" do
+    test "does not eager load attribute methods in production when the schema cache is empty and and check_schema_cache_dump_version=false" do
       app_file "app/models/post.rb", <<-RUBY
         class Post < ActiveRecord::Base
         end
@@ -423,7 +423,7 @@ class Post < ActiveRecord::Base
       assert_not_includes (Post.instance_methods - ActiveRecord::Base.instance_methods), :title
     end
 
-    test "eager loads attribute methods in production when the schema cache is populated" do
+    test "eager loads attribute methods in production when the schema cache is populated and check_schema_cache_dump_version=false" do
       app_file "app/models/post.rb", <<-RUBY
         class Post < ActiveRecord::Base
         end
@@ -442,18 +442,19 @@ class Post < ActiveRecord::Base
       add_to_config <<-RUBY
         config.enable_reloading = false
         config.eager_load = true
+        config.active_record.check_schema_cache_dump_version = false
       RUBY
 
       app_file "config/initializers/schema_cache.rb", <<-RUBY
-        ActiveRecord::Base.connection.schema_cache.add("posts")
+      ActiveRecord::Base.connection.schema_cache.add("posts")
       RUBY
 
       app "production"
 
-      assert_includes Post.instance_methods, :title
+      assert_includes (Post.instance_methods - ActiveRecord::Base.instance_methods), :title
     end
 
-    test "does not attempt to eager load attribute methods for models that aren't connected" do
+    test "does not eager loads attribute methods in production when the schema cache is populated and check_schema_cache_dump_version=true" do
       app_file "app/models/post.rb", <<-RUBY
         class Post < ActiveRecord::Base
         end
@@ -472,17 +473,16 @@ class Post < ActiveRecord::Base
       add_to_config <<-RUBY
         config.enable_reloading = false
         config.eager_load = true
+        config.active_record.check_schema_cache_dump_version = true
       RUBY
 
-      app_file "app/models/comment.rb", <<-RUBY
-        class Comment < ActiveRecord::Base
-          establish_connection(adapter: "mysql2", database: "does_not_exist")
-        end
+      app_file "config/initializers/schema_cache.rb", <<-RUBY
+      ActiveRecord::Base.connection.schema_cache.add("posts")
       RUBY
 
-      assert_nothing_raised do
-        app "production"
-      end
+      app "production"
+
+      assert_not_includes (Post.instance_methods - ActiveRecord::Base.instance_methods), :title
     end
 
     test "application is always added to eager_load namespaces" do