Create a diff deletion worker scheduler to avoid long-running post-migration

2 files changed, 58 insertions, 25 deletions

diff --git a/spec/lib/gitlab/background_migration/schedule_diff_files_deletion_spec.rb b/spec/lib/gitlab/background_migration/schedule_diff_files_deletion_spec.rb
new file mode 100644
index 00000000000..a699571ca90
--- /dev/null
+++ b/spec/lib/gitlab/background_migration/schedule_diff_files_deletion_spec.rb
@@ -0,0 +1,43 @@
+require 'spec_helper'
+
+describe Gitlab::BackgroundMigration::ScheduleDiffFilesDeletion, :migration, schema: 20180619121030 do
+  describe '#perform' do
+    let(:merge_request_diffs) { table(:merge_request_diffs) }
+    let(:merge_requests) { table(:merge_requests) }
+    let(:namespaces) { table(:namespaces) }
+    let(:projects) { table(:projects) }
+
+    before do
+      stub_const("#{described_class.name}::BATCH_SIZE", 3)
+
+      namespaces.create!(id: 1, name: 'gitlab', path: 'gitlab')
+      projects.create!(id: 1, namespace_id: 1, name: 'gitlab', path: 'gitlab')
+
+      merge_requests.create!(id: 1, target_project_id: 1, source_project_id: 1, target_branch: 'feature', source_branch: 'master', state: 'merged')
+
+      merge_request_diffs.create!(id: 1, merge_request_id: 1)
+      merge_request_diffs.create!(id: 2, merge_request_id: 1)
+      merge_request_diffs.create!(id: 3, merge_request_id: 1)
+      merge_request_diffs.create!(id: 4, merge_request_id: 1)
+      merge_request_diffs.create!(id: 5, merge_request_id: 1)
+    end
+
+    it 'correctly schedules diff file deletion workers' do
+      Sidekiq::Testing.fake! do
+        Timecop.freeze do
+          described_class.new.perform([1, 2, 3, 4, 5], 1)
+
+          [1, 2, 3].each do |id|
+            expect(described_class::MIGRATION).to be_scheduled_delayed_migration(10.minutes, id)
+          end
+
+          [4, 5].each do |id|
+            expect(described_class::MIGRATION).to be_scheduled_delayed_migration(11.minutes, id)
+          end
+
+          expect(BackgroundMigrationWorker.jobs.size).to eq(5)
+        end
+      end
+    end
+  end
+end
diff --git a/spec/migrations/enqueue_delete_diff_files_workers_spec.rb b/spec/migrations/enqueue_delete_diff_files_workers_spec.rb
index 52d6e9b2c64..43dbd7e8f53 100644
--- a/spec/migrations/enqueue_delete_diff_files_workers_spec.rb
+++ b/spec/migrations/enqueue_delete_diff_files_workers_spec.rb
@@ -8,7 +8,7 @@ describe EnqueueDeleteDiffFilesWorkers, :migration, :sidekiq do
   let(:projects) { table(:projects) }
 
   before do
-    stub_const("#{described_class.name}::BATCH_SIZE", 7)
+    stub_const("#{described_class.name}::BATCH_SIZE", 4)
 
     namespaces.create!(id: 1, name: 'gitlab', path: 'gitlab')
     projects.create!(id: 1, namespace_id: 1, name: 'gitlab', path: 'gitlab')
@@ -22,35 +22,25 @@ describe EnqueueDeleteDiffFilesWorkers, :migration, :sidekiq do
     merge_request_diffs.create!(id: 5, merge_request_id: 1, state: 'empty')
     merge_request_diffs.create!(id: 6, merge_request_id: 1, state: 'collected')
     merge_request_diffs.create!(id: 7, merge_request_id: 1, state: 'collected')
-    merge_request_diffs.create!(id: 8, merge_request_id: 1, state: 'collected')
-    merge_request_diffs.create!(id: 9, merge_request_id: 1, state: 'collected')
-    merge_request_diffs.create!(id: 10, merge_request_id: 1, state: 'collected')
 
     merge_requests.update(1, latest_merge_request_diff_id: 6)
   end
 
-  it 'correctly schedules diff file deletion workers' do
+  it 'correctly schedules diff file deletion workers schedulers' do
     Sidekiq::Testing.fake! do
-      Timecop.freeze do
-        migrate!
-
-        # 1st batch schedule
-        [1, 3, 4, 6, 7].each do |id|
-          expect(described_class::MIGRATION).to be_scheduled_delayed_migration(10.minutes, id)
-        end
-        [8, 9].each do |id|
-          expect(described_class::MIGRATION).to be_scheduled_delayed_migration(11.minutes, id)
-        end
-
-        # 2nd batch schedule
-        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(20.minutes, 10)
-        expect(BackgroundMigrationWorker.jobs.size).to eq(8)
-      end
-    end
-  end
+      # First scheduling batch
+      expect(BackgroundMigrationWorker).to receive(:perform_async)
+        .with(described_class::SCHEDULER, [[[1], [3], [4], [6]], 1])
+        .and_call_original
+
+      # Second scheduling batch
+      expect(BackgroundMigrationWorker).to receive(:perform_async)
+        .with(described_class::SCHEDULER, [[[7]], 2])
+        .and_call_original
 
-  it 'migrates the data' do
-    expect { migrate! }.to change { merge_request_diffs.where(state: 'without_files').count }
-      .from(1).to(4)
+      migrate!
+
+      expect(BackgroundMigrationWorker.jobs.size).to eq(2)
+    end
   end
 end