From 898b63d6aa274ef7d472e391329b366d8f057908 Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Fri, 2 Feb 2018 10:11:30 -0500 Subject: [PATCH 1/4] Check for restore error cases in EvmDatabaseOps This allows us to do more of the processing using existing code rather than the current implementation which does the checks in different places/repos and does a lot of shelling out to psql rather than using a real database connection. This also makes the error cases more explicit which fixes an issue where a user could not restore a pg_dump backup when pglogical was running. https://bugzilla.redhat.com/show_bug.cgi?id=1540686 --- lib/evm_database_ops.rb | 42 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/lib/evm_database_ops.rb b/lib/evm_database_ops.rb index da62397ce2e..cf669dc6236 100644 --- a/lib/evm_database_ops.rb +++ b/lib/evm_database_ops.rb @@ -92,6 +92,8 @@ def self.restore(db_opts, connect_opts = {}) db_opts[:local_file] = session.uri_to_local_path(uri) end + prepare_for_restore(db_opts[:local_file]) + backup = PostgresAdmin.restore(db_opts) ensure session.disconnect if session @@ -102,6 +104,46 @@ def self.restore(db_opts, connect_opts = {}) uri end + private_class_method def self.prepare_for_restore(filename) + backup_type = validate_backup_file_type(filename) + + if application_connections? + message = "Database restore failed. Shut down all evmserverd processes before attempting a database restore" + _log.error(message) + raise message + end + + MiqRegion.replication_type = :none + 60.times do + break if VmdbDatabaseConnection.where("application_name LIKE 'pglogical manager%'").count.zero? + _log.info("Waiting for pglogical connections to close...") + sleep 5 + end + + connection_count = backup_type == :basebackup ? VmdbDatabaseConnection.unscoped.count : VmdbDatabaseConnection.count + if connection_count > 1 + message = "Database restore failed. #{connection_count - 1} connections remain to the database." + _log.error(message) + raise message + end + end + + private_class_method def self.validate_backup_file_type(filename) + if PostgresAdmin.base_backup_file?(filename) + :basebackup + elsif PostgresAdmin.pg_dump_file?(filename) + :pgdump + else + message = "#{filename} is not in a recognized database backup format" + _log.error(message) + raise message + end + end + + private_class_method def self.application_connections? + VmdbDatabaseConnection.all.map(&:application_name).any? { |app_name| app_name.start_with?("MIQ") } + end + def self.gc(options = {}) PostgresAdmin.gc(options) end From 4d7af028694bc612b5d66f55bc65781b71329bac Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Fri, 2 Feb 2018 13:59:42 -0500 Subject: [PATCH 2/4] Add :environment as a prerequisate rake task for restore We need the rails environment to access the VmdbDatabaseConnection model --- lib/tasks/evm_dba.rake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/tasks/evm_dba.rake b/lib/tasks/evm_dba.rake index 0830c04026e..f7c1ebe4bd2 100644 --- a/lib/tasks/evm_dba.rake +++ b/lib/tasks/evm_dba.rake @@ -194,7 +194,7 @@ namespace :evm do namespace :restore do desc 'Restore the local ManageIQ EVM Database (VMDB) from a local backup file' - task :local do + task :local => :environment do require 'trollop' opts = Trollop.options(EvmRakeHelper.extract_command_options) do opt :local_file, "Destination file", :type => :string, :required => true @@ -215,7 +215,7 @@ namespace :evm do end desc 'Restore the local ManageIQ EVM Database (VMDB) from a remote backup file' - task :remote do + task :remote => :environment do require 'trollop' opts = Trollop.options(EvmRakeHelper.extract_command_options) do opt :uri, "Destination depot URI", :type => :string, :required => true From e574ad35a789eed5c16faa37b92e1fa67601fbe6 Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Fri, 2 Feb 2018 14:00:35 -0500 Subject: [PATCH 3/4] Do the restore without any active connections to the database To do this, we need to disconnect the connection we created when requiring the :environment rake task to run this method. --- lib/evm_database_ops.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/evm_database_ops.rb b/lib/evm_database_ops.rb index cf669dc6236..71badce9585 100644 --- a/lib/evm_database_ops.rb +++ b/lib/evm_database_ops.rb @@ -94,12 +94,14 @@ def self.restore(db_opts, connect_opts = {}) prepare_for_restore(db_opts[:local_file]) - backup = PostgresAdmin.restore(db_opts) + # remove all the connections before we restore; AR will reconnect on the next query + ActiveRecord::Base.connection_pool.disconnect! + backup_file = PostgresAdmin.restore(db_opts) ensure session.disconnect if session end - uri ||= backup + uri ||= backup_file _log.info("[#{db_opts[:dbname]}] database has been restored from file: [#{uri}]") uri end From 1876da320e1b54a12240f0c27ec5bc1d1c121c50 Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Fri, 9 Feb 2018 15:05:45 -0500 Subject: [PATCH 4/4] Fix specs after adding check for file type --- spec/lib/evm_database_ops_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/lib/evm_database_ops_spec.rb b/spec/lib/evm_database_ops_spec.rb index 70cfd32b543..a28cc07c92b 100644 --- a/spec/lib/evm_database_ops_spec.rb +++ b/spec/lib/evm_database_ops_spec.rb @@ -55,6 +55,7 @@ allow_any_instance_of(MiqSmbSession).to receive(:settings_mount_point).and_return(Rails.root.join("tmp")) allow(PostgresAdmin).to receive(:runcmd_with_logging) allow(PostgresAdmin).to receive(:pg_dump_file?).and_return(true) + allow(PostgresAdmin).to receive(:base_backup_file?).and_return(false) end it "from local backup" do