From 1967459c5e63d3aeb169dd85367670a93f09681e Mon Sep 17 00:00:00 2001 From: Richie Thomas Date: Tue, 14 Jan 2020 17:28:32 -0800 Subject: [PATCH] Replace automatic parallelization of backup jobs with opt-in --- README.md | 6 +++ lib/parity/backup.rb | 9 ++-- lib/parity/environment.rb | 10 +++-- spec/parity/backup_spec.rb | 77 +++++++++++++++++++++++++++++++-- spec/parity/environment_spec.rb | 42 ++++++++++++++++-- 5 files changed, 131 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 448497c..0230429 100644 --- a/README.md +++ b/README.md @@ -104,6 +104,12 @@ with `heroku ______ --remote staging` or `heroku ______ --remote production`: watch production ps staging open +You can optionally parallelize a DB restore by passing `--parallelize` +as a flag to the `development` or `production` commands: +``` + development restore-from production --parallelize +``` + [2]: http://redis.io/commands Convention diff --git a/lib/parity/backup.rb b/lib/parity/backup.rb index c4cf45a..45c870b 100644 --- a/lib/parity/backup.rb +++ b/lib/parity/backup.rb @@ -10,6 +10,7 @@ class Backup def initialize(args) @from, @to = args.values_at(:from, :to) @additional_args = args[:additional_args] || BLANK_ARGUMENTS + @parallelize = args[:parallelize] || false end def restore @@ -24,7 +25,9 @@ def restore private - attr_reader :additional_args, :from, :to + attr_reader :additional_args, :from, :to, :parallelize + + alias :parallelize? :parallelize def restore_from_development reset_remote_database @@ -115,10 +118,10 @@ def database_yaml_file end def processor_cores - if ruby_version_over_2_2? + if parallelize? && ruby_version_over_2_2? Etc.nprocessors else - 2 + 1 end end diff --git a/lib/parity/environment.rb b/lib/parity/environment.rb index 237bdf2..15f72a2 100644 --- a/lib/parity/environment.rb +++ b/lib/parity/environment.rb @@ -57,6 +57,7 @@ def restore Backup.new( from: arguments.first, to: environment, + parallelize: parallelize?, additional_args: additional_restore_arguments, ).restore end @@ -72,10 +73,13 @@ def forced? arguments.include?("--force") end + def parallelize? + arguments.include?("--parallelize") + end + def additional_restore_arguments - (arguments.drop(1) - ["--force"] + [restore_confirmation_argument]). - compact. - join(" ") + (arguments.drop(1) - ["--force", "--parallelize"] + + [restore_confirmation_argument]).compact.join(" ") end def restore_confirmation_argument diff --git a/spec/parity/backup_spec.rb b/spec/parity/backup_spec.rb index 7353f39..475bfdd 100644 --- a/spec/parity/backup_spec.rb +++ b/spec/parity/backup_spec.rb @@ -7,7 +7,11 @@ allow(Kernel).to receive(:system) allow(Etc).to receive(:nprocessors).and_return(number_of_processes) - Parity::Backup.new(from: "production", to: "development").restore + Parity::Backup.new( + from: "production", + to: "development", + parallelize: true, + ).restore expect(Kernel). to have_received(:system). @@ -31,7 +35,11 @@ allow(Kernel).to receive(:system) allow(Etc).to receive(:respond_to?).with(:nprocessors).and_return(false) - Parity::Backup.new(from: "production", to: "development").restore + Parity::Backup.new( + from: "production", + to: "development", + parallelize: false, + ).restore expect(Kernel). to have_received(:system). @@ -44,7 +52,64 @@ with(drop_development_database_drop_command) expect(Kernel). to have_received(:system). - with(restore_from_local_temp_backup_command(cores: 2)) + with(restore_from_local_temp_backup_command(cores: 1)) + expect(Kernel). + to have_received(:system). + with(delete_local_temp_backup_command) + end + + it "restores backups in parallel when the right flag is set" do + allow(IO).to receive(:read).and_return(database_fixture) + allow(Kernel).to receive(:system) + allow(Etc).to receive(:nprocessors).and_return(12) + + Parity::Backup.new( + from: "production", + to: "development", + parallelize: true, + ).restore + + expect(Kernel). + to have_received(:system). + with(make_temp_directory_command) + expect(Kernel). + to have_received(:system). + with(download_remote_database_command) + expect(Kernel). + to have_received(:system). + with(drop_development_database_drop_command) + expect(Kernel). + to have_received(:system). + with(restore_from_local_temp_backup_command(cores: 12)) + expect(Kernel). + to have_received(:system). + with(delete_local_temp_backup_command) + end + + it "does not restore backups in parallel when the right flag is set" + + "but the ruby version is under 2.2" do + allow(IO).to receive(:read).and_return(database_fixture) + allow(Kernel).to receive(:system) + allow(Etc).to receive(:respond_to?).with(:nprocessors).and_return(false) + + Parity::Backup.new( + from: "production", + to: "development", + parallelize: true, + ).restore + + expect(Kernel). + to have_received(:system). + with(make_temp_directory_command) + expect(Kernel). + to have_received(:system). + with(download_remote_database_command) + expect(Kernel). + to have_received(:system). + with(drop_development_database_drop_command) + expect(Kernel). + to have_received(:system). + with(restore_from_local_temp_backup_command(cores: 1)) expect(Kernel). to have_received(:system). with(delete_local_temp_backup_command) @@ -55,7 +120,11 @@ allow(Kernel).to receive(:system) allow(Etc).to receive(:nprocessors).and_return(number_of_processes) - Parity::Backup.new(from: "production", to: "development").restore + Parity::Backup.new( + from: "production", + to: "development", + parallelize: true, + ).restore expect(Kernel). to have_received(:system). diff --git a/spec/parity/environment_spec.rb b/spec/parity/environment_spec.rb index 5351fe2..f9b7a56 100644 --- a/spec/parity/environment_spec.rb +++ b/spec/parity/environment_spec.rb @@ -6,6 +6,23 @@ allow(Kernel).to receive(:system).and_return(true) end + it "restores in parallel when passed the --parallelize flag" do + backup = stub_parity_backup + allow(Parity::Backup).to receive(:new).and_return(backup) + + Parity::Environment.new("development", + ["restore", "staging", "--parallelize"]).run + + expect(Parity::Backup).to have_received(:new). + with( + from: "staging", + to: "development", + parallelize: true, + additional_args: "", + ) + expect(backup).to have_received(:restore) + end + it "passes through arguments with correct quoting" do Parity::Environment.new( "production", @@ -54,6 +71,7 @@ with( from: "production", to: "staging", + parallelize: false, additional_args: "--confirm parity-integration-staging", ) expect(backup).to have_received(:restore) @@ -71,6 +89,7 @@ with( from: "production", to: "staging", + parallelize: false, additional_args: "--confirm parity-staging", ) expect(backup).to have_received(:restore) @@ -88,6 +107,7 @@ with( from: "production", to: "staging", + parallelize: false, additional_args: "--confirm parity-staging", ) expect(backup).to have_received(:restore) @@ -104,6 +124,7 @@ with( from: "production", to: "staging", + parallelize: false, additional_args: "--confirm parity-staging", ) expect(backup).to have_received(:restore) @@ -116,7 +137,12 @@ Parity::Environment.new("development", ["restore", "production"]).run expect(Parity::Backup).to have_received(:new). - with(from: "production", to: "development", additional_args: "") + with( + from: "production", + to: "development", + parallelize: false, + additional_args: "", + ) expect(backup).to have_received(:restore) end @@ -127,7 +153,12 @@ Parity::Environment.new("development", ["restore", "staging"]).run expect(Parity::Backup).to have_received(:new). - with(from: "staging", to: "development", additional_args: "") + with( + from: "staging", + to: "development", + parallelize: false, + additional_args: "", + ) expect(backup).to have_received(:restore) end @@ -152,7 +183,12 @@ Parity::Environment.new("production", ["restore", "staging", "--force"]).run expect(Parity::Backup).to have_received(:new). - with(from: "staging", to: "production", additional_args: "") + with( + from: "staging", + to: "production", + parallelize: false, + additional_args: "", + ) expect(backup).to have_received(:restore) end