Commit 200f0d90 authored by Thong Kuah's avatar Thong Kuah

Fix PG::ObjectInUse error when dropping databases

This requires a few things to be true:

1. Database connections are opened during routes. This is true for the
   GitLab application. Note this is not allowed anymore in Rails 7.
2. Multiple databases are configured. With a single database, Rails has
   always accidentally closed the database connection for the main
   database.

This fix removes prior attempts to drop the connection(s)

Database connections opened in other initializers are still an issue but
Rails kindly closes them for us. Unfortunately Rails has an initializer,
set_routes_reloader_hook which is run _after_ `config.after_initialize`.
The sequence goes like this:

1. Various initializers run, including route loading.
1. `config.after_initialize` blocks run (called by
   Gitlab::Application.finisher_hook). One of the
   `config.after_initialize` blocks run is to close all database
   connections.
1. `Gitlab::Application.set_routes_reloader_hook` initializer runs. This
   loads routes again.

So this fix is simple: we simply call the same thing Rails to close all
database connections again.
parent 7a2ed1d3
......@@ -517,6 +517,14 @@ module Gitlab
end
end
# We run the contents of active_record.clear_active_connections again
# because we connect to database from routes
# https://github.com/rails/rails/blob/fdf840f69a2e33d78a9d40b91d9b7fddb76711e9/activerecord/lib/active_record/railtie.rb#L308
initializer :clear_active_connections_again, after: :set_routes_reloader_hook do
ActiveRecord::Base.clear_active_connections!
ActiveRecord::Base.flush_idle_connections!
end
# DO NOT PLACE ANY INITIALIZERS AFTER THIS.
config.after_initialize do
# on_master_start yields immediately in unclustered environments and runs
......
......@@ -233,14 +233,6 @@ namespace :gitlab do
# :nocov:
end
desc "Clear all connections"
task :clear_all_connections do
ActiveRecord::Base.clear_all_connections!
end
Rake::Task['db:test:purge'].enhance(['gitlab:db:clear_all_connections'])
Rake::Task['db:drop'].enhance(['gitlab:db:clear_all_connections'])
# During testing, db:test:load restores the database schema from scratch
# which does not include dynamic partitions. We cannot rely on application
# initializers here as the application can continue to run while
......
......@@ -20,14 +20,6 @@ RSpec.describe 'gitlab:db namespace rake task', :silence_stdout do
allow(Rake::Task['db:seed_fu']).to receive(:invoke).and_return(true)
end
describe 'clear_all_connections' do
it 'calls clear_all_connections!' do
expect(ActiveRecord::Base).to receive(:clear_all_connections!)
run_rake_task('gitlab:db:clear_all_connections')
end
end
describe 'mark_migration_complete' do
context 'with a single database' do
let(:main_model) { ActiveRecord::Base }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment