Commit d204bdd2 authored by dfrazao-gitlab's avatar dfrazao-gitlab

Log transitions and errors for batched jobs

- Start using state machine
- Log transitions
- Log exceptions
- Record transitions + exceptions

Changelog: added

Relates to https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75110
parent 84ff263c
...@@ -12,23 +12,58 @@ module Gitlab ...@@ -12,23 +12,58 @@ module Gitlab
MAX_ATTEMPTS = 3 MAX_ATTEMPTS = 3
STUCK_JOBS_TIMEOUT = 1.hour.freeze STUCK_JOBS_TIMEOUT = 1.hour.freeze
enum status: {
pending: 0,
running: 1,
failed: 2,
succeeded: 3
}
belongs_to :batched_migration, foreign_key: :batched_background_migration_id belongs_to :batched_migration, foreign_key: :batched_background_migration_id
has_many :batched_job_transition_logs, foreign_key: :batched_background_migration_job_id has_many :batched_job_transition_logs, foreign_key: :batched_background_migration_job_id
scope :active, -> { where(status: [:pending, :running]) } scope :active, -> { with_statuses(:pending, :running) }
scope :stuck, -> { active.where('updated_at <= ?', STUCK_JOBS_TIMEOUT.ago) } scope :stuck, -> { active.where('updated_at <= ?', STUCK_JOBS_TIMEOUT.ago) }
scope :retriable, -> { from_union([failed.where('attempts < ?', MAX_ATTEMPTS), self.stuck]) } scope :retriable, -> { from_union([with_status(:failed).where('attempts < ?', MAX_ATTEMPTS), self.stuck]) }
scope :except_succeeded, -> { where(status: self.statuses.except(:succeeded).values) } scope :except_succeeded, -> { without_status(:succeeded) }
scope :successful_in_execution_order, -> { where.not(finished_at: nil).succeeded.order(:finished_at) } scope :successful_in_execution_order, -> { where.not(finished_at: nil).with_status(:succeeded).order(:finished_at) }
scope :with_preloads, -> { preload(:batched_migration) } scope :with_preloads, -> { preload(:batched_migration) }
state_machine :status, initial: :pending do
state :pending, value: 0
state :running, value: 1
state :failed, value: 2
state :succeeded, value: 3
event :succeed do
transition any => :succeeded
end
event :failure do
transition any => :failed
end
event :run do
transition any => :running
end
before_transition any => [:failed, :succeeded] do |job|
job.finished_at = Time.current
end
before_transition any => :running do |job|
job.attempts += 1
job.started_at = Time.current
job.finished_at = nil
job.metrics = {}
end
after_transition do |job, transition|
error_hash = transition.args.find { |arg| arg[:error].present? }
exception = error_hash&.fetch(:error)
job.batched_job_transition_logs.create(previous_status: transition.from, next_status: transition.to, exception_class: exception&.class, exception_message: exception&.message)
Gitlab::ErrorTracking.track_exception(exception, batched_job_id: job.id) if exception
Gitlab::AppLogger.info(message: 'BatchedJob transition', batched_job_id: job.id, previous_state: transition.from_name, new_state: transition.to_name)
end
end
delegate :job_class, :table_name, :column_name, :job_arguments, delegate :job_class, :table_name, :column_name, :job_arguments,
to: :batched_migration, prefix: :migration to: :batched_migration, prefix: :migration
......
...@@ -8,6 +8,8 @@ module Gitlab ...@@ -8,6 +8,8 @@ module Gitlab
self.table_name = :batched_background_migration_job_transition_logs self.table_name = :batched_background_migration_job_transition_logs
self.primary_key = :id
partitioned_by :created_at, strategy: :monthly, retain_for: 6.months partitioned_by :created_at, strategy: :monthly, retain_for: 6.months
belongs_to :batched_job, foreign_key: :batched_background_migration_job_id belongs_to :batched_job, foreign_key: :batched_background_migration_job_id
...@@ -17,8 +19,8 @@ module Gitlab ...@@ -17,8 +19,8 @@ module Gitlab
validates :exception_class, length: { maximum: 100 } validates :exception_class, length: { maximum: 100 }
validates :exception_message, length: { maximum: 1000 } validates :exception_message, length: { maximum: 1000 }
enum previous_status: BatchedJob.statuses, _prefix: true enum previous_status: Gitlab::Database::BackgroundMigration::BatchedJob.state_machine.states.map(&:name), _prefix: true
enum next_status: BatchedJob.statuses, _prefix: true enum next_status: Gitlab::Database::BackgroundMigration::BatchedJob.state_machine.states.map(&:name), _prefix: true
end end
end end
end end
......
...@@ -47,7 +47,7 @@ module Gitlab ...@@ -47,7 +47,7 @@ module Gitlab
def self.successful_rows_counts(migrations) def self.successful_rows_counts(migrations)
BatchedJob BatchedJob
.succeeded .with_status(:succeeded)
.where(batched_background_migration_id: migrations) .where(batched_background_migration_id: migrations)
.group(:batched_background_migration_id) .group(:batched_background_migration_id)
.sum(:batch_size) .sum(:batch_size)
...@@ -71,7 +71,7 @@ module Gitlab ...@@ -71,7 +71,7 @@ module Gitlab
end end
def retry_failed_jobs! def retry_failed_jobs!
batched_jobs.failed.each_batch(of: 100) do |batch| batched_jobs.with_status(:failed).each_batch(of: 100) do |batch|
self.class.transaction do self.class.transaction do
batch.lock.each(&:split_and_retry!) batch.lock.each(&:split_and_retry!)
self.active! self.active!
...@@ -102,7 +102,7 @@ module Gitlab ...@@ -102,7 +102,7 @@ module Gitlab
end end
def migrated_tuple_count def migrated_tuple_count
batched_jobs.succeeded.sum(:batch_size) batched_jobs.with_status(:succeeded).sum(:batch_size)
end end
def prometheus_labels def prometheus_labels
......
...@@ -67,7 +67,7 @@ module Gitlab ...@@ -67,7 +67,7 @@ module Gitlab
Gitlab::AppLogger.warn "Batched background migration for the given configuration is already finished: #{configuration}" Gitlab::AppLogger.warn "Batched background migration for the given configuration is already finished: #{configuration}"
else else
migration.finalizing! migration.finalizing!
migration.batched_jobs.pending.each { |job| migration_wrapper.perform(job) } migration.batched_jobs.with_status(:pending).each { |job| migration_wrapper.perform(job) }
run_migration_while(migration, :finalizing) run_migration_while(migration, :finalizing)
...@@ -116,7 +116,7 @@ module Gitlab ...@@ -116,7 +116,7 @@ module Gitlab
def finish_active_migration(active_migration) def finish_active_migration(active_migration)
return if active_migration.batched_jobs.active.exists? return if active_migration.batched_jobs.active.exists?
if active_migration.batched_jobs.failed.exists? if active_migration.batched_jobs.with_status(:failed).exists?
active_migration.failed! active_migration.failed!
else else
active_migration.finished! active_migration.finished!
......
...@@ -18,20 +18,19 @@ module Gitlab ...@@ -18,20 +18,19 @@ module Gitlab
execute_batch(batch_tracking_record) execute_batch(batch_tracking_record)
batch_tracking_record.status = :succeeded batch_tracking_record.succeed!
rescue Exception # rubocop:disable Lint/RescueException rescue Exception => error # rubocop:disable Lint/RescueException
batch_tracking_record.status = :failed batch_tracking_record.failure!(error: error)
raise raise
ensure ensure
finish_tracking_execution(batch_tracking_record)
track_prometheus_metrics(batch_tracking_record) track_prometheus_metrics(batch_tracking_record)
end end
private private
def start_tracking_execution(tracking_record) def start_tracking_execution(tracking_record)
tracking_record.update!(attempts: tracking_record.attempts + 1, status: :running, started_at: Time.current, finished_at: nil, metrics: {}) tracking_record.run!
end end
def execute_batch(tracking_record) def execute_batch(tracking_record)
...@@ -51,11 +50,6 @@ module Gitlab ...@@ -51,11 +50,6 @@ module Gitlab
end end
end end
def finish_tracking_execution(tracking_record)
tracking_record.finished_at = Time.current
tracking_record.save!
end
def track_prometheus_metrics(tracking_record) def track_prometheus_metrics(tracking_record)
migration = tracking_record.batched_migration migration = tracking_record.batched_migration
base_labels = migration.prometheus_labels base_labels = migration.prometheus_labels
......
...@@ -9,5 +9,21 @@ FactoryBot.define do ...@@ -9,5 +9,21 @@ FactoryBot.define do
batch_size { 5 } batch_size { 5 }
sub_batch_size { 1 } sub_batch_size { 1 }
pause_ms { 100 } pause_ms { 100 }
trait(:pending) do
status { 0 }
end
trait(:running) do
status { 1 }
end
trait(:failed) do
status { 2 }
end
trait(:succeeded) do
status { 3 }
end
end end
end end
...@@ -10,7 +10,7 @@ RSpec.describe "Admin > Admin sees background migrations" do ...@@ -10,7 +10,7 @@ RSpec.describe "Admin > Admin sees background migrations" do
let_it_be(:finished_migration) { create(:batched_background_migration, table_name: 'finished', status: :finished) } let_it_be(:finished_migration) { create(:batched_background_migration, table_name: 'finished', status: :finished) }
before_all do before_all do
create(:batched_background_migration_job, batched_migration: failed_migration, batch_size: 10, min_value: 6, max_value: 15, status: :failed, attempts: 3) create(:batched_background_migration_job, :failed, batched_migration: failed_migration, batch_size: 10, min_value: 6, max_value: 15, attempts: 3)
end end
before do before do
......
...@@ -10,16 +10,82 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d ...@@ -10,16 +10,82 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d
it { is_expected.to have_many(:batched_job_transition_logs).with_foreign_key(:batched_background_migration_job_id) } it { is_expected.to have_many(:batched_job_transition_logs).with_foreign_key(:batched_background_migration_job_id) }
end end
describe 'state machine' do
let_it_be(:job) { create(:batched_background_migration_job, :failed) }
context 'when a job is running' do
it 'logs the transition' do
expect(Gitlab::AppLogger).to receive(:info).with( { batched_job_id: job.id, message: 'BatchedJob transition', new_state: :running, previous_state: :failed } )
expect { job.run! }.to change(job, :started_at)
end
end
context 'when a job succeed' do
let(:job) { create(:batched_background_migration_job, :running) }
it 'logs the transition' do
expect(Gitlab::AppLogger).to receive(:info).with( { batched_job_id: job.id, message: 'BatchedJob transition', new_state: :succeeded, previous_state: :running } )
job.succeed!
end
it 'updates the finished_at' do
expect { job.succeed! }.to change(job, :finished_at).from(nil).to(Time)
end
it 'creates a new transition log' do
job.succeed!
transition_log = job.batched_job_transition_logs.first
expect(transition_log.next_status).to eq('succeeded')
expect(transition_log.exception_class).to be_nil
expect(transition_log.exception_message).to be_nil
end
end
context 'when a job fails' do
let(:job) { create(:batched_background_migration_job, :running) }
it 'logs the transition' do
expect(Gitlab::AppLogger).to receive(:info).with( { batched_job_id: job.id, message: 'BatchedJob transition', new_state: :failed, previous_state: :running } )
job.failure!
end
it 'tracks the exception' do
expect(Gitlab::ErrorTracking).to receive(:track_exception).with(RuntimeError, { batched_job_id: job.id } )
job.failure!(error: RuntimeError.new)
end
it 'updates the finished_at' do
expect { job.failure! }.to change(job, :finished_at).from(nil).to(Time)
end
it 'creates a new transition log' do
job.failure!(error: RuntimeError.new)
transition_log = job.batched_job_transition_logs.first
expect(transition_log.next_status).to eq('failed')
expect(transition_log.exception_class).to eq('RuntimeError')
expect(transition_log.exception_message).to eq('RuntimeError')
end
end
end
describe 'scopes' do describe 'scopes' do
let_it_be(:fixed_time) { Time.new(2021, 04, 27, 10, 00, 00, 00) } let_it_be(:fixed_time) { Time.new(2021, 04, 27, 10, 00, 00, 00) }
let_it_be(:pending_job) { create(:batched_background_migration_job, status: :pending, updated_at: fixed_time) } let_it_be(:pending_job) { create(:batched_background_migration_job, :pending, updated_at: fixed_time) }
let_it_be(:running_job) { create(:batched_background_migration_job, status: :running, updated_at: fixed_time) } let_it_be(:running_job) { create(:batched_background_migration_job, :running, updated_at: fixed_time) }
let_it_be(:stuck_job) { create(:batched_background_migration_job, status: :pending, updated_at: fixed_time - described_class::STUCK_JOBS_TIMEOUT) } let_it_be(:stuck_job) { create(:batched_background_migration_job, :pending, updated_at: fixed_time - described_class::STUCK_JOBS_TIMEOUT) }
let_it_be(:failed_job) { create(:batched_background_migration_job, status: :failed, attempts: 1) } let_it_be(:failed_job) { create(:batched_background_migration_job, :failed, attempts: 1) }
let!(:max_attempts_failed_job) { create(:batched_background_migration_job, status: :failed, attempts: described_class::MAX_ATTEMPTS) } let!(:max_attempts_failed_job) { create(:batched_background_migration_job, :failed, attempts: described_class::MAX_ATTEMPTS) }
let!(:succeeded_job) { create(:batched_background_migration_job, status: :succeeded) } let!(:succeeded_job) { create(:batched_background_migration_job, :succeeded) }
before do before do
travel_to fixed_time travel_to fixed_time
...@@ -83,10 +149,10 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d ...@@ -83,10 +149,10 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d
subject { job.time_efficiency } subject { job.time_efficiency }
let(:migration) { build(:batched_background_migration, interval: 120.seconds) } let(:migration) { build(:batched_background_migration, interval: 120.seconds) }
let(:job) { build(:batched_background_migration_job, status: :succeeded, batched_migration: migration) } let(:job) { build(:batched_background_migration_job, :succeeded, batched_migration: migration) }
context 'when job has not yet succeeded' do context 'when job has not yet succeeded' do
let(:job) { build(:batched_background_migration_job, status: :running) } let(:job) { build(:batched_background_migration_job, :running) }
it 'returns nil' do it 'returns nil' do
expect(subject).to be_nil expect(subject).to be_nil
...@@ -131,7 +197,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d ...@@ -131,7 +197,7 @@ 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, attempts: 3) } let!(:job) { create(:batched_background_migration_job, :failed, batch_size: 10, min_value: 6, max_value: 15, attempts: 3) }
context 'when job can be split' do context 'when job can be split' do
before do before do
...@@ -147,7 +213,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d ...@@ -147,7 +213,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d
min_value: 6, min_value: 6,
max_value: 10, max_value: 10,
batch_size: 5, batch_size: 5,
status: 'failed', status_name: :failed,
attempts: 0, attempts: 0,
started_at: nil, started_at: nil,
finished_at: nil, finished_at: nil,
...@@ -161,7 +227,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d ...@@ -161,7 +227,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d
min_value: 11, min_value: 11,
max_value: 15, max_value: 15,
batch_size: 5, batch_size: 5,
status: 'failed', status_name: :failed,
attempts: 0, attempts: 0,
started_at: nil, started_at: nil,
finished_at: nil, finished_at: nil,
...@@ -178,7 +244,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d ...@@ -178,7 +244,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d
end end
context 'when job is not failed' do context 'when job is not failed' do
let!(:job) { create(:batched_background_migration_job, status: :succeeded) } let!(:job) { create(:batched_background_migration_job, :succeeded) }
it 'raises an exception' do it 'raises an exception' do
expect { job.split_and_retry! }.to raise_error 'Only failed jobs can be split' expect { job.split_and_retry! }.to raise_error 'Only failed jobs can be split'
...@@ -186,7 +252,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d ...@@ -186,7 +252,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d
end end
context 'when batch size is already 1' do context 'when batch size is already 1' do
let!(:job) { create(:batched_background_migration_job, batch_size: 1, status: :failed) } let!(:job) { create(:batched_background_migration_job, :failed, batch_size: 1) }
it 'raises an exception' do it 'raises an exception' do
expect { job.split_and_retry! }.to raise_error 'Job cannot be split further' expect { job.split_and_retry! }.to raise_error 'Job cannot be split further'
...@@ -205,7 +271,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d ...@@ -205,7 +271,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d
expect(job.batch_size).to eq(5) expect(job.batch_size).to eq(5)
expect(job.attempts).to eq(0) expect(job.attempts).to eq(0)
expect(job.status).to eq('failed') expect(job.status_name).to eq(:failed)
end end
end end
end end
......
...@@ -96,13 +96,12 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do ...@@ -96,13 +96,12 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do
end end
let!(:previous_job) do let!(:previous_job) do
create(:batched_background_migration_job, create(:batched_background_migration_job, :succeeded,
batched_migration: migration, batched_migration: migration,
min_value: event1.id, min_value: event1.id,
max_value: event2.id, max_value: event2.id,
batch_size: 2, batch_size: 2,
sub_batch_size: 1, sub_batch_size: 1
status: :succeeded
) )
end end
...@@ -144,7 +143,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do ...@@ -144,7 +143,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do
context 'when migration has failed jobs' do context 'when migration has failed jobs' do
before do before do
previous_job.update!(status: :failed) previous_job.failure!
end end
it 'retries the failed job' do it 'retries the failed job' do
...@@ -172,7 +171,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do ...@@ -172,7 +171,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do
context 'when migration has stuck jobs' do context 'when migration has stuck jobs' do
before do before do
previous_job.update!(status: :running, updated_at: 1.hour.ago - Gitlab::Database::BackgroundMigration::BatchedJob::STUCK_JOBS_TIMEOUT) previous_job.update!(status_event: 'run', updated_at: 1.hour.ago - Gitlab::Database::BackgroundMigration::BatchedJob::STUCK_JOBS_TIMEOUT)
end end
it 'retries the stuck job' do it 'retries the stuck job' do
...@@ -186,7 +185,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do ...@@ -186,7 +185,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do
context 'when migration has possible stuck jobs' do context 'when migration has possible stuck jobs' do
before do before do
previous_job.update!(status: :running, updated_at: 1.hour.from_now - Gitlab::Database::BackgroundMigration::BatchedJob::STUCK_JOBS_TIMEOUT) previous_job.update!(status_event: 'run', updated_at: 1.hour.from_now - Gitlab::Database::BackgroundMigration::BatchedJob::STUCK_JOBS_TIMEOUT)
end end
it 'keeps the migration active' do it 'keeps the migration active' do
...@@ -201,13 +200,13 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do ...@@ -201,13 +200,13 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do
context 'when the migration has batches to process and failed jobs' do context 'when the migration has batches to process and failed jobs' do
before do before do
migration.update!(max_value: event3.id) migration.update!(max_value: event3.id)
previous_job.update!(status: :failed) previous_job.failure!
end end
it 'runs next batch then retries the failed job' do it 'runs next batch then retries the failed job' do
expect(migration_wrapper).to receive(:perform) do |job_record| expect(migration_wrapper).to receive(:perform) do |job_record|
expect(job_record).to eq(job_relation.last) expect(job_record).to eq(job_relation.last)
job_record.update!(status: :succeeded) job_record.succeed!
end end
expect { runner.run_migration_job(migration) }.to change { job_relation.count }.by(1) expect { runner.run_migration_job(migration) }.to change { job_relation.count }.by(1)
...@@ -264,12 +263,12 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do ...@@ -264,12 +263,12 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do
it 'runs all jobs inline until finishing the migration' do it 'runs all jobs inline until finishing the migration' do
expect(migration_wrapper).to receive(:perform) do |job_record| expect(migration_wrapper).to receive(:perform) do |job_record|
expect(job_record).to eq(job_relation.first) expect(job_record).to eq(job_relation.first)
job_record.update!(status: :succeeded) job_record.succeed!
end end
expect(migration_wrapper).to receive(:perform) do |job_record| expect(migration_wrapper).to receive(:perform) do |job_record|
expect(job_record).to eq(job_relation.last) expect(job_record).to eq(job_relation.last)
job_record.update!(status: :succeeded) job_record.succeed!
end end
expect { runner.run_entire_migration(migration) }.to change { job_relation.count }.by(2) expect { runner.run_entire_migration(migration) }.to change { job_relation.count }.by(2)
...@@ -330,9 +329,9 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do ...@@ -330,9 +329,9 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do
pause_ms: 0 pause_ms: 0
} }
create(:batched_background_migration_job, common_attributes.merge(status: :succeeded, min_value: 1, max_value: 2)) create(:batched_background_migration_job, :succeeded, common_attributes.merge(min_value: 1, max_value: 2))
create(:batched_background_migration_job, common_attributes.merge(status: :pending, min_value: 3, max_value: 4)) create(:batched_background_migration_job, :pending, common_attributes.merge(min_value: 3, max_value: 4))
create(:batched_background_migration_job, common_attributes.merge(status: :failed, min_value: 5, max_value: 6, attempts: 1)) create(:batched_background_migration_job, :failed, common_attributes.merge(min_value: 5, max_value: 6, attempts: 1))
end end
it 'completes the migration' do it 'completes the migration' do
...@@ -359,7 +358,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do ...@@ -359,7 +358,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do
context 'when migration fails to complete' do context 'when migration fails to complete' do
it 'raises an error' do it 'raises an error' do
batched_migration.batched_jobs.failed.update_all(attempts: Gitlab::Database::BackgroundMigration::BatchedJob::MAX_ATTEMPTS) batched_migration.batched_jobs.with_status(:failed).update_all(attempts: Gitlab::Database::BackgroundMigration::BatchedJob::MAX_ATTEMPTS)
expect do expect do
runner.finalize( runner.finalize(
......
...@@ -26,7 +26,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m ...@@ -26,7 +26,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m
context 'when there are failed jobs' do context 'when there are failed jobs' do
let(:batched_migration) { create(:batched_background_migration, status: :active, total_tuple_count: 100) } let(:batched_migration) { create(:batched_background_migration, status: :active, total_tuple_count: 100) }
let!(:batched_job) { create(:batched_background_migration_job, batched_migration: batched_migration, status: :failed) } let!(:batched_job) { create(:batched_background_migration_job, :failed, batched_migration: batched_migration) }
it 'raises an exception' do it 'raises an exception' do
expect { batched_migration.finished! }.to raise_error(ActiveRecord::RecordInvalid) expect { batched_migration.finished! }.to raise_error(ActiveRecord::RecordInvalid)
...@@ -37,7 +37,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m ...@@ -37,7 +37,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m
context 'when the jobs are completed' do context 'when the jobs are completed' do
let(:batched_migration) { create(:batched_background_migration, status: :active, total_tuple_count: 100) } let(:batched_migration) { create(:batched_background_migration, status: :active, total_tuple_count: 100) }
let!(:batched_job) { create(:batched_background_migration_job, batched_migration: batched_migration, status: :succeeded) } let!(:batched_job) { create(:batched_background_migration_job, :succeeded, batched_migration: batched_migration) }
it 'finishes the migration' do it 'finishes the migration' do
batched_migration.finished! batched_migration.finished!
...@@ -64,7 +64,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m ...@@ -64,7 +64,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m
it 'returns the first active migration according to queue order' do it 'returns the first active migration according to queue order' do
expect(described_class.active_migration).to eq(migration2) expect(described_class.active_migration).to eq(migration2)
create(:batched_background_migration_job, batched_migration: migration1, batch_size: 1000, status: :succeeded) create(:batched_background_migration_job, :succeeded, batched_migration: migration1, batch_size: 1000)
end end
end end
...@@ -84,10 +84,10 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m ...@@ -84,10 +84,10 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m
let!(:migration_without_jobs) { create(:batched_background_migration) } let!(:migration_without_jobs) { create(:batched_background_migration) }
before do before do
create(:batched_background_migration_job, batched_migration: migration1, batch_size: 1000, status: :succeeded) create(:batched_background_migration_job, :succeeded, batched_migration: migration1, batch_size: 1000)
create(:batched_background_migration_job, batched_migration: migration1, batch_size: 200, status: :failed) create(:batched_background_migration_job, :failed, batched_migration: migration1, batch_size: 200)
create(:batched_background_migration_job, batched_migration: migration2, batch_size: 500, status: :succeeded) create(:batched_background_migration_job, :succeeded, batched_migration: migration2, batch_size: 500)
create(:batched_background_migration_job, batched_migration: migration2, batch_size: 200, status: :running) create(:batched_background_migration_job, :running, batched_migration: migration2, batch_size: 200)
end end
it 'returns totals from successful jobs' do it 'returns totals from successful jobs' do
...@@ -268,7 +268,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m ...@@ -268,7 +268,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m
subject(:retry_failed_jobs) { batched_migration.retry_failed_jobs! } subject(:retry_failed_jobs) { batched_migration.retry_failed_jobs! }
context 'when there are failed migration jobs' do context 'when there are failed migration jobs' do
let!(:batched_background_migration_job) { create(:batched_background_migration_job, batched_migration: batched_migration, batch_size: 10, min_value: 6, max_value: 15, status: :failed, attempts: 3) } let!(:batched_background_migration_job) { create(:batched_background_migration_job, :failed, batched_migration: batched_migration, batch_size: 10, min_value: 6, max_value: 15, attempts: 3) }
before do before do
allow_next_instance_of(Gitlab::BackgroundMigration::BatchingStrategies::PrimaryKeyBatchingStrategy) do |batch_class| allow_next_instance_of(Gitlab::BackgroundMigration::BatchingStrategies::PrimaryKeyBatchingStrategy) do |batch_class|
...@@ -312,9 +312,9 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m ...@@ -312,9 +312,9 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m
let(:batched_migration) { create(:batched_background_migration) } let(:batched_migration) { create(:batched_background_migration) }
before do before do
create_list(:batched_background_migration_job, 5, status: :succeeded, batch_size: 1_000, batched_migration: batched_migration) create_list(:batched_background_migration_job, 5, :succeeded, batch_size: 1_000, batched_migration: batched_migration)
create_list(:batched_background_migration_job, 1, status: :running, batch_size: 1_000, batched_migration: batched_migration) create_list(:batched_background_migration_job, 1, :running, batch_size: 1_000, batched_migration: batched_migration)
create_list(:batched_background_migration_job, 1, status: :failed, batch_size: 1_000, batched_migration: batched_migration) create_list(:batched_background_migration_job, 1, :failed, batch_size: 1_000, batched_migration: batched_migration)
end end
it 'sums the batch_size of succeeded jobs' do it 'sums the batch_size of succeeded jobs' do
...@@ -347,7 +347,6 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m ...@@ -347,7 +347,6 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m
let_it_be(:common_attrs) do let_it_be(:common_attrs) do
{ {
status: :succeeded,
batched_migration: migration, batched_migration: migration,
finished_at: end_time finished_at: end_time
} }
...@@ -357,7 +356,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m ...@@ -357,7 +356,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m
subject { migration.smoothed_time_efficiency(number_of_jobs: 10) } subject { migration.smoothed_time_efficiency(number_of_jobs: 10) }
it 'returns nil' do it 'returns nil' do
create_list(:batched_background_migration_job, 9, **common_attrs) create_list(:batched_background_migration_job, 9, :succeeded, **common_attrs)
expect(subject).to be_nil expect(subject).to be_nil
end end
...@@ -369,6 +368,8 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m ...@@ -369,6 +368,8 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m
subject { migration.smoothed_time_efficiency(number_of_jobs: number_of_jobs) } subject { migration.smoothed_time_efficiency(number_of_jobs: number_of_jobs) }
let!(:jobs) { create_list(:batched_background_migration_job, number_of_jobs, :succeeded, **common_attrs.merge(batched_migration: migration)) }
before do before do
expect(migration).to receive_message_chain(:batched_jobs, :successful_in_execution_order, :reverse_order, :limit, :with_preloads) expect(migration).to receive_message_chain(:batched_jobs, :successful_in_execution_order, :reverse_order, :limit, :with_preloads)
.and_return(jobs) .and_return(jobs)
......
...@@ -35,8 +35,6 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationWrapper, ' ...@@ -35,8 +35,6 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationWrapper, '
expect(job_instance).to receive(:perform) expect(job_instance).to receive(:perform)
expect(job_instance).to receive(:batch_metrics).and_return(test_metrics) expect(job_instance).to receive(:batch_metrics).and_return(test_metrics)
expect(job_record).to receive(:update!).with(hash_including(attempts: 1, status: :running)).and_call_original
freeze_time do freeze_time do
subject subject
...@@ -51,11 +49,10 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationWrapper, ' ...@@ -51,11 +49,10 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationWrapper, '
context 'when running a job that failed previously' do context 'when running a job that failed previously' do
let!(:job_record) do let!(:job_record) do
create(:batched_background_migration_job, create(:batched_background_migration_job, :failed,
batched_migration: active_migration, batched_migration: active_migration,
pause_ms: pause_ms, pause_ms: pause_ms,
attempts: 1, attempts: 1,
status: :failed,
finished_at: 1.hour.ago, finished_at: 1.hour.ago,
metrics: { 'my_metrics' => 'some_value' } metrics: { 'my_metrics' => 'some_value' }
) )
...@@ -67,10 +64,6 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationWrapper, ' ...@@ -67,10 +64,6 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationWrapper, '
expect(job_instance).to receive(:perform) expect(job_instance).to receive(:perform)
expect(job_instance).to receive(:batch_metrics).and_return(updated_metrics) expect(job_instance).to receive(:batch_metrics).and_return(updated_metrics)
expect(job_record).to receive(:update!).with(
hash_including(attempts: 2, status: :running, finished_at: nil, metrics: {})
).and_call_original
freeze_time do freeze_time do
subject subject
......
...@@ -13,7 +13,7 @@ RSpec.describe Admin::BackgroundMigrationsController, :enable_admin_mode do ...@@ -13,7 +13,7 @@ RSpec.describe Admin::BackgroundMigrationsController, :enable_admin_mode do
let(:migration) { create(:batched_background_migration, status: 'failed') } let(:migration) { create(:batched_background_migration, status: 'failed') }
before do before do
create(:batched_background_migration_job, batched_migration: migration, batch_size: 10, min_value: 6, max_value: 15, status: :failed, attempts: 3) create(:batched_background_migration_job, :failed, batched_migration: migration, batch_size: 10, min_value: 6, max_value: 15, attempts: 3)
allow_next_instance_of(Gitlab::BackgroundMigration::BatchingStrategies::PrimaryKeyBatchingStrategy) do |batch_class| allow_next_instance_of(Gitlab::BackgroundMigration::BatchingStrategies::PrimaryKeyBatchingStrategy) do |batch_class|
allow(batch_class).to receive(:next_batch).with(anything, anything, batch_min_value: 6, batch_size: 5).and_return([6, 10]) allow(batch_class).to receive(:next_batch).with(anything, anything, batch_min_value: 6, batch_size: 5).and_return([6, 10])
......
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