Commit b511f383 authored by Thong Kuah's avatar Thong Kuah

Merge branch 'close_database_connections_again_after_set_routes_reloader_hook' into 'master'

Better fix for PG::ObjectInUse error when dropping databases

See merge request gitlab-org/gitlab!85006
parents 24517a0a 7f00d2cc
...@@ -517,6 +517,14 @@ module Gitlab ...@@ -517,6 +517,14 @@ module Gitlab
end end
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. # DO NOT PLACE ANY INITIALIZERS AFTER THIS.
config.after_initialize do config.after_initialize do
# on_master_start yields immediately in unclustered environments and runs # on_master_start yields immediately in unclustered environments and runs
......
...@@ -233,14 +233,6 @@ namespace :gitlab do ...@@ -233,14 +233,6 @@ namespace :gitlab do
# :nocov: # :nocov:
end 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 # During testing, db:test:load restores the database schema from scratch
# which does not include dynamic partitions. We cannot rely on application # which does not include dynamic partitions. We cannot rely on application
# initializers here as the application can continue to run while # initializers here as the application can continue to run while
......
...@@ -6,7 +6,7 @@ namespace :gitlab do ...@@ -6,7 +6,7 @@ namespace :gitlab do
namespace :db do namespace :db do
desc 'Validates `config/database.yml` to ensure a correct behavior is configured' desc 'Validates `config/database.yml` to ensure a correct behavior is configured'
task validate_config: :environment do task validate_config: :environment do
should_reconnect = ActiveRecord::Base.connection_pool.active_connection? original_db_config = ActiveRecord::Base.connection_db_config
# The include_replicas: is a legacy name to fetch all hidden entries (replica: true or database_tasks: false) # The include_replicas: is a legacy name to fetch all hidden entries (replica: true or database_tasks: false)
# Once we upgrade to Rails 7.x this should be changed to `include_hidden: true` # Once we upgrade to Rails 7.x this should be changed to `include_hidden: true`
...@@ -97,9 +97,7 @@ namespace :gitlab do ...@@ -97,9 +97,7 @@ namespace :gitlab do
end end
ensure ensure
if should_reconnect ActiveRecord::Base.establish_connection(original_db_config) # rubocop: disable Database/EstablishConnection
ActiveRecord::Base.establish_connection(ActiveRecord::Tasks::DatabaseTasks.env.to_sym) # rubocop: disable Database/EstablishConnection
end
end end
Rake::Task['db:migrate'].enhance(['gitlab:db:validate_config']) Rake::Task['db:migrate'].enhance(['gitlab:db:validate_config'])
......
...@@ -46,6 +46,12 @@ RSpec.describe 'gitlab:db:validate_config', :silence_stdout do ...@@ -46,6 +46,12 @@ RSpec.describe 'gitlab:db:validate_config', :silence_stdout do
expect { run_rake_task('gitlab:db:validate_config') }.not_to raise_error expect { run_rake_task('gitlab:db:validate_config') }.not_to raise_error
end end
it 'always re-establishes ActiveRecord::Base connection to main config' do
run_rake_task('gitlab:db:validate_config')
expect(ActiveRecord::Base.connection_db_config.configuration_hash).to include(main_database_config) # rubocop: disable Database/MultipleDatabases
end
it 'if GITLAB_VALIDATE_DATABASE_CONFIG is set' do it 'if GITLAB_VALIDATE_DATABASE_CONFIG is set' do
stub_env('GITLAB_VALIDATE_DATABASE_CONFIG', '1') stub_env('GITLAB_VALIDATE_DATABASE_CONFIG', '1')
allow(Gitlab).to receive(:dev_or_test_env?).and_return(false) allow(Gitlab).to receive(:dev_or_test_env?).and_return(false)
...@@ -78,6 +84,12 @@ RSpec.describe 'gitlab:db:validate_config', :silence_stdout do ...@@ -78,6 +84,12 @@ RSpec.describe 'gitlab:db:validate_config', :silence_stdout do
expect { run_rake_task('gitlab:db:validate_config') }.to raise_error(match) expect { run_rake_task('gitlab:db:validate_config') }.to raise_error(match)
end end
it 'always re-establishes ActiveRecord::Base connection to main config' do
expect { run_rake_task('gitlab:db:validate_config') }.to raise_error(match)
expect(ActiveRecord::Base.connection_db_config.configuration_hash).to include(main_database_config) # rubocop: disable Database/MultipleDatabases
end
it 'if GITLAB_VALIDATE_DATABASE_CONFIG=1' do it 'if GITLAB_VALIDATE_DATABASE_CONFIG=1' do
stub_env('GITLAB_VALIDATE_DATABASE_CONFIG', '1') stub_env('GITLAB_VALIDATE_DATABASE_CONFIG', '1')
......
...@@ -20,14 +20,6 @@ RSpec.describe 'gitlab:db namespace rake task', :silence_stdout do ...@@ -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) allow(Rake::Task['db:seed_fu']).to receive(:invoke).and_return(true)
end 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 describe 'mark_migration_complete' do
context 'with a single database' do context 'with a single database' do
let(:main_model) { ActiveRecord::Base } 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