Fix migration ordering across databases

Previously if there were 2 migrations in one db and 1 migration in the
other db all the migrations for db one would run and then all migrations
for db two would run. If a migration in one database depended on a
migration in another database then it could fail. This is probably
pretty rare, however in a multi-db application that's moving tables from
one db to another, running them out of order could result in a migration
error.

In this this change we collect all the versions for each migration and
the corresponding db_config so we can run them in the order they are
created rather than per-db.

Closes rails#41664
Related rails#41538

Co-authored-by: John Crepezzi <[email protected]>
Co-authored-by: Kiril Dokh <[email protected]>

Author: 3 people

activerecord/lib/active_record/migration.rb

@@ -1140,7 +1140,11 @@ def current_version # :nodoc:
     end
 
     def needs_migration? # :nodoc:
-      (migrations.collect(&:version) - get_all_versions).size > 0
+      pending_migration_versions.size > 0
+    end
+
+    def pending_migration_versions # :nodoc:
+      migrations.collect(&:version) - get_all_versions
     end
 
     def migrations # :nodoc:

activerecord/lib/active_record/railties/databases.rake

@@ -86,14 +86,23 @@ db_namespace = namespace :db do
 
   desc "Migrate the database (options: VERSION=x, VERBOSE=false, SCOPE=blog)."
   task migrate: :load_config do
-    original_db_config = ActiveRecord::Base.connection_db_config
-    ActiveRecord::Base.configurations.configs_for(env_name: ActiveRecord::Tasks::DatabaseTasks.env).each do |db_config|
-      ActiveRecord::Base.establish_connection(db_config)
+    db_configs = ActiveRecord::Base.configurations.configs_for(env_name: ActiveRecord::Tasks::DatabaseTasks.env)
+
+    if db_configs.size == 1
       ActiveRecord::Tasks::DatabaseTasks.migrate
+    else
+      original_db_config = ActiveRecord::Base.connection_db_config
+      mapped_versions = ActiveRecord::Tasks::DatabaseTasks.db_configs_with_versions(db_configs)
+
+      mapped_versions.sort.each do |version, db_config|
+        ActiveRecord::Base.establish_connection(db_config)
+        ActiveRecord::Tasks::DatabaseTasks.migrate(version)
+      end
     end
+
     db_namespace["_dump"].invoke
   ensure
-    ActiveRecord::Base.establish_connection(original_db_config)
+    ActiveRecord::Base.establish_connection(original_db_config) if original_db_config
   end
 
   # IMPORTANT: This task won't dump the schema if ActiveRecord.dump_schema_after_migration is set to false

activerecord/lib/active_record/tasks/database_tasks.rb

@@ -268,13 +268,13 @@ def truncate_all(environment = env)
         end
       end
 
-      def migrate
+      def migrate(version = nil)
         check_target_version
 
         scope = ENV["SCOPE"]
         verbose_was, Migration.verbose = Migration.verbose, verbose?
 
-        Base.connection.migration_context.migrate(target_version) do |migration|
+        Base.connection.migration_context.migrate(target_version || version) do |migration|
           scope.blank? || scope == migration.scope
         end.tap do |migrations_ran|
           Migration.write("No migrations ran. (using #{scope} scope)") if scope.present? && migrations_ran.empty?
@@ -285,6 +285,23 @@ def migrate
         Migration.verbose = verbose_was
       end
 
+      def db_configs_with_versions(db_configs) # :nodoc:
+        db_configs_with_versions = {}
+
+        db_configs.each do |db_config|
+          ActiveRecord::Base.establish_connection(db_config)
+          versions_to_run = ActiveRecord::Base.connection.migration_context.pending_migration_versions
+          target_version = ActiveRecord::Tasks::DatabaseTasks.target_version
+
+          versions_to_run.each do |version|
+            next if target_version && target_version != version
+            db_configs_with_versions[version] = db_config
+          end
+        end
+
+        db_configs_with_versions
+      end
+
       def migrate_status
         unless ActiveRecord::Base.connection.schema_migration.table_exists?
           Kernel.abort "Schema migrations table does not exist yet."

railties/test/application/rake/multi_dbs_test.rb

@@ -427,6 +427,30 @@ def generate_models_for_animals
         end
       end
 
+      test "db:migrate respects timestamp ordering across databases" do
+        require "#{app_path}/config/environment"
+        app_file "db/migrate/01_one_migration.rb", <<-MIGRATION
+          class OneMigration < ActiveRecord::Migration::Current
+          end
+        MIGRATION
+
+        app_file "db/animals_migrate/02_two_migration.rb", <<-MIGRATION
+          class TwoMigration < ActiveRecord::Migration::Current
+          end
+        MIGRATION
+
+        app_file "db/migrate/03_three_migration.rb", <<-MIGRATION
+          class ThreeMigration < ActiveRecord::Migration::Current
+          end
+        MIGRATION
+
+        Dir.chdir(app_path) do
+          output = rails "db:migrate"
+          entries = output.scan(/^== (\d+).+migrated/).map(&:first).map(&:to_i)
+          assert_equal [1, 2, 3], entries
+        end
+      end
+
       test "db:migrate and db:schema:dump and db:schema:load works on all databases" do
         db_migrate_and_schema_dump_and_load
       end