Use schedulers and delete diff files upon deadtuples check

3 files changed, 57 insertions, 42 deletions

diff --git a/spec/lib/gitlab/background_migration/delete_diff_files_spec.rb b/spec/lib/gitlab/background_migration/delete_diff_files_spec.rb
index 247e0786dd6..64c994a268f 100644
--- a/spec/lib/gitlab/background_migration/delete_diff_files_spec.rb
+++ b/spec/lib/gitlab/background_migration/delete_diff_files_spec.rb
@@ -9,14 +9,18 @@ describe Gitlab::BackgroundMigration::DeleteDiffFiles, :migration, schema: 20180
         merge_request.merge_request_diffs.first
       end
 
+      let(:perform) do
+        described_class.new.perform(MergeRequestDiff.pluck(:id))
+      end
+
       it 'deletes all merge request diff files' do
-        expect { described_class.new.perform }
+        expect { perform }
           .to change { merge_request_diff.merge_request_diff_files.count }
           .from(20).to(0)
       end
 
       it 'updates state to without_files' do
-        expect { described_class.new.perform }
+        expect { perform }
           .to change { merge_request_diff.reload.state }
           .from('collected').to('without_files')
       end
@@ -25,7 +29,7 @@ describe Gitlab::BackgroundMigration::DeleteDiffFiles, :migration, schema: 20180
         expect(described_class::MergeRequestDiffFile).to receive_message_chain(:where, :delete_all)
           .and_raise
 
-        expect { described_class.new.perform }
+        expect { perform }
           .to raise_error
 
         merge_request_diff.reload
@@ -35,50 +39,18 @@ describe Gitlab::BackgroundMigration::DeleteDiffFiles, :migration, schema: 20180
       end
     end
 
-    it 'deletes no merge request diff files when MR is not merged' do
-      merge_request = create(:merge_request, :opened)
-      merge_request.create_merge_request_diff
-      merge_request_diff = merge_request.merge_request_diffs.first
-
-      expect { described_class.new.perform }
-        .not_to change { merge_request_diff.merge_request_diff_files.count }
-        .from(20)
-    end
-
-    it 'deletes no merge request diff files when diff is marked as "without_files"' do
-      merge_request = create(:merge_request, :merged)
-      merge_request.create_merge_request_diff
-      merge_request_diff = merge_request.merge_request_diffs.first
-
-      merge_request_diff.clean!
-
-      expect { described_class.new.perform }
-        .not_to change { merge_request_diff.merge_request_diff_files.count }
-        .from(20)
-    end
-
-    it 'deletes no merge request diff files when diff is the latest' do
+    it 'reschedules itself when should_wait_deadtuple_vacuum' do
       merge_request = create(:merge_request, :merged)
-      merge_request_diff = merge_request.merge_request_diff
-
-      expect { described_class.new.perform }
-        .not_to change { merge_request_diff.merge_request_diff_files.count }
-        .from(20)
-    end
+      first_diff = merge_request.merge_request_diff
+      second_diff = merge_request.create_merge_request_diff
 
-    it 'reschedules itself when should_wait_deadtuple_vacuum' do
       Sidekiq::Testing.fake! do
         worker = described_class.new
-
         allow(worker).to receive(:should_wait_deadtuple_vacuum?) { true }
 
-        expect(BackgroundMigrationWorker)
-          .to receive(:perform_in)
-          .with(described_class::VACUUM_WAIT_TIME, 'DeleteDiffFiles')
-          .and_call_original
-
-        worker.perform
+        worker.perform([first_diff.id, second_diff.id])
 
+        expect(described_class.name.demodulize).to be_scheduled_delayed_migration(5.minutes, [first_diff.id, second_diff.id])
         expect(BackgroundMigrationWorker.jobs.size).to eq(1)
       end
     end
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..fb5093b0bd1
--- /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}::DIFF_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, state: 'collected')
+      merge_request_diffs.create!(id: 2, merge_request_id: 1, state: 'empty')
+      merge_request_diffs.create!(id: 3, merge_request_id: 1, state: 'without_files')
+      merge_request_diffs.create!(id: 4, merge_request_id: 1, state: 'collected')
+      merge_request_diffs.create!(id: 5, merge_request_id: 1, state: 'collected')
+      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_requests.update(1, latest_merge_request_diff_id: 7)
+    end
+
+    it 'correctly schedules diff file deletion workers' do
+      Sidekiq::Testing.fake! do
+        Timecop.freeze do
+          described_class.new.perform
+
+          expect(described_class::MIGRATION).to be_scheduled_delayed_migration(5.minutes, [1, 4, 5])
+
+          expect(described_class::MIGRATION).to be_scheduled_delayed_migration(10.minutes, [6])
+
+          expect(BackgroundMigrationWorker.jobs.size).to eq(2)
+        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 803830d81cf..6bae870920c 100644
--- a/spec/migrations/enqueue_delete_diff_files_workers_spec.rb
+++ b/spec/migrations/enqueue_delete_diff_files_workers_spec.rb
@@ -2,11 +2,11 @@ require 'spec_helper'
 require Rails.root.join('db', 'post_migrate', '20180619121030_enqueue_delete_diff_files_workers.rb')
 
 describe EnqueueDeleteDiffFilesWorkers, :migration, :sidekiq do
-  it 'correctly schedules diff files deletion' do
+  it 'correctly schedules diff files deletion schedulers' do
     Sidekiq::Testing.fake! do
       expect(BackgroundMigrationWorker)
         .to receive(:perform_async)
-        .with(described_class::MIGRATION)
+        .with(described_class::SCHEDULER)
         .and_call_original
 
       migrate!