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

4 files changed, 98 insertions, 8 deletions

diff --git a/changelogs/unreleased/sh-insert-git-data-in-separate-transaction.yml b/changelogs/unreleased/sh-insert-git-data-in-separate-transaction.yml
new file mode 100644
index 00000000000..116929b2f53
--- /dev/null
+++ b/changelogs/unreleased/sh-insert-git-data-in-separate-transaction.yml
@@ -0,0 +1,5 @@
+---
+title: 'Bitbucket Server importer: Eliminate most idle-in-transaction issues'
+merge_request:
+author:
+type: performance
diff --git a/lib/gitlab/bitbucket_server_import/importer.rb b/lib/gitlab/bitbucket_server_import/importer.rb
index 268d21a77d1..b591d94668f 100644
--- a/lib/gitlab/bitbucket_server_import/importer.rb
+++ b/lib/gitlab/bitbucket_server_import/importer.rb
@@ -1,7 +1,10 @@
+# frozen_string_literal: true
+
 module Gitlab
   module BitbucketServerImport
     class Importer
       include Gitlab::ShellAdapter
+
       attr_reader :recover_missing_commits
       attr_reader :project, :project_key, :repository_slug, :client, :errors, :users
 
@@ -175,21 +178,18 @@ module Gitlab
         description = ''
         description += @formatter.author_line(pull_request.author) unless find_user_id(pull_request.author_email)
         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)
 
         attributes = {
           iid: pull_request.iid,
           title: pull_request.title,
           description: description,
-          source_project: project,
+          source_project_id: project.id,
           source_branch: Gitlab::Git.ref_name(pull_request.source_branch_name),
-          source_branch_sha: source_branch_sha,
-          target_project: project,
+          source_branch_sha: pull_request.source_branch_sha,
+          target_project_id: project.id,
           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,
           author_id: author_id,
           assignee_id: nil,
@@ -197,7 +197,9 @@ module Gitlab
           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?
       end
 
diff --git a/lib/gitlab/import/merge_request_creator.rb b/lib/gitlab/import/merge_request_creator.rb
new file mode 100644
index 00000000000..a01951b0762
--- /dev/null
+++ b/lib/gitlab/import/merge_request_creator.rb
@@ -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
diff --git a/spec/lib/gitlab/import/merge_request_creator_spec.rb b/spec/lib/gitlab/import/merge_request_creator_spec.rb
new file mode 100644
index 00000000000..cd3359da341
--- /dev/null
+++ b/spec/lib/gitlab/import/merge_request_creator_spec.rb
@@ -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