Fix off-by-one error in background migration retries
This commit is contained in:
parent
7b146ab6c3
commit
af41bd41e9
2 changed files with 25 additions and 14 deletions
|
@ -48,12 +48,17 @@ module Gitlab
|
||||||
#
|
#
|
||||||
# arguments - The arguments to pass to the background migration's "perform"
|
# arguments - The arguments to pass to the background migration's "perform"
|
||||||
# method.
|
# method.
|
||||||
def self.perform(class_name, arguments, retries: 1)
|
def self.perform(class_name, arguments, retries: 0)
|
||||||
const_get(class_name).new.perform(*arguments)
|
const_get(class_name).new.perform(*arguments)
|
||||||
rescue => e
|
rescue StandardError
|
||||||
|
if retries > 0
|
||||||
Rails.logger.warn("Retrying background migration #{class_name} " \
|
Rails.logger.warn("Retrying background migration #{class_name} " \
|
||||||
"with #{arguments}")
|
"with #{arguments}")
|
||||||
(retries -= 1) > 0 ? retry : raise
|
retries -= 1
|
||||||
|
retry
|
||||||
|
else
|
||||||
|
raise
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -63,14 +63,10 @@ describe Gitlab::BackgroundMigration do
|
||||||
end
|
end
|
||||||
|
|
||||||
context 'when standard error is being raised' do
|
context 'when standard error is being raised' do
|
||||||
before do
|
|
||||||
allow(migration).to receive(:perform).with(10, 20)
|
|
||||||
.and_raise(StandardError, 'Migration error')
|
|
||||||
end
|
|
||||||
|
|
||||||
it 'recovers from an exception and retries the migration' do
|
it 'recovers from an exception and retries the migration' do
|
||||||
expect(migration).to receive(:perform).with(10, 20)
|
expect(migration).to receive(:perform).with(10, 20)
|
||||||
.exactly(3).times.ordered
|
.and_raise(StandardError, 'Migration error')
|
||||||
|
.exactly(4).times.ordered
|
||||||
expect(migration).to receive(:perform).with(20, 30)
|
expect(migration).to receive(:perform).with(20, 30)
|
||||||
.once.ordered
|
.once.ordered
|
||||||
expect(Rails.logger).to receive(:warn)
|
expect(Rails.logger).to receive(:warn)
|
||||||
|
@ -153,10 +149,20 @@ describe Gitlab::BackgroundMigration do
|
||||||
end
|
end
|
||||||
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
|
context 'when retries count is larger than zero' do
|
||||||
it 'retries a background migration when needed' do
|
it 'retries a background migration when needed' do
|
||||||
expect(migration).to receive(:perform).with(10, 20)
|
expect(migration).to receive(:perform).with(10, 20)
|
||||||
.and_raise(StandardError).exactly(3).times
|
.and_raise(StandardError).exactly(4).times
|
||||||
|
|
||||||
expect { described_class.perform('Foo', [10, 20], retries: 3) }
|
expect { described_class.perform('Foo', [10, 20], retries: 3) }
|
||||||
.to raise_error(StandardError)
|
.to raise_error(StandardError)
|
||||||
|
|
Loading…
Reference in a new issue