Fix retrying of batched background migrations

This also changes the spec to actually call the batching strategy class
so that future regressions can be avoided.

Changelog: fixed

lib/gitlab/database/background_migration/batched_job.rb

@@ -92,7 +92,8 @@ module Gitlab
               batched_migration.table_name,
               batched_migration.column_name,
               batch_min_value: min_value,
-              batch_size: new_batch_size
+              batch_size: new_batch_size,
+              job_arguments: batched_migration.job_arguments
             )
             midpoint = next_batch_bounds.last
 

spec/features/admin/admin_sees_background_migrations_spec.rb

@@ -56,7 +56,13 @@ RSpec.describe "Admin > Admin sees background migrations" do
   context 'when there are failed migrations' do
     before do
       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,
+          job_arguments: failed_migration.job_arguments
+        ).and_return([6, 10])
       end
     end
 

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

@@ -199,15 +199,17 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d
   end
 
   describe '#split_and_retry!' do
-    let!(:job) { create(:batched_background_migration_job, :failed, batch_size: 10, min_value: 6, max_value: 15, attempts: 3) }
+    let_it_be(:migration) { create(:batched_background_migration, table_name: :events) }
+    let_it_be(:job) { create(:batched_background_migration_job, :failed, batched_migration: migration, batch_size: 10, min_value: 6, max_value: 15, attempts: 3) }
+    let_it_be(:project) { create(:project) }
 
-    context 'when job can be split' do
-      before do
-        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])
-        end
+    before_all do
+      (6..16).each do |id|
+        create(:event, id: id, project: project)
       end
+    end
 
+    context 'when job can be split' do
       it 'sets the correct attributes' do
         expect { job.split_and_retry! }.to change { described_class.count }.by(1)
 
@@ -263,9 +265,7 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d
 
     context 'when computed midpoint is larger than the max value of the batch' do
       before do
-        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, 16])
-        end
+        Event.where(id: 6..12).delete_all
       end
 
       it 'lowers the batch size and resets the number of attempts' do

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

@@ -274,7 +274,13 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m
 
       before do
         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,
+            job_arguments: batched_migration.job_arguments
+          ).and_return([6, 10])
         end
       end
 

spec/requests/admin/background_migrations_controller_spec.rb

@@ -16,7 +16,13 @@ RSpec.describe Admin::BackgroundMigrationsController, :enable_admin_mode do
       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])
+        allow(batch_class).to receive(:next_batch).with(
+          anything,
+          anything,
+          batch_min_value: 6,
+          batch_size: 5,
+          job_arguments: migration.job_arguments
+        ).and_return([6, 10])
       end
     end