Bitbucket Server importer: Eliminate most idle-in-transaction issues
Just like with the GitHub importer, the Bitbucket Server importer can hit the default 60 s idle-in-transaction timeouts if it takes too long to create the merge request. We solve this by using the same approach as the GitHub importer: 1. Bypass all validation and hooks in creating a merge request 2. Insert the Git data in a separate transaction Part of #50021
This commit is contained in:
parent
452fd703f3
commit
09cdd7dca0
|
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
title: 'Bitbucket Server importer: Eliminate most idle-in-transaction issues'
|
||||||
|
merge_request:
|
||||||
|
author:
|
||||||
|
type: performance
|
||||||
|
|
@ -1,7 +1,10 @@
|
||||||
|
# frozen_string_literal: true
|
||||||
|
|
||||||
module Gitlab
|
module Gitlab
|
||||||
module BitbucketServerImport
|
module BitbucketServerImport
|
||||||
class Importer
|
class Importer
|
||||||
include Gitlab::ShellAdapter
|
include Gitlab::ShellAdapter
|
||||||
|
|
||||||
attr_reader :recover_missing_commits
|
attr_reader :recover_missing_commits
|
||||||
attr_reader :project, :project_key, :repository_slug, :client, :errors, :users
|
attr_reader :project, :project_key, :repository_slug, :client, :errors, :users
|
||||||
|
|
||||||
|
|
@ -175,21 +178,18 @@ module Gitlab
|
||||||
description = ''
|
description = ''
|
||||||
description += @formatter.author_line(pull_request.author) unless find_user_id(pull_request.author_email)
|
description += @formatter.author_line(pull_request.author) unless find_user_id(pull_request.author_email)
|
||||||
description += pull_request.description if pull_request.description
|
description += pull_request.description if pull_request.description
|
||||||
|
|
||||||
source_branch_sha = pull_request.source_branch_sha
|
|
||||||
target_branch_sha = pull_request.target_branch_sha
|
|
||||||
author_id = gitlab_user_id(pull_request.author_email)
|
author_id = gitlab_user_id(pull_request.author_email)
|
||||||
|
|
||||||
attributes = {
|
attributes = {
|
||||||
iid: pull_request.iid,
|
iid: pull_request.iid,
|
||||||
title: pull_request.title,
|
title: pull_request.title,
|
||||||
description: description,
|
description: description,
|
||||||
source_project: project,
|
source_project_id: project.id,
|
||||||
source_branch: Gitlab::Git.ref_name(pull_request.source_branch_name),
|
source_branch: Gitlab::Git.ref_name(pull_request.source_branch_name),
|
||||||
source_branch_sha: source_branch_sha,
|
source_branch_sha: pull_request.source_branch_sha,
|
||||||
target_project: project,
|
target_project_id: project.id,
|
||||||
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: target_branch_sha,
|
target_branch_sha: pull_request.target_branch_sha,
|
||||||
state: pull_request.state,
|
state: pull_request.state,
|
||||||
author_id: author_id,
|
author_id: author_id,
|
||||||
assignee_id: nil,
|
assignee_id: nil,
|
||||||
|
|
@ -197,7 +197,9 @@ module Gitlab
|
||||||
updated_at: pull_request.updated_at
|
updated_at: pull_request.updated_at
|
||||||
}
|
}
|
||||||
|
|
||||||
merge_request = project.merge_requests.create!(attributes)
|
creator = Gitlab::Import::MergeRequestCreator.new(project)
|
||||||
|
merge_request = creator.execute(attributes)
|
||||||
|
|
||||||
import_pull_request_comments(pull_request, merge_request) if merge_request.persisted?
|
import_pull_request_comments(pull_request, merge_request) if merge_request.persisted?
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,40 @@
|
||||||
|
# frozen_string_literal: true
|
||||||
|
|
||||||
|
# This module is designed for importers that need to create many merge
|
||||||
|
# requests quickly. When creating merge requests there are a lot of hooks
|
||||||
|
# that may run, for many different reasons. Many of these hooks (e.g. the ones
|
||||||
|
# used for rendering Markdown) are completely unnecessary and may even lead to
|
||||||
|
# transaction timeouts.
|
||||||
|
#
|
||||||
|
# To ensure importing merge requests requests has a minimal impact and can
|
||||||
|
# complete in a reasonable time we bypass all the hooks by inserting the row
|
||||||
|
# and then retrieving it. We then only perform the additional work that is
|
||||||
|
# strictly necessary.
|
||||||
|
module Gitlab
|
||||||
|
module Import
|
||||||
|
class MergeRequestCreator
|
||||||
|
include ::Gitlab::Import::DatabaseHelpers
|
||||||
|
include ::Gitlab::Import::MergeRequestHelpers
|
||||||
|
|
||||||
|
attr_accessor :project
|
||||||
|
|
||||||
|
def initialize(project)
|
||||||
|
@project = project
|
||||||
|
end
|
||||||
|
|
||||||
|
def execute(attributes)
|
||||||
|
source_branch_sha = attributes.delete(:source_branch_sha)
|
||||||
|
target_branch_sha = attributes.delete(:target_branch_sha)
|
||||||
|
iid = attributes[:iid]
|
||||||
|
|
||||||
|
merge_request, already_exists = create_merge_request_without_hooks(project, attributes, iid)
|
||||||
|
|
||||||
|
if merge_request
|
||||||
|
insert_or_replace_git_data(merge_request, source_branch_sha, target_branch_sha, already_exists)
|
||||||
|
end
|
||||||
|
|
||||||
|
merge_request
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
@ -0,0 +1,43 @@
|
||||||
|
# frozen_string_literal: true
|
||||||
|
|
||||||
|
require 'spec_helper'
|
||||||
|
|
||||||
|
describe Gitlab::Import::MergeRequestCreator do
|
||||||
|
let(:project) { create(:project, :repository) }
|
||||||
|
|
||||||
|
subject { described_class.new(project) }
|
||||||
|
|
||||||
|
describe '#execute' do
|
||||||
|
context 'merge request already exists' do
|
||||||
|
let(:merge_request) { create(:merge_request, target_project: project, source_project: project) }
|
||||||
|
let(:commits) { merge_request.merge_request_diffs.first.commits }
|
||||||
|
let(:attributes) { HashWithIndifferentAccess.new(merge_request.attributes) }
|
||||||
|
|
||||||
|
it 'updates the data' do
|
||||||
|
commits_count = commits.count
|
||||||
|
merge_request.merge_request_diffs.destroy_all # rubocop: disable DestroyAll
|
||||||
|
|
||||||
|
expect(merge_request.merge_request_diffs.count).to eq(0)
|
||||||
|
|
||||||
|
subject.execute(attributes)
|
||||||
|
|
||||||
|
expect(merge_request.reload.merge_request_diffs.count).to eq(1)
|
||||||
|
expect(merge_request.reload.merge_request_diffs.first.commits.count).to eq(commits_count)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'new merge request' do
|
||||||
|
let(:merge_request) { build(:merge_request, target_project: project, source_project: project) }
|
||||||
|
let(:attributes) { HashWithIndifferentAccess.new(merge_request.attributes) }
|
||||||
|
|
||||||
|
it 'creates a new merge request' do
|
||||||
|
attributes.delete(:id)
|
||||||
|
|
||||||
|
expect { subject.execute(attributes) }.to change { MergeRequest.count }.by(1)
|
||||||
|
|
||||||
|
new_mr = MergeRequest.last
|
||||||
|
expect(new_mr.merge_request_diffs.count).to eq(1)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
Loading…
Reference in New Issue