Remove extra advisory lock connection

In #38235 I moved advisory locks to their own named connections, then
in #39758 the advisory lock was left on Base.connection but then moved
it it's own connection handler. I believe with #45450 that this change
was made obsolete and can be returned to the prior behavior without
having to open an additional connection. The tests added pass and I also
tested this in my local demo to ensure that this is working correctly.

When I originally changed the behavior here Matthew noted that this
could be surprising for some setups that expect only one connection for
a running migration. I thought there was an issue related to this but I
can't find it.
This commit is contained in:
eileencodes 2022-10-11 11:22:00 -04:00
parent 69296dd716
commit 07d8b99ebe
No known key found for this signature in database
GPG Key ID: BA5C575120BBE8DF
2 changed files with 15 additions and 25 deletions

@ -1466,29 +1466,18 @@ def use_advisory_lock?
def with_advisory_lock def with_advisory_lock
lock_id = generate_migrator_advisory_lock_id lock_id = generate_migrator_advisory_lock_id
connection = ActiveRecord::Base.connection
with_advisory_lock_connection do |connection| got_lock = connection.get_advisory_lock(lock_id)
got_lock = connection.get_advisory_lock(lock_id) raise ConcurrentMigrationError unless got_lock
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
load_migrated # reload schema_migrations to be sure it wasn't changed by another process before we got the lock yield
yield
ensure
if got_lock && !connection.release_advisory_lock(lock_id)
raise ConcurrentMigrationError.new(
ConcurrentMigrationError::RELEASE_LOCK_FAILED_MESSAGE
)
end
end
end
def with_advisory_lock_connection(&block)
pool = ActiveRecord::ConnectionAdapters::ConnectionHandler.new.establish_connection(
ActiveRecord::Base.connection_db_config
)
pool.with_connection(&block)
ensure ensure
pool&.disconnect! if got_lock && !connection.release_advisory_lock(lock_id)
raise ConcurrentMigrationError.new(
ConcurrentMigrationError::RELEASE_LOCK_FAILED_MESSAGE
)
end
end end
MIGRATOR_SALT = 2053462845 MIGRATOR_SALT = 2053462845

@ -1095,6 +1095,9 @@ def migrate(x)
end end
def test_with_advisory_lock_doesnt_release_closed_connections def test_with_advisory_lock_doesnt_release_closed_connections
# make sure we have a connection
ActiveRecord::Base.establish_connection :arunit
migration = Class.new(ActiveRecord::Migration::Current).new migration = Class.new(ActiveRecord::Migration::Current).new
migrator = ActiveRecord::Migrator.new(:up, [migration], @schema_migration, @internal_metadata, 100) migrator = ActiveRecord::Migrator.new(:up, [migration], @schema_migration, @internal_metadata, 100)
@ -1137,10 +1140,8 @@ def test_with_advisory_lock_raises_the_right_error_when_it_fails_to_release_lock
e = assert_raises(ActiveRecord::ConcurrentMigrationError) do e = assert_raises(ActiveRecord::ConcurrentMigrationError) do
silence_stream($stderr) do silence_stream($stderr) do
migrator.stub(:with_advisory_lock_connection, ->(&block) { block.call(ActiveRecord::Base.connection) }) do migrator.send(:with_advisory_lock) do
migrator.send(:with_advisory_lock) do ActiveRecord::Base.connection.release_advisory_lock(lock_id)
ActiveRecord::Base.connection.release_advisory_lock(lock_id)
end
end end
end end
end end