Commit de47cb58 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Make sure jobs are picked up after being split

The migration runner only picks up existing jobs that are failed with
attempts less than the max, or jobs that are stuck in pending or running
for an hour.

We keep the split jobs in the failed state so that they can be picked up
immediately.
parent ab4f0292
...@@ -69,7 +69,7 @@ module Gitlab ...@@ -69,7 +69,7 @@ module Gitlab
# In this case, we just lower the batch size so that future calls to this # In this case, we just lower the batch size so that future calls to this
# method could eventually split the job if it continues to fail. # method could eventually split the job if it continues to fail.
if midpoint >= max_value if midpoint >= max_value
update!(batch_size: new_batch_size, status: :pending) update!(batch_size: new_batch_size, attempts: 0)
else else
old_max_value = max_value old_max_value = max_value
...@@ -77,7 +77,6 @@ module Gitlab ...@@ -77,7 +77,6 @@ module Gitlab
batch_size: new_batch_size, batch_size: new_batch_size,
max_value: midpoint, max_value: midpoint,
attempts: 0, attempts: 0,
status: :pending,
started_at: nil, started_at: nil,
finished_at: nil, finished_at: nil,
metrics: {} metrics: {}
......
...@@ -126,40 +126,50 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d ...@@ -126,40 +126,50 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d
end end
describe '#split_and_retry!' do describe '#split_and_retry!' do
let!(:job) { create(:batched_background_migration_job, batch_size: 10, min_value: 6, max_value: 15, status: :failed) } let!(:job) { create(:batched_background_migration_job, batch_size: 10, min_value: 6, max_value: 15, status: :failed, attempts: 3) }
it 'splits the job into two and marks them as pending' do context 'when job can be split' do
allow_next_instance_of(Gitlab::BackgroundMigration::BatchingStrategies::PrimaryKeyBatchingStrategy) do |batch_class| before do
allow(batch_class).to receive(:next_batch).with(anything, anything, batch_min_value: 6, batch_size: 5).and_return([6, 10]) allow_next_instance_of(Gitlab::BackgroundMigration::BatchingStrategies::PrimaryKeyBatchingStrategy) do |batch_class|
end allow(batch_class).to receive(:next_batch).with(anything, anything, batch_min_value: 6, batch_size: 5).and_return([6, 10])
end
expect { job.split_and_retry! }.to change { described_class.count }.by(1) end
expect(job).to have_attributes( it 'sets the correct attributes' do
min_value: 6, expect { job.split_and_retry! }.to change { described_class.count }.by(1)
max_value: 10,
batch_size: 5, expect(job).to have_attributes(
status: 'pending', min_value: 6,
attempts: 0, max_value: 10,
started_at: nil, batch_size: 5,
finished_at: nil, status: 'failed',
metrics: {} attempts: 0,
) started_at: nil,
finished_at: nil,
new_job = described_class.last metrics: {}
)
expect(new_job).to have_attributes(
batched_background_migration_id: job.batched_background_migration_id, new_job = described_class.last
min_value: 11,
max_value: 15, expect(new_job).to have_attributes(
batch_size: 5, batched_background_migration_id: job.batched_background_migration_id,
status: 'pending', min_value: 11,
attempts: 0, max_value: 15,
started_at: nil, batch_size: 5,
finished_at: nil, status: 'failed',
metrics: {} attempts: 0,
) started_at: nil,
expect(new_job.created_at).not_to eq(job.created_at) finished_at: nil,
metrics: {}
)
expect(new_job.created_at).not_to eq(job.created_at)
end
it 'splits the jobs into retriable jobs' do
migration = job.batched_migration
expect { job.split_and_retry! }.to change { migration.batched_jobs.retriable.count }.from(0).to(2)
end
end end
context 'when job is not failed' do context 'when job is not failed' do
...@@ -185,11 +195,12 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d ...@@ -185,11 +195,12 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d
end end
end end
it 'lowers the batch size and marks the job as pending' do it 'lowers the batch size and resets the number of attempts' do
expect { job.split_and_retry! }.not_to change { described_class.count } expect { job.split_and_retry! }.not_to change { described_class.count }
expect(job.batch_size).to eq(5) expect(job.batch_size).to eq(5)
expect(job.status).to eq('pending') expect(job.attempts).to eq(0)
expect(job.status).to eq('failed')
end end
end end
end end
......
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