Stop creating keep-arounds on merge base commits

Keep-arounds work by ensuring a commit is referred by a reference and so it
will not be considered orphaned. The commit that the keep-around
references also references the commits that it was based off. So by
extension those commits will also be kept. Since the head commit is kept
and since merge base commits are by definition upstream of the head
commit, this means that adding a keep-around on a merge base commit is
always superfluous.

Changelog: performance

app/models/merge_request_diff.rb

@@ -882,8 +882,13 @@ def find_base_sha
   end
 
   def keep_around_commits
+    # The merge head keeps track of what an actual merge might look like. The
+    # referenced merge is temporary and so is kept alive with
+    # MergeRequest#merge_ref_path which is updated as required.
+    return if merge_head?
+
     [repository, merge_request.source_project.repository].uniq.each do |repo|
-      repo.keep_around(start_commit_sha, head_commit_sha, base_commit_sha, source: self.class.name)
+      repo.keep_around(start_commit_sha, head_commit_sha, source: self.class.name)
     end
   end
 

lib/tasks/gitlab/keep_around.rake

@@ -58,12 +58,11 @@ namespace :gitlab do
       merge_requests = MergeRequest.from_and_to_forks(project)
       merge_request_diffs = MergeRequestDiff
         .joins(:merge_request).merge(merge_requests)
-        .select(:id, :start_commit_sha, :head_commit_sha, :base_commit_sha)
+        .select(:id, :start_commit_sha, :head_commit_sha)
 
-      merge_request_diffs.find_each do |diff|
+      merge_request_diffs.where.not(diff_type: :merge_head).find_each do |diff|
         add_match(csv, diff.start_commit_sha)
         add_match(csv, diff.head_commit_sha)
-        add_match(csv, diff.base_commit_sha)
       end
     end
 

spec/models/merge_request_diff_spec.rb

@@ -44,6 +44,10 @@
   describe 'create new record' do
     subject { diff_with_commits }
 
+    before do
+      allow(Gitlab::Git::KeepAround).to receive(:execute).and_call_original
+    end
+
     it { expect(subject).to be_valid }
     it { expect(subject).to be_persisted }
     it { expect(subject.commits.count).to eq(29) }
@@ -61,10 +65,20 @@
       merge_request.create_merge_request_diff
     end
 
+    it 'creates hidden refs' do
+      hidden_refs = subject.project.repository.raw.list_refs(["refs/#{Repository::REF_MERGE_REQUEST}/", "refs/#{Repository::REF_KEEP_AROUND}/"])
+
+      expect(hidden_refs).to match_array([
+        Gitaly::ListRefsResponse::Reference.new(name: subject.merge_request.ref_path, target: subject.head_commit_sha),
+        Gitaly::ListRefsResponse::Reference.new(name: "refs/#{Repository::REF_KEEP_AROUND}/#{subject.head_commit_sha}", target: subject.head_commit_sha),
+        Gitaly::ListRefsResponse::Reference.new(name: "refs/#{Repository::REF_KEEP_AROUND}/#{subject.start_commit_sha}", target: subject.start_commit_sha)
+      ])
+    end
+
     context 'when diff_type is merge_head' do
-      let_it_be(:merge_request) { create(:merge_request) }
+      let(:merge_request) { create(:merge_request) }
 
-      let_it_be(:merge_head) do
+      let!(:merge_head) do
         MergeRequests::MergeToRefService
           .new(project: merge_request.project, current_user: merge_request.author)
           .execute(merge_request)
@@ -79,6 +93,17 @@
       it { expect(merge_head.head_commit_sha).to eq(merge_request.merge_ref_head.diff_refs.head_sha) }
       it { expect(merge_head.base_commit_sha).to eq(merge_request.merge_ref_head.diff_refs.base_sha) }
       it { expect(merge_head.start_commit_sha).to eq(merge_request.target_branch_sha) }
+
+      it 'creates hidden refs' do
+        hidden_refs = merge_request.project.repository.raw.list_refs(["refs/#{Repository::REF_MERGE_REQUEST}/", "refs/#{Repository::REF_KEEP_AROUND}/"])
+
+        expect(hidden_refs).to match_array([
+          Gitaly::ListRefsResponse::Reference.new(name: merge_request.ref_path, target: merge_request.source_branch_sha),
+          Gitaly::ListRefsResponse::Reference.new(name: merge_request.merge_ref_path, target: merge_head.head_commit_sha),
+          Gitaly::ListRefsResponse::Reference.new(name: "refs/#{Repository::REF_KEEP_AROUND}/#{merge_head.start_commit_sha}", target: merge_head.start_commit_sha),
+          Gitaly::ListRefsResponse::Reference.new(name: "refs/#{Repository::REF_KEEP_AROUND}/#{merge_request.source_branch_sha}", target: merge_request.source_branch_sha)
+        ])
+      end
     end
   end