Merge pull request #38235 from eileencodes/fix-advisory-lock

Move advisory lock to it's own connection

activerecord/CHANGELOG.md

@@ -1,3 +1,11 @@
+*   Store advisory locks on their own named connection.
+
+    Previously advisory locks were taken out against a connection when a migration started. This works fine in single database applications but doesn't work well when migrations need to open new connections which results in the lock getting dropped.
+
+    In order to fix this we are storing the advisory lock on a new connection with the connection specification name `AdisoryLockBase`. The caveat is that we need to maintain at least 2 connections to a database while migrations are running in order to do this.
+
+    *Eileen M. Uchitelle*, *John Crepezzi*
+
 *   Ensure `:reading` connections always raise if a write is attempted.
 
     Now Rails will raise an `ActiveRecord::ReadOnlyError` if any connection on the reading handler attempts to make a write. If your reading role needs to write you should name the role something other than `:reading`.
@@ -22,8 +30,13 @@
 
     *Patrick Rebsch*
 
+<<<<<<< HEAD
 
 ## Rails 6.0.2.1 (December 18, 2019) ##
+=======
+    Applications should use `configs_for`. `#default_hash` and `#[]` will be removed in 6.2.
+=======
+>>>>>>> 59d54b350d... Merge pull request #38235 from eileencodes/fix-advisory-lock
 
 *   No changes.
 

activerecord/lib/active_record.rb

@@ -35,6 +35,7 @@
 module ActiveRecord
   extend ActiveSupport::Autoload
 
+  autoload :AdvisoryLockBase
   autoload :Base
   autoload :Callbacks
   autoload :Core

activerecord/lib/active_record/advisory_lock_base.rb

@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+module ActiveRecord
+  # This class is used to create a connection that we can use for advisory
+  # locks. This will take out a "global" lock that can't be accidentally
+  # removed if a new connection is established during a migration.
+  class AdvisoryLockBase < ActiveRecord::Base # :nodoc:
+    self.abstract_class = true
+
+    self.connection_specification_name = "AdvisoryLockBase"
+
+    class << self
+      def _internal?
+        true
+      end
+    end
+  end
+end

activerecord/lib/active_record/migration.rb

@@ -1374,7 +1374,8 @@ def use_advisory_lock?
 
       def with_advisory_lock
         lock_id = generate_migrator_advisory_lock_id
-        connection = Base.connection
+        AdvisoryLockBase.establish_connection(ActiveRecord::Base.connection_db_config) unless AdvisoryLockBase.connected?
+        connection = AdvisoryLockBase.connection
         got_lock = connection.get_advisory_lock(lock_id)
         raise ConcurrentMigrationError unless got_lock
         load_migrated # reload schema_migrations to be sure it wasn't changed by another process before we got the lock

activerecord/test/cases/migration_test.rb

@@ -705,6 +705,17 @@ def migrate(x)
         "without an advisory lock, the Migrator should not make any changes, but it did."
     end
 
+    def test_with_advisory_lock_doesnt_release_closed_connections
+      migration = Class.new(ActiveRecord::Migration::Current).new
+      migrator = ActiveRecord::Migrator.new(:up, [migration], @schema_migration, 100)
+
+      silence_stream($stderr) do
+        migrator.send(:with_advisory_lock) do
+          ActiveRecord::Base.establish_connection :arunit
+        end
+      end
+    end
+
     def test_with_advisory_lock_raises_the_right_error_when_it_fails_to_release_lock
       migration = Class.new(ActiveRecord::Migration::Current).new
       migrator = ActiveRecord::Migrator.new(:up, [migration], @schema_migration, 100)
@@ -713,7 +724,7 @@ def test_with_advisory_lock_raises_the_right_error_when_it_fails_to_release_lock
       e = assert_raises(ActiveRecord::ConcurrentMigrationError) do
         silence_stream($stderr) do
           migrator.send(:with_advisory_lock) do
-            ActiveRecord::Base.connection.release_advisory_lock(lock_id)
+            ActiveRecord::AdvisoryLockBase.connection.release_advisory_lock(lock_id)
           end
         end
       end