Automatic merge of gitlab-org/gitlab-ce master

changelogs/unreleased/issue_57906_fix_github_import.yml

+---
+title: Fix issuables state_id nil when importing projects from GitHub
+merge_request: 28027
+author:
+type: fixed

db/migrate/20190506135337_add_temporary_indexes_to_state_id.rb

+# frozen_string_literal: true
+
+# This migration adds temporary indexes to state_id column of issues
+# and merge_requests tables. It will be used only to peform the scheduling
+# for populating state_id in a post migrate and will be removed after it.
+# Check: ScheduleSyncIssuablesStateIdWhereNil.
+
+class AddTemporaryIndexesToStateId < ActiveRecord::Migration[5.1]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    %w(issues merge_requests).each do |table|
+      add_concurrent_index(
+        table,
+        'id',
+        name: index_name_for(table),
+        where: "state_id IS NULL"
+      )
+    end
+  end
+
+  def down
+    remove_concurrent_index_by_name(:issues, index_name_for("issues"))
+    remove_concurrent_index_by_name(:merge_requests, index_name_for("merge_requests"))
+  end
+
+  def index_name_for(table)
+    "idx_on_#{table}_where_state_id_is_null"
+  end
+end

db/post_migrate/20190506135400_schedule_sync_issuables_state_id_where_nil.rb

+# frozen_string_literal: true
+
+class ScheduleSyncIssuablesStateIdWhereNil < ActiveRecord::Migration[5.1]
+  # Issues and MergeRequests imported by GitHub are being created with
+  # state_id = null, this fixes them.
+  #
+  # Part of a bigger plan: https://gitlab.com/gitlab-org/gitlab-ce/issues/51789
+
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  # 2019-05-02 gitlab.com issuable numbers
+  # issues with state_id nil: ~40000
+  # merge requests with state_id nil: ~200000
+  #
+  # Using 5000 as batch size and 120 seconds interval will create:
+  # ~8 jobs for issues - taking ~16 minutes
+  # ~40 jobs for merge requests - taking ~1.34 hours
+  #
+  BATCH_SIZE = 5000
+  DELAY_INTERVAL = 120.seconds.to_i
+  ISSUES_MIGRATION = 'SyncIssuesStateId'.freeze
+  MERGE_REQUESTS_MIGRATION = 'SyncMergeRequestsStateId'.freeze
+
+  disable_ddl_transaction!
+
+  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
+    queue_background_migration_jobs_by_range_at_intervals(
+      Issue.where(state_id: nil),
+      ISSUES_MIGRATION,
+      DELAY_INTERVAL,
+      batch_size: BATCH_SIZE
+    )
+
+    queue_background_migration_jobs_by_range_at_intervals(
+      MergeRequest.where(state_id: nil),
+      MERGE_REQUESTS_MIGRATION,
+      DELAY_INTERVAL,
+      batch_size: BATCH_SIZE
+    )
+
+    # Remove temporary indexes added on "AddTemporaryIndexesToStateId"
+    remove_concurrent_index_by_name(:issues, "idx_on_issues_where_state_id_is_null")
+    remove_concurrent_index_by_name(:merge_requests, "idx_on_merge_requests_where_state_id_is_null")
+  end
+
+  def down
+    # No op
+  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: 20190426180107) do
+ActiveRecord::Schema.define(version: 20190506135400) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"

lib/gitlab/bitbucket_server_import/importer.rb

@@ -201,6 +201,7 @@ module Gitlab
           target_branch: Gitlab::Git.ref_name(pull_request.target_branch_name),
           target_branch_sha: pull_request.target_branch_sha,
           state: pull_request.state,
+          state_id: MergeRequest.available_states[pull_request.state],
           author_id: author_id,
           assignee_id: nil,
           created_at: pull_request.created_at,

lib/gitlab/github_import/importer/issue_importer.rb

@@ -53,6 +53,7 @@ module Gitlab
             description: description,
             milestone_id: milestone_finder.id_for(issue),
             state: issue.state,
+            state_id: ::Issue.available_states[issue.state],
             created_at: issue.created_at,
             updated_at: issue.updated_at
           }

lib/gitlab/github_import/importer/pull_request_importer.rb

@@ -55,6 +55,7 @@ module Gitlab
             source_branch: pull_request.formatted_source_branch,
             target_branch: pull_request.target_branch,
             state: pull_request.state,
+            state_id: ::MergeRequest.available_states[pull_request.state],
             milestone_id: milestone_finder.id_for(pull_request),
             author_id: author_id,
             assignee_id: user_finder.assignee_id_for(pull_request),

spec/lib/gitlab/bitbucket_server_import/importer_spec.rb

@@ -105,6 +105,7 @@ describe Gitlab::BitbucketServerImport::Importer do
       expect(merge_request.metrics.merged_by).to eq(project.owner)
       expect(merge_request.metrics.merged_at).to eq(@merge_event.merge_timestamp)
       expect(merge_request.merge_commit_sha).to eq('12345678')
+      expect(merge_request.state_id).to eq(3)
     end
 
     it 'imports comments' do

spec/lib/gitlab/github_import/importer/issue_importer_spec.rb

@@ -98,6 +98,7 @@ describe Gitlab::GithubImport::Importer::IssueImporter, :clean_gitlab_redis_cach
               description: 'This is my issue',
               milestone_id: milestone.id,
               state: :opened,
+              state_id: 1,
               created_at: created_at,
               updated_at: updated_at
             },
@@ -127,6 +128,7 @@ describe Gitlab::GithubImport::Importer::IssueImporter, :clean_gitlab_redis_cach
               description: "*Created by: alice*\n\nThis is my issue",
               milestone_id: milestone.id,
               state: :opened,
+              state_id: 1,
               created_at: created_at,
               updated_at: updated_at
             },

spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb

@@ -93,6 +93,7 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi
               source_branch: 'github/fork/alice/feature',
               target_branch: 'master',
               state: :merged,
+              state_id: 3,
               milestone_id: milestone.id,
               author_id: user.id,
               assignee_id: user.id,
@@ -138,6 +139,7 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi
               source_branch: 'github/fork/alice/feature',
               target_branch: 'master',
               state: :merged,
+              state_id: 3,
               milestone_id: milestone.id,
               author_id: project.creator_id,
               assignee_id: user.id,
@@ -184,6 +186,7 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi
               source_branch: 'master-42',
               target_branch: 'master',
               state: :merged,
+              state_id: 3,
               milestone_id: milestone.id,
               author_id: user.id,
               assignee_id: user.id,

spec/migrations/schedule_sync_issuables_state_id_where_nil_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20190506135400_schedule_sync_issuables_state_id_where_nil')
+
+describe ScheduleSyncIssuablesStateIdWhereNil, :migration, :sidekiq do
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:merge_requests) { table(:merge_requests) }
+  let(:issues) { table(:issues) }
+  let(:migration) { described_class.new }
+  let(:group) { namespaces.create!(name: 'gitlab', path: 'gitlab') }
+  let(:project) { projects.create!(namespace_id: group.id) }
+
+  shared_examples 'scheduling migrations' do
+    before do
+      Sidekiq::Worker.clear_all
+      stub_const("#{described_class.name}::BATCH_SIZE", 2)
+    end
+
+    it 'correctly schedules issuable sync background migration' do
+      Sidekiq::Testing.fake! do
+        Timecop.freeze do
+          migrate!
+
+          expect(migration).to be_scheduled_delayed_migration(120.seconds, resource_1.id, resource_3.id)
+          expect(migration).to be_scheduled_delayed_migration(240.seconds, resource_5.id, resource_5.id)
+          expect(BackgroundMigrationWorker.jobs.size).to eq(2)
+        end
+      end
+    end
+  end
+
+  describe '#up' do
+    context 'issues' do
+      it_behaves_like 'scheduling migrations' do
+        let(:migration) { described_class::ISSUES_MIGRATION }
+        let!(:resource_1) { issues.create!(description: 'first', state: 'opened', state_id: nil) }
+        let!(:resource_2) { issues.create!(description: 'second', state: 'closed', state_id: 2) }
+        let!(:resource_3) { issues.create!(description: 'third', state: 'closed', state_id: nil) }
+        let!(:resource_4) { issues.create!(description: 'fourth', state: 'closed', state_id: 2) }
+        let!(:resource_5) { issues.create!(description: 'fifth', state: 'closed', state_id: nil) }
+      end
+    end
+
+    context 'merge requests' do
+      it_behaves_like 'scheduling migrations' do
+        let(:migration) { described_class::MERGE_REQUESTS_MIGRATION }
+        let!(:resource_1) { merge_requests.create!(state: 'opened', state_id: nil, target_project_id: project.id, target_branch: 'feature1', source_branch: 'master') }
+        let!(:resource_2) { merge_requests.create!(state: 'closed', state_id: 2, target_project_id: project.id, target_branch: 'feature2', source_branch: 'master') }
+        let!(:resource_3) { merge_requests.create!(state: 'merged', state_id: nil, target_project_id: project.id, target_branch: 'feature3', source_branch: 'master') }
+        let!(:resource_4) { merge_requests.create!(state: 'locked', state_id: 3, target_project_id: project.id, target_branch: 'feature4', source_branch: 'master') }
+        let!(:resource_5) { merge_requests.create!(state: 'locked', state_id: nil, target_project_id: project.id, target_branch: 'feature4', source_branch: 'master') }
+      end
+    end
+  end
+end