diff --git a/lib/gitlab/background_migration.rb b/lib/gitlab/background_migration.rb index 3586c6a25600800d7bca4daed3ef75197224010f..b0741b1fba743594710f851114c2b5b54b608d4b 100644 --- a/lib/gitlab/background_migration.rb +++ b/lib/gitlab/background_migration.rb @@ -27,9 +27,7 @@ module Gitlab begin perform(migration_class, migration_args, retries: 3) if job.delete - rescue StandardError - next - rescue Exception + rescue Exception # rubocop:disable Lint/RescueException BackgroundMigrationWorker # enqueue this migration again .perform_async(migration_class, migration_args) @@ -40,25 +38,15 @@ module Gitlab end ## - # Performs a background migration. In case of `StandardError` being caught - # this will retry a migration up to three times. + # Performs a background migration. # # class_name - The name of the background migration class as defined in the # Gitlab::BackgroundMigration namespace. # # arguments - The arguments to pass to the background migration's "perform" # method. - def self.perform(class_name, arguments, retries: 0) + def self.perform(class_name, arguments) const_get(class_name).new.perform(*arguments) - rescue StandardError - if retries > 0 - Rails.logger.warn("Retrying background migration #{class_name} " \ - "with #{arguments}") - retries -= 1 - retry - else - raise - end end end end diff --git a/spec/lib/gitlab/background_migration_spec.rb b/spec/lib/gitlab/background_migration_spec.rb index 64f59c72cecb84273a1424d8d843fdddd3c45c4f..cfa59280139c718aaabf511314b7b8089aa231af 100644 --- a/spec/lib/gitlab/background_migration_spec.rb +++ b/spec/lib/gitlab/background_migration_spec.rb @@ -62,32 +62,14 @@ describe Gitlab::BackgroundMigration do allow(queue[1]).to receive(:delete).and_return(true) end - context 'when standard error is being raised' do - it 'recovers from an exception and retries the migration' do - expect(migration).to receive(:perform).with(10, 20) - .and_raise(StandardError, 'Migration error') - .exactly(4).times.ordered - expect(migration).to receive(:perform).with(20, 30) - .once.ordered - expect(Rails.logger).to receive(:warn) - .with(/Retrying background migration/).exactly(3).times - - described_class.steal('Foo') - end - end - - context 'when top level exception is being raised' do - it 'enqueues the migration again and reraises the error' do - allow(migration).to receive(:perform).with(10, 20) - .and_raise(Exception, 'Migration error').once + it 'enqueues the migration again and re-raises the error' do + allow(migration).to receive(:perform).with(10, 20) + .and_raise(Exception, 'Migration error').once - expect(BackgroundMigrationWorker).to receive(:perform_async) - .with('Foo', [10, 20]).once + expect(BackgroundMigrationWorker).to receive(:perform_async) + .with('Foo', [10, 20]).once - expect(Rails.logger).not_to receive(:warn) - expect { described_class.steal('Foo') } - .to raise_error(Exception) - end + expect { described_class.steal('Foo') }.to raise_error(Exception) end end end @@ -131,42 +113,10 @@ describe Gitlab::BackgroundMigration do stub_const("#{described_class.name}::Foo", migration) end - context 'when retries count is not specified' do - it 'performs a background migration' do - expect(migration).to receive(:perform).with(10, 20).once - - described_class.perform('Foo', [10, 20]) - end - end - - context 'when retries count is zero' do - it 'perform a background migration only once' do - expect(migration).to receive(:perform).with(10, 20) - .and_raise(StandardError).once + it 'performs a background migration' do + expect(migration).to receive(:perform).with(10, 20).once - expect { described_class.perform('Foo', [10, 20], retries: 0) } - .to raise_error(StandardError) - end - end - - context 'when retries count is one' do - it 'retries a background migration when needed' do - expect(migration).to receive(:perform).with(10, 20) - .and_raise(StandardError).twice - - expect { described_class.perform('Foo', [10, 20], retries: 1) } - .to raise_error(StandardError) - end - end - - context 'when retries count is larger than zero' do - it 'retries a background migration when needed' do - expect(migration).to receive(:perform).with(10, 20) - .and_raise(StandardError).exactly(4).times - - expect { described_class.perform('Foo', [10, 20], retries: 3) } - .to raise_error(StandardError) - end + described_class.perform('Foo', [10, 20]) end end end