Move EpicIssuesPosition migration to background

db/post_migrate/20171221154744_schedule_epic_issue_positions_migration.rb

+class ScheduleEpicIssuePositionsMigration < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  MIGRATION = 'SetEpicIssuesPositionValues'.freeze
+  BATCH_SIZE = 100
+
+  class Epic < ActiveRecord::Base
+    self.table_name = 'epics'
+
+    include EachBatch
+  end
+
+  def up
+    Epic.select(:id).each_batch(of: BATCH_SIZE) do |batch, index|
+      start_id, end_id = batch.pluck('MIN(id), MAX(id)').first
+
+      BackgroundMigrationWorker.perform_in(index * 5.minutes, MIGRATION, [start_id, end_id])
+    end
+  end
+end

db/post_migrate/20171221154744_set_epic_issue_position_values.rb

-class SetEpicIssuePositionValues < ActiveRecord::Migration
-  include Gitlab::Database::MigrationHelpers
-
-  DOWNTIME = false
-
-  disable_ddl_transaction!
-
-  class Epic < ActiveRecord::Base
-    self.table_name = 'epics'
-  end
-
-  class EpicIssue < ActiveRecord::Base
-    self.table_name = 'epic_issues'
-  end
-
-  def up
-    epic_issues = select_all('SELECT id, epic_id FROM epic_issues ORDER by epic_id, id')
-      .group_by { |e| e['epic_id'] }
-
-    epic_issues.each do |epic_id, epic_issues|
-      epic_issues.each_with_index do |epic_issue, index|
-        execute("UPDATE epic_issues SET position = #{index + 1} WHERE id = #{epic_issue['id']}")
-      end
-    end
-  end
-
-  def down
-    execute('UPDATE epic_issues SET position = 1')
-  end
-end

lib/gitlab/background_migration/set_epic_issues_position_values.rb

+module Gitlab
+  module BackgroundMigration
+    class SetEpicIssuesPositionValues
+      class Epic < ActiveRecord::Base
+        self.table_name = 'epics'
+      end
+
+      class EpicIssue < ActiveRecord::Base
+        self.table_name = 'epic_issues'
+      end
+
+      def perform(start_id, end_id)
+        epic_issues =  EpicIssue.where(epic_id: start_id..end_id).order('epic_id, id').group_by { |e| e['epic_id'] }
+        return if epic_issues.empty?
+        update = []
+
+        epic_issues.each do |epic_id, issues|
+          issues.each_with_index do |epic_issue, index|
+            update << "WHEN id = #{epic_issue['id']} THEN #{index + 1}"
+          end
+        end
+
+        EpicIssue.where(epic_id: start_id..end_id).update_all("position = CASE #{update.join(' ')} END")
+      end
+    end
+  end
+end

spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb

@@ -2,21 +2,10 @@ require 'spec_helper'
 
 describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do
   include TrackUntrackedUploadsHelpers
+  include MigrationsHelpers
 
   let!(:untracked_files_for_uploads) { described_class::UntrackedFile }
 
-  matcher :be_scheduled_migration do |*expected|
-    match do |migration|
-      BackgroundMigrationWorker.jobs.any? do |job|
-        job['args'] == [migration, expected]
-      end
-    end
-
-    failure_message do |migration|
-      "Migration `#{migration}` with args `#{expected.inspect}` not scheduled!"
-    end
-  end
-
   before do
     DatabaseCleaner.clean
 

spec/lib/gitlab/background_migration/set_epic_issue_position_values_spec.rb

+require 'spec_helper'
+
+describe Gitlab::BackgroundMigration::SetEpicIssuesPositionValues, :migration, schema: 20171221154744 do
+  let(:groups) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:users) { table(:users) }
+  let(:issues) { table(:issues) }
+  let(:epics) { table(:epics) }
+  let(:epic_issues) { table(:epic_issues) }
+
+  let(:group) { groups.create(name: 'group', path: 'group') }
+  let(:project) { projects.create(name: 'group', namespace_id: group.id) }
+  let(:user) { users.create(username: 'User') }
+
+  describe '#perform' do
+    context 'when there are some epics in the db' do
+      let(:epic1) { epics.create(id: 1, title: 'Epic 1', title_html: 'Epic 1', group_id: group.id, author_id: user.id, iid: 1) }
+      let(:epic2) { epics.create(id: 2, title: 'Epic 2', title_html: 'Epic 2', group_id: group.id, author_id: user.id, iid: 2) }
+      let!(:epic3) { epics.create(id: 3, title: 'Epic 3', title_html: 'Epic 3', group_id: group.id, author_id: user.id, iid: 3) }
+
+      let(:issue1) { issues.create(title: 'Issue 1', title_html: 'Issue 1', project_id: project.id, author_id: user.id) }
+      let(:issue2) { issues.create(title: 'Issue 2', title_html: 'Issue 2', project_id: project.id, author_id: user.id) }
+      let(:issue3) { issues.create(title: 'Issue 3', title_html: 'Issue 3', project_id: project.id, author_id: user.id) }
+      let(:issue4) { issues.create(title: 'Issue 4', title_html: 'Issue 4', project_id: project.id, author_id: user.id) }
+      let(:issue5) { issues.create(title: 'Issue 5', title_html: 'Issue 5', project_id: project.id, author_id: user.id) }
+
+      let!(:epic_issue1) { epic_issues.create!(epic_id: epic1.id, issue_id: issue1.id) }
+      let!(:epic_issue2) { epic_issues.create!(epic_id: epic1.id, issue_id: issue2.id) }
+      let!(:epic_issue3) { epic_issues.create!(epic_id: epic2.id, issue_id: issue3.id) }
+      let!(:epic_issue4) { epic_issues.create!(epic_id: epic2.id, issue_id: issue4.id) }
+      let!(:epic_issue5) { epic_issues.create!(epic_id: epic2.id, issue_id: issue5.id) }
+
+      it 'sets the position value correctly' do
+        subject.perform(1, 3)
+
+        expect(epic_issue1.reload.position).to eq(1)
+        expect(epic_issue2.reload.position).to eq(2)
+        expect(epic_issue3.reload.position).to eq(1)
+        expect(epic_issue4.reload.position).to eq(2)
+        expect(epic_issue5.reload.position).to eq(3)
+      end
+    end
+
+    context 'when there are no epics in the db' do
+      it 'runs the migration without errors' do
+        expect(subject.perform(1, 2)).to be_nil
+      end
+    end
+  end
+end

spec/migrations/migrate_stage_id_reference_in_background_spec.rb

@@ -35,9 +35,9 @@ describe MigrateStageIdReferenceInBackground, :migration, :sidekiq do
       Timecop.freeze do
         migrate!
 
-        expect(described_class::MIGRATION).to be_scheduled_migration(2.minutes, 1, 2)
-        expect(described_class::MIGRATION).to be_scheduled_migration(2.minutes, 3, 3)
-        expect(described_class::MIGRATION).to be_scheduled_migration(4.minutes, 4, 5)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(2.minutes, 1, 2)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(2.minutes, 3, 3)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(4.minutes, 4, 5)
         expect(BackgroundMigrationWorker.jobs.size).to eq 3
       end
     end

spec/migrations/migrate_stages_statuses_spec.rb

@@ -50,9 +50,9 @@ describe MigrateStagesStatuses, :migration do
       Timecop.freeze do
         migrate!
 
-        expect(described_class::MIGRATION).to be_scheduled_migration(5.minutes, 1, 1)
-        expect(described_class::MIGRATION).to be_scheduled_migration(5.minutes, 2, 2)
-        expect(described_class::MIGRATION).to be_scheduled_migration(10.minutes, 3, 3)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(5.minutes, 1, 1)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(5.minutes, 2, 2)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(10.minutes, 3, 3)
         expect(BackgroundMigrationWorker.jobs.size).to eq 3
       end
     end

spec/migrations/schedule_create_gpg_key_subkeys_from_gpg_keys_spec.rb

@@ -2,18 +2,6 @@ require 'spec_helper'
 require Rails.root.join('db', 'post_migrate', '20171005130944_schedule_create_gpg_key_subkeys_from_gpg_keys')
 
 describe ScheduleCreateGpgKeySubkeysFromGpgKeys, :migration, :sidekiq do
-  matcher :be_scheduled_migration do |*expected|
-    match do |migration|
-      BackgroundMigrationWorker.jobs.any? do |job|
-        job['args'] == [migration, expected]
-      end
-    end
-
-    failure_message do |migration|
-      "Migration `#{migration}` with args `#{expected.inspect}` not scheduled!"
-    end
-  end
-
   before do
     create(:gpg_key, id: 1, key: GpgHelpers::User1.public_key)
     create(:gpg_key, id: 2, key: GpgHelpers::User3.public_key)

spec/migrations/schedule_epic_issue_positions_migration_spec.rb

+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20171221154744_schedule_epic_issue_positions_migration.rb')
+
+describe ScheduleEpicIssuePositionsMigration, :migration, :sidekiq do
+  let(:groups) { table(:namespaces) }
+  let(:users) { table(:users) }
+  let(:epics) { table(:epics) }
+
+  before do
+    stub_const("#{described_class.name}::BATCH_SIZE", 2)
+
+    group = groups.create(name: 'group', path: 'group')
+    user = users.create(username: 'User')
+    epics.create(id: 1, title: 'Epic 1', title_html: 'Epic 1', group_id: group.id, author_id: user.id, iid: 1)
+    epics.create(id: 2, title: 'Epic 2', title_html: 'Epic 2', group_id: group.id, author_id: user.id, iid: 2)
+    epics.create(id: 3, title: 'Epic 3', title_html: 'Epic 3', group_id: group.id, author_id: user.id, iid: 3)
+  end
+
+  it 'correctly schedules background migrations' do
+    Sidekiq::Testing.fake! do
+      Timecop.freeze do
+        migrate!
+
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(5.minutes, 1, 2)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(10.minutes, 3, 3)
+        expect(BackgroundMigrationWorker.jobs.size).to eq(2)
+      end
+    end
+  end
+end

spec/migrations/schedule_merge_request_diff_migrations_spec.rb

@@ -24,9 +24,9 @@ describe ScheduleMergeRequestDiffMigrations, :migration, :sidekiq do
       Timecop.freeze do
         migrate!
 
-        expect(described_class::MIGRATION).to be_scheduled_migration(5.minutes, 1, 1)
-        expect(described_class::MIGRATION).to be_scheduled_migration(10.minutes, 2, 2)
-        expect(described_class::MIGRATION).to be_scheduled_migration(15.minutes, 4, 4)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(5.minutes, 1, 1)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(10.minutes, 2, 2)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(15.minutes, 4, 4)
         expect(BackgroundMigrationWorker.jobs.size).to eq 3
       end
     end

spec/migrations/schedule_merge_request_diff_migrations_take_two_spec.rb

@@ -24,9 +24,9 @@ describe ScheduleMergeRequestDiffMigrationsTakeTwo, :migration, :sidekiq do
       Timecop.freeze do
         migrate!
 
-        expect(described_class::MIGRATION).to be_scheduled_migration(10.minutes, 1, 1)
-        expect(described_class::MIGRATION).to be_scheduled_migration(20.minutes, 2, 2)
-        expect(described_class::MIGRATION).to be_scheduled_migration(30.minutes, 4, 4)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(10.minutes, 1, 1)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(20.minutes, 2, 2)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(30.minutes, 4, 4)
         expect(BackgroundMigrationWorker.jobs.size).to eq 3
       end
     end

spec/migrations/schedule_merge_request_latest_merge_request_diff_id_migrations_spec.rb

@@ -44,9 +44,9 @@ describe ScheduleMergeRequestLatestMergeRequestDiffIdMigrations, :migration, :si
       Timecop.freeze do
         migrate!
 
-        expect(described_class::MIGRATION).to be_scheduled_migration(5.minutes, merge_request_1.id, merge_request_1.id)
-        expect(described_class::MIGRATION).to be_scheduled_migration(10.minutes, merge_request_2.id, merge_request_2.id)
-        expect(described_class::MIGRATION).to be_scheduled_migration(15.minutes, merge_request_4.id, merge_request_4.id)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(5.minutes, merge_request_1.id, merge_request_1.id)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(10.minutes, merge_request_2.id, merge_request_2.id)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(15.minutes, merge_request_4.id, merge_request_4.id)
         expect(BackgroundMigrationWorker.jobs.size).to eq 3
       end
     end

spec/migrations/set_epic_issue_position_values_spec.rb

-require 'spec_helper'
-require Rails.root.join('db', 'post_migrate', '20171221154744_set_epic_issue_position_values.rb')
-
-describe SetEpicIssuePositionValues, :migration do
-  let(:groups) { table(:namespaces) }
-  let(:projects) { table(:projects) }
-  let(:users) { table(:users) }
-  let(:issues) { table(:issues) }
-  let(:epics) { table(:epics) }
-  let(:epic_issues) { table(:epic_issues) }
-
-  let(:group) { groups.create(name: 'group', path: 'group') }
-  let(:project) { projects.create(name: 'group', namespace_id: group.id) }
-  let(:user) { users.create(username: 'User') }
-
-  let(:epic1) { epics.create(title: 'Epic 1', title_html: 'Epic 1', group_id: group.id, author_id: user.id, iid: 1) }
-  let(:epic2) { epics.create(title: 'Epic 1', title_html: 'Epic 1', group_id: group.id, author_id: user.id, iid: 1) }
-
-  let(:issue1) { issues.create(title: 'Issue 1', title_html: 'Issue 1', project_id: project.id, author_id: user.id) }
-  let(:issue2) { issues.create(title: 'Issue 2', title_html: 'Issue 1', project_id: project.id, author_id: user.id) }
-  let(:issue3) { issues.create(title: 'Issue 3', title_html: 'Issue 1', project_id: project.id, author_id: user.id) }
-  let(:issue4) { issues.create(title: 'Issue 4', title_html: 'Issue 1', project_id: project.id, author_id: user.id) }
-  let(:issue5) { issues.create(title: 'Issue 5', title_html: 'Issue 1', project_id: project.id, author_id: user.id) }
-
-  let!(:epic_issue1) { epic_issues.create!(epic_id: epic1.id, issue_id: issue1.id) }
-  let!(:epic_issue2) { epic_issues.create!(epic_id: epic1.id, issue_id: issue2.id) }
-  let!(:epic_issue3) { epic_issues.create!(epic_id: epic2.id, issue_id: issue3.id) }
-  let!(:epic_issue4) { epic_issues.create!(epic_id: epic2.id, issue_id: issue4.id) }
-  let!(:epic_issue5) { epic_issues.create!(epic_id: epic2.id, issue_id: issue5.id) }
-
-  it 'sets the position value correctly' do
-    migrate!
-
-    expect(epic_issue1.reload.position).to eq(1)
-    expect(epic_issue2.reload.position).to eq(2)
-    expect(epic_issue3.reload.position).to eq(1)
-    expect(epic_issue4.reload.position).to eq(2)
-    expect(epic_issue5.reload.position).to eq(3)
-  end
-end

spec/migrations/track_untracked_uploads_spec.rb

@@ -4,18 +4,6 @@ require Rails.root.join('db', 'post_migrate', '20171103140253_track_untracked_up
 describe TrackUntrackedUploads, :migration, :sidekiq do
   include TrackUntrackedUploadsHelpers
 
-  matcher :be_scheduled_migration do
-    match do |migration|
-      BackgroundMigrationWorker.jobs.any? do |job|
-        job['args'] == [migration]
-      end
-    end
-
-    failure_message do |migration|
-      "Migration `#{migration}` with args `#{expected.inspect}` not scheduled!"
-    end
-  end
-
   it 'correctly schedules the follow-up background migration' do
     Sidekiq::Testing.fake! do
       migrate!

spec/support/background_migrations_matchers.rb

-RSpec::Matchers.define :be_scheduled_migration do |delay, *expected|
+RSpec::Matchers.define :be_scheduled_delayed_migration do |delay, *expected|
   match do |migration|
     BackgroundMigrationWorker.jobs.any? do |job|
       job['args'] == [migration, expected] &&
@@ -11,3 +11,16 @@ RSpec::Matchers.define :be_scheduled_migration do |delay, *expected|
       'not scheduled in expected time!'
   end
 end
+
+RSpec::Matchers.define :be_scheduled_migration do |*expected|
+  match do |migration|
+    BackgroundMigrationWorker.jobs.any? do |job|
+      args = job['args'].size == 1 ? [BackgroundMigrationWorker.jobs[0]['args'][0], []] : job['args']
+      args == [migration, expected]
+    end
+  end
+
+  failure_message do |migration|
+    "Migration `#{migration}` with args `#{expected.inspect}` not scheduled!"
+  end
+end