Migrate issuable states to integer patch 1

Patch 1 that migrates issues/merge requests states from integer to string.
On this commit we are only adding the state_id column and syncing it with a backgroud migration.

On Patch 2 the code to use the new integer column will be deployed and the old column will be
removed.

app/models/concerns/issuable.rb

@@ -23,6 +23,7 @@ module Issuable
   include Sortable
   include CreatedAtFilterable
   include UpdatedAtFilterable
+  include IssuableStates
 
   # This object is used to gather issuable meta data for displaying
   # upvotes, downvotes, notes and closing merge requests count for issues and merge requests

app/models/concerns/issuable_states.rb

@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+# == IssuableStates concern
+#
+# Defines statuses shared by issuables which are persisted on state column
+# using the state machine.
+#
+# Used by EE::Epic, Issue and MergeRequest
+#
+module IssuableStates
+  extend ActiveSupport::Concern
+
+  # Override this constant on model where different states are needed
+  # Check MergeRequest::AVAILABLE_STATES
+  AVAILABLE_STATES = { opened: 1, closed: 2 }.freeze
+
+  included do
+    before_save :set_state_id
+  end
+
+  class_methods do
+    def states
+      @states ||= OpenStruct.new(self::AVAILABLE_STATES)
+    end
+  end
+
+  # The state:string column is being migrated to state_id:integer column
+  # This is a temporary hook to populate state_id column with new values
+  # and can be removed after the complete migration is done.
+  def set_state_id
+    return if state.nil? || state.empty?
+
+    states_hash = self.class.states.to_h.with_indifferent_access
+
+    self.state_id = states_hash[state]
+  end
+end

app/models/merge_request.rb

@@ -22,6 +22,8 @@ class MergeRequest < ActiveRecord::Base
   self.reactive_cache_lifetime = 10.minutes
 
   SORTING_PREFERENCE_FIELD = :merge_requests_sort
+  MERGE_REQUEST_STATES =
+  AVAILABLE_STATES = AVAILABLE_STATES.merge(merged: 3, locked: 4).freeze
 
   ignore_column :locked_at,
                 :ref_fetched,

db/migrate/20190211131150_add_state_id_to_issuables.rb

@@ -0,0 +1,37 @@
+class AddStateIdToIssuables < ActiveRecord::Migration[5.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  MIGRATION = 'SyncIssuablesStateId'.freeze
+
+  # TODO - find out how many issues and merge requests in production
+  # to adapt the batch size and delay interval
+  # Keep in mind that the migration will be scheduled for issues and merge requests.
+  BATCH_SIZE = 5000
+  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: 1
+    add_column :merge_requests, :state_id, :integer, limit: 1
+
+    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
+
+  def down
+    remove_column :issues, :state_id
+    remove_column :merge_requests, :state_id
+  end
+end

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: 20190204115450) do
+ActiveRecord::Schema.define(version: 20190211131150) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -1046,6 +1046,7 @@ ActiveRecord::Schema.define(version: 20190204115450) do
     t.boolean "discussion_locked"
     t.datetime_with_timezone "closed_at"
     t.integer "closed_by_id"
+    t.integer "state_id", limit: 2
     t.index ["author_id"], name: "index_issues_on_author_id", using: :btree
     t.index ["closed_by_id"], name: "index_issues_on_closed_by_id", using: :btree
     t.index ["confidential"], name: "index_issues_on_confidential", using: :btree
@@ -1283,6 +1284,7 @@ ActiveRecord::Schema.define(version: 20190204115450) do
     t.string "rebase_commit_sha"
     t.boolean "squash", default: false, null: false
     t.boolean "allow_maintainer_to_push"
+    t.integer "state_id", limit: 2
     t.index ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree
     t.index ["author_id"], name: "index_merge_requests_on_author_id", using: :btree
     t.index ["created_at"], name: "index_merge_requests_on_created_at", using: :btree

lib/gitlab/background_migration/sync_issuables_state_id.rb

@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+# rubocop:disable Style/Documentation
+
+module Gitlab
+  module BackgroundMigration
+    class SyncIssuablesStateId
+      def perform(start_id, end_id, model_class)
+        populate_new_state_id(start_id, end_id, model_class)
+      end
+
+      def populate_new_state_id(start_id, end_id, model_class)
+        Rails.logger.info("#{model_class.model_name.human} - Populating state_id: #{start_id} - #{end_id}")
+
+        ActiveRecord::Base.connection.execute <<~SQL
+          UPDATE #{model_class.table_name}
+          SET state_id =
+            CASE state
+            WHEN 'opened' THEN 1
+            WHEN 'closed' THEN 2
+            WHEN 'merged' THEN 3
+            WHEN 'locked' THEN 4
+            END
+          WHERE state_id IS NULL
+          AND id BETWEEN #{start_id} AND #{end_id}
+        SQL
+      end
+    end
+  end
+end

lib/gitlab/database/migration_helpers.rb

@@ -1033,7 +1033,7 @@ into similar problems in the future (e.g. when new tables are created).
           # `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])
+          BackgroundMigrationWorker.perform_in(delay_interval * index, job_class_name, [start_id, end_id, model_class])
         end
       end
 

spec/migrations/add_state_id_to_issuables_spec.rb

@@ -0,0 +1,89 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'migrate', '20190206144959_change_issuable_states_to_integer.rb')
+
+describe AddStateIdToIssuables, :migration do
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:merge_requests) { table(:merge_requests) }
+  let(:issues) { table(:issues) }
+  let(:migration) { described_class.new }
+
+  before do
+    @group = namespaces.create!(name: 'gitlab', path: 'gitlab')
+    @project = projects.create!(namespace_id: @group.id)
+  end
+
+  describe '#up' do
+    context 'issues' do
+      it 'migrates state column to integer' do
+        opened_issue = issues.create!(description: 'first', state: 'opened')
+        closed_issue = issues.create!(description: 'second', state: 'closed')
+        nil_state_issue = issues.create!(description: 'third', state: nil)
+
+        migrate!
+
+        issues.reset_column_information
+        expect(opened_issue.reload.state).to eq(Issue.states.opened)
+        expect(closed_issue.reload.state).to eq(Issue.states.closed)
+        expect(nil_state_issue.reload.state).to eq(nil)
+      end
+    end
+
+    context 'merge requests' do
+      it 'migrates state column to integer' do
+        opened_merge_request = merge_requests.create!(state: 'opened', target_project_id: @project.id, target_branch: 'feature1', source_branch: 'master')
+        closed_merge_request = merge_requests.create!(state: 'closed', target_project_id: @project.id, target_branch: 'feature2', source_branch: 'master')
+        merged_merge_request = merge_requests.create!(state: 'merged', target_project_id: @project.id, target_branch: 'feature3', source_branch: 'master')
+        locked_merge_request = merge_requests.create!(state: 'locked', target_project_id: @project.id, target_branch: 'feature4', source_branch: 'master')
+        nil_state_merge_request = merge_requests.create!(state: nil, target_project_id: @project.id, target_branch: 'feature5', source_branch: 'master')
+
+        migrate!
+
+        merge_requests.reset_column_information
+        expect(opened_merge_request.reload.state).to eq(MergeRequest.states.opened)
+        expect(closed_merge_request.reload.state).to eq(MergeRequest.states.closed)
+        expect(merged_merge_request.reload.state).to eq(MergeRequest.states.merged)
+        expect(locked_merge_request.reload.state).to eq(MergeRequest.states.locked)
+        expect(nil_state_merge_request.reload.state).to eq(nil)
+      end
+    end
+  end
+
+  describe '#down' do
+    context 'issues' do
+      it 'migrates state column to string' do
+        opened_issue = issues.create!(description: 'first', state: 1)
+        closed_issue = issues.create!(description: 'second', state: 2)
+        nil_state_issue = issues.create!(description: 'third', state: nil)
+
+        migration.down
+
+        issues.reset_column_information
+        expect(opened_issue.reload.state).to eq('opened')
+        expect(closed_issue.reload.state).to eq('closed')
+        expect(nil_state_issue.reload.state).to eq(nil)
+      end
+    end
+
+    context 'merge requests' do
+      it 'migrates state column to string' do
+        opened_merge_request = merge_requests.create!(state: 1, target_project_id: @project.id, target_branch: 'feature1', source_branch: 'master')
+        closed_merge_request = merge_requests.create!(state: 2, target_project_id: @project.id, target_branch: 'feature2', source_branch: 'master')
+        merged_merge_request = merge_requests.create!(state: 3, target_project_id: @project.id, target_branch: 'feature3', source_branch: 'master')
+        locked_merge_request = merge_requests.create!(state: 4, target_project_id: @project.id, target_branch: 'feature4', source_branch: 'master')
+        nil_state_merge_request = merge_requests.create!(state: nil, target_project_id: @project.id, target_branch: 'feature5', source_branch: 'master')
+
+        migration.down
+
+        merge_requests.reset_column_information
+        expect(opened_merge_request.reload.state).to eq('opened')
+        expect(closed_merge_request.reload.state).to eq('closed')
+        expect(merged_merge_request.reload.state).to eq('merged')
+        expect(locked_merge_request.reload.state).to eq('locked')
+        expect(nil_state_merge_request.reload.state).to eq(nil)
+      end
+    end
+  end
+end