Commit b80893c2 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'issue_57906_fix_github_import' into 'master'

Fix issuables state_id nil when importing projects from GitHub

Closes #57906

See merge request gitlab-org/gitlab-ce!28027
parents adc83d25 c40bad74
---
title: Fix issuables state_id nil when importing projects from GitHub
merge_request: 28027
author:
type: fixed
# 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
# 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
...@@ -10,7 +10,7 @@ ...@@ -10,7 +10,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # 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 # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
......
...@@ -201,6 +201,7 @@ module Gitlab ...@@ -201,6 +201,7 @@ module Gitlab
target_branch: Gitlab::Git.ref_name(pull_request.target_branch_name), target_branch: Gitlab::Git.ref_name(pull_request.target_branch_name),
target_branch_sha: pull_request.target_branch_sha, target_branch_sha: pull_request.target_branch_sha,
state: pull_request.state, state: pull_request.state,
state_id: MergeRequest.available_states[pull_request.state],
author_id: author_id, author_id: author_id,
assignee_id: nil, assignee_id: nil,
created_at: pull_request.created_at, created_at: pull_request.created_at,
......
...@@ -53,6 +53,7 @@ module Gitlab ...@@ -53,6 +53,7 @@ module Gitlab
description: description, description: description,
milestone_id: milestone_finder.id_for(issue), milestone_id: milestone_finder.id_for(issue),
state: issue.state, state: issue.state,
state_id: ::Issue.available_states[issue.state],
created_at: issue.created_at, created_at: issue.created_at,
updated_at: issue.updated_at updated_at: issue.updated_at
} }
......
...@@ -55,6 +55,7 @@ module Gitlab ...@@ -55,6 +55,7 @@ module Gitlab
source_branch: pull_request.formatted_source_branch, source_branch: pull_request.formatted_source_branch,
target_branch: pull_request.target_branch, target_branch: pull_request.target_branch,
state: pull_request.state, state: pull_request.state,
state_id: ::MergeRequest.available_states[pull_request.state],
milestone_id: milestone_finder.id_for(pull_request), milestone_id: milestone_finder.id_for(pull_request),
author_id: author_id, author_id: author_id,
assignee_id: user_finder.assignee_id_for(pull_request), assignee_id: user_finder.assignee_id_for(pull_request),
......
...@@ -105,6 +105,7 @@ describe Gitlab::BitbucketServerImport::Importer do ...@@ -105,6 +105,7 @@ describe Gitlab::BitbucketServerImport::Importer do
expect(merge_request.metrics.merged_by).to eq(project.owner) 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.metrics.merged_at).to eq(@merge_event.merge_timestamp)
expect(merge_request.merge_commit_sha).to eq('12345678') expect(merge_request.merge_commit_sha).to eq('12345678')
expect(merge_request.state_id).to eq(3)
end end
it 'imports comments' do it 'imports comments' do
......
...@@ -98,6 +98,7 @@ describe Gitlab::GithubImport::Importer::IssueImporter, :clean_gitlab_redis_cach ...@@ -98,6 +98,7 @@ describe Gitlab::GithubImport::Importer::IssueImporter, :clean_gitlab_redis_cach
description: 'This is my issue', description: 'This is my issue',
milestone_id: milestone.id, milestone_id: milestone.id,
state: :opened, state: :opened,
state_id: 1,
created_at: created_at, created_at: created_at,
updated_at: updated_at updated_at: updated_at
}, },
...@@ -127,6 +128,7 @@ describe Gitlab::GithubImport::Importer::IssueImporter, :clean_gitlab_redis_cach ...@@ -127,6 +128,7 @@ describe Gitlab::GithubImport::Importer::IssueImporter, :clean_gitlab_redis_cach
description: "*Created by: alice*\n\nThis is my issue", description: "*Created by: alice*\n\nThis is my issue",
milestone_id: milestone.id, milestone_id: milestone.id,
state: :opened, state: :opened,
state_id: 1,
created_at: created_at, created_at: created_at,
updated_at: updated_at updated_at: updated_at
}, },
......
...@@ -93,6 +93,7 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi ...@@ -93,6 +93,7 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi
source_branch: 'github/fork/alice/feature', source_branch: 'github/fork/alice/feature',
target_branch: 'master', target_branch: 'master',
state: :merged, state: :merged,
state_id: 3,
milestone_id: milestone.id, milestone_id: milestone.id,
author_id: user.id, author_id: user.id,
assignee_id: user.id, assignee_id: user.id,
...@@ -138,6 +139,7 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi ...@@ -138,6 +139,7 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi
source_branch: 'github/fork/alice/feature', source_branch: 'github/fork/alice/feature',
target_branch: 'master', target_branch: 'master',
state: :merged, state: :merged,
state_id: 3,
milestone_id: milestone.id, milestone_id: milestone.id,
author_id: project.creator_id, author_id: project.creator_id,
assignee_id: user.id, assignee_id: user.id,
...@@ -184,6 +186,7 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi ...@@ -184,6 +186,7 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi
source_branch: 'master-42', source_branch: 'master-42',
target_branch: 'master', target_branch: 'master',
state: :merged, state: :merged,
state_id: 3,
milestone_id: milestone.id, milestone_id: milestone.id,
author_id: user.id, author_id: user.id,
assignee_id: user.id, assignee_id: user.id,
......
# 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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment