Merge branch '346359-state-machine-gem-batchedjob' into 'master'

Resolve "Logs transitions and errors for BatchedJob"

See merge request gitlab-org/gitlab!75110

lib/gitlab/database/background_migration/batched_job.rb

@@ -12,23 +12,58 @@ module Gitlab
         MAX_ATTEMPTS = 3
         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
         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 :retriable, -> { from_union([failed.where('attempts < ?', MAX_ATTEMPTS), self.stuck]) }
-        scope :except_succeeded, -> { where(status: self.statuses.except(:succeeded).values) }
-        scope :successful_in_execution_order, -> { where.not(finished_at: nil).succeeded.order(:finished_at) }
+        scope :retriable, -> { from_union([with_status(:failed).where('attempts < ?', MAX_ATTEMPTS), self.stuck]) }
+        scope :except_succeeded, -> { without_status(:succeeded) }
+        scope :successful_in_execution_order, -> { where.not(finished_at: nil).with_status(:succeeded).order(:finished_at) }
         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,
           to: :batched_migration, prefix: :migration
 

lib/gitlab/database/background_migration/batched_job_transition_log.rb

@@ -8,6 +8,8 @@ module Gitlab
 
         self.table_name = :batched_background_migration_job_transition_logs
 
+        self.primary_key = :id
+
         partitioned_by :created_at, strategy: :monthly, retain_for: 6.months
 
         belongs_to :batched_job, foreign_key: :batched_background_migration_job_id
@@ -17,8 +19,8 @@ module Gitlab
         validates :exception_class, length: { maximum: 100 }
         validates :exception_message, length: { maximum: 1000 }
 
-        enum previous_status: BatchedJob.statuses, _prefix: true
-        enum next_status: BatchedJob.statuses, _prefix: true
+        enum previous_status: Gitlab::Database::BackgroundMigration::BatchedJob.state_machine.states.map(&:name), _prefix: true
+        enum next_status: Gitlab::Database::BackgroundMigration::BatchedJob.state_machine.states.map(&:name), _prefix: true
       end
     end
   end

lib/gitlab/database/background_migration/batched_migration.rb

@@ -47,7 +47,7 @@ module Gitlab
 
         def self.successful_rows_counts(migrations)
           BatchedJob
-            .succeeded
+            .with_status(:succeeded)
             .where(batched_background_migration_id: migrations)
             .group(:batched_background_migration_id)
             .sum(:batch_size)
@@ -71,7 +71,7 @@ module Gitlab
         end
 
         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
               batch.lock.each(&:split_and_retry!)
               self.active!
@@ -102,7 +102,7 @@ module Gitlab
         end
 
         def migrated_tuple_count
-          batched_jobs.succeeded.sum(:batch_size)
+          batched_jobs.with_status(:succeeded).sum(:batch_size)
         end
 
         def prometheus_labels

lib/gitlab/database/background_migration/batched_migration_runner.rb

@@ -67,7 +67,7 @@ module Gitlab
             Gitlab::AppLogger.warn "Batched background migration for the given configuration is already finished: #{configuration}"
           else
             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)
 
@@ -116,7 +116,7 @@ module Gitlab
         def finish_active_migration(active_migration)
           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!
           else
             active_migration.finished!

lib/gitlab/database/background_migration/batched_migration_wrapper.rb

@@ -18,20 +18,19 @@ module Gitlab
 
           execute_batch(batch_tracking_record)
 
-          batch_tracking_record.status = :succeeded
-        rescue Exception # rubocop:disable Lint/RescueException
-          batch_tracking_record.status = :failed
+          batch_tracking_record.succeed!
+        rescue Exception => error # rubocop:disable Lint/RescueException
+          batch_tracking_record.failure!(error: error)
 
           raise
         ensure
-          finish_tracking_execution(batch_tracking_record)
           track_prometheus_metrics(batch_tracking_record)
         end
 
         private
 
         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
 
         def execute_batch(tracking_record)
@@ -51,11 +50,6 @@ module Gitlab
           end
         end
 
-        def finish_tracking_execution(tracking_record)
-          tracking_record.finished_at = Time.current
-          tracking_record.save!
-        end
-
         def track_prometheus_metrics(tracking_record)
           migration = tracking_record.batched_migration
           base_labels = migration.prometheus_labels

spec/factories/gitlab/database/background_migration/batched_jobs.rb

@@ -9,5 +9,21 @@ FactoryBot.define do
     batch_size { 5 }
     sub_batch_size { 1 }
     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

spec/features/admin/admin_sees_background_migrations_spec.rb

@@ -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) }
 
   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
 
   before do

spec/lib/gitlab/database/background_migration/batched_job_spec.rb

@@ -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) }
   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
     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(:running_job) { create(:batched_background_migration_job, status: :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(:failed_job) { create(:batched_background_migration_job, status: :failed, attempts: 1) }
+    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, :running, updated_at: fixed_time) }
+    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, :failed, attempts: 1) }
 
-    let!(:max_attempts_failed_job) { create(:batched_background_migration_job, status: :failed, attempts: described_class::MAX_ATTEMPTS) }
-    let!(:succeeded_job) { create(:batched_background_migration_job, status: :succeeded) }
+    let!(:max_attempts_failed_job) { create(:batched_background_migration_job, :failed, attempts: described_class::MAX_ATTEMPTS) }
+    let!(:succeeded_job) { create(:batched_background_migration_job, :succeeded) }
 
     before do
       travel_to fixed_time
@@ -83,10 +149,10 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d
     subject { job.time_efficiency }
 
     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
-      let(:job) { build(:batched_background_migration_job, status: :running) }
+      let(:job) { build(:batched_background_migration_job, :running) }
 
       it 'returns nil' do
         expect(subject).to be_nil
@@ -131,7 +197,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d
   end
 
   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
       before do
@@ -147,7 +213,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d
           min_value: 6,
           max_value: 10,
           batch_size: 5,
-          status: 'failed',
+          status_name: :failed,
           attempts: 0,
           started_at: nil,
           finished_at: nil,
@@ -161,7 +227,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d
           min_value: 11,
           max_value: 15,
           batch_size: 5,
-          status: 'failed',
+          status_name: :failed,
           attempts: 0,
           started_at: nil,
           finished_at: nil,
@@ -178,7 +244,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d
     end
 
     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
         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
     end
 
     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
         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
 
         expect(job.batch_size).to eq(5)
         expect(job.attempts).to eq(0)
-        expect(job.status).to eq('failed')
+        expect(job.status_name).to eq(:failed)
       end
     end
   end

spec/lib/gitlab/database/background_migration/batched_migration_runner_spec.rb

@@ -96,13 +96,12 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do
       end
 
       let!(:previous_job) do
-        create(:batched_background_migration_job,
+        create(:batched_background_migration_job, :succeeded,
           batched_migration: migration,
           min_value: event1.id,
           max_value: event2.id,
           batch_size: 2,
-          sub_batch_size: 1,
-          status: :succeeded
+          sub_batch_size: 1
         )
       end
 
@@ -144,7 +143,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do
 
       context 'when migration has failed jobs' do
         before do
-          previous_job.update!(status: :failed)
+          previous_job.failure!
         end
 
         it 'retries the failed job' do
@@ -172,7 +171,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do
 
       context 'when migration has stuck jobs' 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
 
         it 'retries the stuck job' do
@@ -186,7 +185,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do
 
       context 'when migration has possible stuck jobs' 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
 
         it 'keeps the migration active' do
@@ -201,13 +200,13 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do
       context 'when the migration has batches to process and failed jobs' do
         before do
           migration.update!(max_value: event3.id)
-          previous_job.update!(status: :failed)
+          previous_job.failure!
         end
 
         it 'runs next batch then retries the failed job' do
           expect(migration_wrapper).to receive(:perform) do |job_record|
             expect(job_record).to eq(job_relation.last)
-            job_record.update!(status: :succeeded)
+            job_record.succeed!
           end
 
           expect { runner.run_migration_job(migration) }.to change { job_relation.count }.by(1)
@@ -264,12 +263,12 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do
       it 'runs all jobs inline until finishing the migration' do
         expect(migration_wrapper).to receive(:perform) do |job_record|
           expect(job_record).to eq(job_relation.first)
-          job_record.update!(status: :succeeded)
+          job_record.succeed!
         end
 
         expect(migration_wrapper).to receive(:perform) do |job_record|
           expect(job_record).to eq(job_relation.last)
-          job_record.update!(status: :succeeded)
+          job_record.succeed!
         end
 
         expect { runner.run_entire_migration(migration) }.to change { job_relation.count }.by(2)
@@ -330,9 +329,9 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do
           pause_ms: 0
         }
 
-        create(:batched_background_migration_job, common_attributes.merge(status: :succeeded, 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, common_attributes.merge(status: :failed, min_value: 5, max_value: 6, attempts: 1))
+        create(:batched_background_migration_job, :succeeded, common_attributes.merge(min_value: 1, max_value: 2))
+        create(:batched_background_migration_job, :pending, common_attributes.merge(min_value: 3, max_value: 4))
+        create(:batched_background_migration_job, :failed, common_attributes.merge(min_value: 5, max_value: 6, attempts: 1))
       end
 
       it 'completes the migration' do
@@ -359,7 +358,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do
 
       context 'when migration fails to complete' 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
             runner.finalize(

spec/lib/gitlab/database/background_migration/batched_migration_spec.rb

@@ -26,7 +26,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m
 
     context 'when there are failed jobs' do
       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
         expect { batched_migration.finished! }.to raise_error(ActiveRecord::RecordInvalid)
@@ -37,7 +37,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m
 
     context 'when the jobs are completed' do
       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
         batched_migration.finished!
@@ -64,7 +64,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m
 
     it 'returns the first active migration according to queue order' do
       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
 
@@ -84,10 +84,10 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m
     let!(:migration_without_jobs) { create(:batched_background_migration) }
 
     before do
-      create(:batched_background_migration_job, batched_migration: migration1, batch_size: 1000, status: :succeeded)
-      create(:batched_background_migration_job, batched_migration: migration1, batch_size: 200, status: :failed)
-      create(:batched_background_migration_job, batched_migration: migration2, batch_size: 500, status: :succeeded)
-      create(:batched_background_migration_job, batched_migration: migration2, batch_size: 200, status: :running)
+      create(:batched_background_migration_job, :succeeded, batched_migration: migration1, batch_size: 1000)
+      create(:batched_background_migration_job, :failed, batched_migration: migration1, batch_size: 200)
+      create(:batched_background_migration_job, :succeeded, batched_migration: migration2, batch_size: 500)
+      create(:batched_background_migration_job, :running, batched_migration: migration2, batch_size: 200)
     end
 
     it 'returns totals from successful jobs' do
@@ -268,7 +268,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m
     subject(:retry_failed_jobs) { batched_migration.retry_failed_jobs! }
 
     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
         allow_next_instance_of(Gitlab::BackgroundMigration::BatchingStrategies::PrimaryKeyBatchingStrategy) do |batch_class|
@@ -312,9 +312,9 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m
     let(:batched_migration) { create(:batched_background_migration) }
 
     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, 1, status: :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, 5, :succeeded, 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, :failed, batch_size: 1_000, batched_migration: batched_migration)
     end
 
     it 'sums the batch_size of succeeded jobs' do
@@ -347,7 +347,6 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m
 
     let_it_be(:common_attrs) do
       {
-        status: :succeeded,
         batched_migration: migration,
         finished_at: end_time
       }
@@ -357,7 +356,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m
       subject { migration.smoothed_time_efficiency(number_of_jobs: 10) }
 
       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
       end
@@ -369,6 +368,8 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m
 
       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
         expect(migration).to receive_message_chain(:batched_jobs, :successful_in_execution_order, :reverse_order, :limit, :with_preloads)
                                .and_return(jobs)

spec/lib/gitlab/database/background_migration/batched_migration_wrapper_spec.rb

@@ -35,8 +35,6 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationWrapper, '
     expect(job_instance).to receive(:perform)
     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
       subject
 
@@ -51,11 +49,10 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationWrapper, '
 
   context 'when running a job that failed previously' do
     let!(:job_record) do
-      create(:batched_background_migration_job,
+      create(:batched_background_migration_job, :failed,
         batched_migration: active_migration,
         pause_ms: pause_ms,
         attempts: 1,
-        status: :failed,
         finished_at: 1.hour.ago,
         metrics: { 'my_metrics' => 'some_value' }
       )
@@ -67,10 +64,6 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationWrapper, '
       expect(job_instance).to receive(:perform)
       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
         subject
 

spec/requests/admin/background_migrations_controller_spec.rb

@@ -13,7 +13,7 @@ RSpec.describe Admin::BackgroundMigrationsController, :enable_admin_mode do
     let(:migration) { create(:batched_background_migration, status: 'failed') }
 
     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(batch_class).to receive(:next_batch).with(anything, anything, batch_min_value: 6, batch_size: 5).and_return([6, 10])