Split background migration for issues and merge requests

8 files changed, 81 insertions, 50 deletions

diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 6baf479d8d1..0ec0789a24f 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -194,7 +194,7 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def self.available_states
-    @states ||= super.merge(locked: 3, merged: 4)
+    @states ||= super.merge(merged: 3, locked: 4)
   end
 
   def rebase_in_progress?
diff --git a/db/migrate/20190211131150_add_state_id_to_issuables.rb b/db/migrate/20190211131150_add_state_id_to_issuables.rb
index 440f577e1a3..cf3f7671a67 100644
--- a/db/migrate/20190211131150_add_state_id_to_issuables.rb
+++ b/db/migrate/20190211131150_add_state_id_to_issuables.rb
@@ -1,41 +1,11 @@
 class AddStateIdToIssuables < ActiveRecord::Migration[5.0]
   include Gitlab::Database::MigrationHelpers
-  #include AfterCommitQueue
 
   DOWNTIME = false
-  MIGRATION = 'SyncIssuablesStateId'.freeze
-
-  # 2019-02-12 Gitlab.com issuable numbers
-  # issues count: 13587305
-  # merge requests count: 18925274
-  # Using this 25000 as batch size should take around 26 hours
-  # to migrate both issues and merge requests
-  BATCH_SIZE = 25000
-  DELAY_INTERVAL = 5.minutes.to_i
-
-  class Issue < ActiveRecord::Base
-    include EachBatch
-
-    self.table_name = 'issues'
-  end
-
-  class MergeRequest < ActiveRecord::Base
-    include EachBatch
-
-    self.table_name = 'merge_requests'
-  end
 
   def up
     add_column :issues, :state_id, :integer, limit: 2
     add_column :merge_requests, :state_id, :integer, limit: 2
-
-    # Is this safe?
-    # Added to avoid an warning about jobs running inside transactions.
-    # Since we only add a column this should be ok
-    Sidekiq::Worker.skipping_transaction_check do
-      queue_background_migration_jobs_by_range_at_intervals(Issue.where(state_id: nil), MIGRATION, DELAY_INTERVAL, batch_size: BATCH_SIZE)
-      queue_background_migration_jobs_by_range_at_intervals(MergeRequest.where(state_id: nil), MIGRATION, DELAY_INTERVAL, batch_size: BATCH_SIZE)
-    end
   end
 
   def down
diff --git a/db/post_migrate/20190214112022_schedule_sync_issuables_state_id.rb b/db/post_migrate/20190214112022_schedule_sync_issuables_state_id.rb
new file mode 100644
index 00000000000..d9b77d2f02c
--- /dev/null
+++ b/db/post_migrate/20190214112022_schedule_sync_issuables_state_id.rb
@@ -0,0 +1,43 @@
+# frozen_string_literal: true
+
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class ScheduleSyncIssuablesStateId < ActiveRecord::Migration[5.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  # 2019-02-12 Gitlab.com issuable numbers
+  # issues count: 13587305
+  # merge requests count: 18925274
+  # Using this 25000 as batch size should take around 26 hours
+  # to migrate both issues and merge requests
+  BATCH_SIZE = 25000
+  DELAY_INTERVAL = 5.minutes.to_i
+  ISSUE_MIGRATION = 'SyncIssuesStateId'.freeze
+  MERGE_REQUEST_MIGRATION = 'SyncMergeRequestsStateId'.freeze
+
+  class Issue < ActiveRecord::Base
+    include EachBatch
+
+    self.table_name = 'issues'
+  end
+
+  class MergeRequest < ActiveRecord::Base
+    include EachBatch
+
+    self.table_name = 'merge_requests'
+  end
+
+  def up
+    Sidekiq::Worker.skipping_transaction_check do
+      queue_background_migration_jobs_by_range_at_intervals(Issue.where(state_id: nil), ISSUE_MIGRATION, DELAY_INTERVAL, batch_size: BATCH_SIZE)
+      queue_background_migration_jobs_by_range_at_intervals(MergeRequest.where(state_id: nil), MERGE_REQUEST_MIGRATION, DELAY_INTERVAL, batch_size: BATCH_SIZE)
+    end
+  end
+
+  def down
+    # No op
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 0fb31ce2ef2..4a347e7289b 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20190211131150) do
+ActiveRecord::Schema.define(version: 20190214112022) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
diff --git a/lib/gitlab/background_migration/sync_issues_state_id.rb b/lib/gitlab/background_migration/sync_issues_state_id.rb
new file mode 100644
index 00000000000..33b997c8533
--- /dev/null
+++ b/lib/gitlab/background_migration/sync_issues_state_id.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+# rubocop:disable Style/Documentation
+
+module Gitlab
+  module BackgroundMigration
+    class SyncIssuesStateId
+      def perform(start_id, end_id)
+        Rails.logger.info("Issues - Populating state_id: #{start_id} - #{end_id}")
+
+        ActiveRecord::Base.connection.execute <<~SQL
+          UPDATE issues
+          SET state_id =
+            CASE state
+            WHEN 'opened' THEN 1
+            WHEN 'closed' THEN 2
+            END
+          WHERE state_id IS NULL
+          AND id BETWEEN #{start_id} AND #{end_id}
+        SQL
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/background_migration/sync_issuables_state_id.rb b/lib/gitlab/background_migration/sync_merge_requests_state_id.rb
index 95734a7310e..923ceaeec54 100644
--- a/lib/gitlab/background_migration/sync_issuables_state_id.rb
+++ b/lib/gitlab/background_migration/sync_merge_requests_state_id.rb
@@ -3,16 +3,12 @@
 
 module Gitlab
   module BackgroundMigration
-    class SyncIssuablesStateId
-      def perform(start_id, end_id, table_name)
-        populate_new_state_id(start_id, end_id, table_name)
-      end
-
-      def populate_new_state_id(start_id, end_id, table_name)
-        Rails.logger.info("#{table_name} - Populating state_id: #{start_id} - #{end_id}")
+    class SyncMergeRequestsStateId
+      def perform(start_id, end_id)
+        Rails.logger.info("Merge Requests - Populating state_id: #{start_id} - #{end_id}")
 
         ActiveRecord::Base.connection.execute <<~SQL
-          UPDATE #{table_name}
+          UPDATE merge_requests
           SET state_id =
             CASE state
             WHEN 'opened' THEN 1
diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb
index 46b36d07c20..3abd0600e9d 100644
--- a/lib/gitlab/database/migration_helpers.rb
+++ b/lib/gitlab/database/migration_helpers.rb
@@ -1029,12 +1029,11 @@ into similar problems in the future (e.g. when new tables are created).
 
         model_class.each_batch(of: batch_size) do |relation, index|
           start_id, end_id = relation.pluck('MIN(id), MAX(id)').first
-          table_name = relation.base_class.table_name
 
           # `BackgroundMigrationWorker.bulk_perform_in` schedules all jobs for
           # the same time, which is not helpful in most cases where we wish to
           # spread the work over time.
-          BackgroundMigrationWorker.perform_in(delay_interval * index, job_class_name, [start_id, end_id, table_name])
+          BackgroundMigrationWorker.perform_in(delay_interval * index, job_class_name, [start_id, end_id])
         end
       end
 
diff --git a/spec/migrations/add_state_id_to_issuables_spec.rb b/spec/migrations/schedule_sync_issuables_state_id_spec.rb
index b0e285db1f3..a926ee38387 100644
--- a/spec/migrations/add_state_id_to_issuables_spec.rb
+++ b/spec/migrations/schedule_sync_issuables_state_id_spec.rb
@@ -1,9 +1,9 @@
 # frozen_string_literal: true
 
 require 'spec_helper'
-require Rails.root.join('db', 'migrate', '20190211131150_add_state_id_to_issuables.rb')
+require Rails.root.join('db', 'post_migrate', '20190214112022_schedule_sync_issuables_state_id.rb')
 
-describe AddStateIdToIssuables, :migration do
+describe ScheduleSyncIssuablesStateId, :migration do
   let(:namespaces) { table(:namespaces) }
   let(:projects) { table(:projects) }
   let(:merge_requests) { table(:merge_requests) }
@@ -25,8 +25,8 @@ describe AddStateIdToIssuables, :migration do
 
         migrate!
 
-        expect(opened_issue.reload.state_id).to eq(Issue.states.opened)
-        expect(closed_issue.reload.state_id).to eq(Issue.states.closed)
+        expect(opened_issue.reload.state_id).to eq(Issue.available_states[:opened])
+        expect(closed_issue.reload.state_id).to eq(Issue.available_states[:closed])
         expect(invalid_state_issue.reload.state_id).to be_nil
         expect(nil_state_issue.reload.state_id).to be_nil
       end
@@ -42,10 +42,10 @@ describe AddStateIdToIssuables, :migration do
 
         migrate!
 
-        expect(opened_merge_request.reload.state_id).to eq(MergeRequest.states.opened)
-        expect(closed_merge_request.reload.state_id).to eq(MergeRequest.states.closed)
-        expect(merged_merge_request.reload.state_id).to eq(MergeRequest.states.merged)
-        expect(locked_merge_request.reload.state_id).to eq(MergeRequest.states.locked)
+        expect(opened_merge_request.reload.state_id).to eq(MergeRequest.available_states[:opened])
+        expect(closed_merge_request.reload.state_id).to eq(MergeRequest.available_states[:closed])
+        expect(merged_merge_request.reload.state_id).to eq(MergeRequest.available_states[:merged])
+        expect(locked_merge_request.reload.state_id).to eq(MergeRequest.available_states[:locked])
         expect(invalid_state_merge_request.reload.state_id).to be_nil
       end
     end