Clean up stale merge request HEAD ref

14 days after a merge request gets closed/merged, its HEAD ref
will be deleted and moved as a `keep-around` ref (if not yet
added).

This is to ensure that the commit won't be deleted when GC runs
and still being able to reduce the number of advertised refs on
git push/pull.

app/services/concerns/merge_requests/removes_refs.rb

+# frozen_string_literal: true
+
+module MergeRequests
+  module RemovesRefs
+    def cleanup_refs(merge_request)
+      CleanupRefsService.schedule(merge_request)
+    end
+  end
+end

app/services/merge_requests/cleanup_refs_service.rb

+# frozen_string_literal: true
+
+module MergeRequests
+  class CleanupRefsService
+    include BaseServiceUtility
+
+    TIME_THRESHOLD = 14.days
+
+    attr_reader :merge_request
+
+    def self.schedule(merge_request)
+      MergeRequestCleanupRefsWorker.perform_in(TIME_THRESHOLD, merge_request.id)
+    end
+
+    def initialize(merge_request)
+      @merge_request = merge_request
+      @repository = merge_request.project.repository
+      @ref_path = merge_request.ref_path
+      @ref_head_sha = @repository.commit(merge_request.ref_path).id
+    end
+
+    def execute
+      return error("Merge request has not been closed nor merged for #{TIME_THRESHOLD.inspect}.") unless eligible?
+
+      # Ensure that commit shas of refs are kept around so we won't lose them when GC runs.
+      keep_around
+
+      return error('Failed to create keep around refs.') unless kept_around?
+
+      delete_refs
+      success
+    end
+
+    private
+
+    attr_reader :repository, :ref_path, :ref_head_sha
+
+    def eligible?
+      return met_time_threshold?(merge_request.metrics&.latest_closed_at) if merge_request.closed?
+
+      merge_request.merged? && met_time_threshold?(merge_request.metrics&.merged_at)
+    end
+
+    def met_time_threshold?(attr)
+      attr.nil? || attr.to_i <= TIME_THRESHOLD.ago.to_i
+    end
+
+    def kept_around?
+      Gitlab::Git::KeepAround.new(repository).kept_around?(ref_head_sha)
+    end
+
+    def keep_around
+      repository.keep_around(ref_head_sha)
+    end
+
+    def delete_refs
+      repository.delete_refs(ref_path)
+    end
+  end
+end

app/services/merge_requests/close_service.rb

@@ -2,6 +2,8 @@
 
 module MergeRequests
   class CloseService < MergeRequests::BaseService
+    include RemovesRefs
+
     def execute(merge_request, commit = nil)
       return merge_request unless can?(current_user, :update_merge_request, merge_request)
 
@@ -19,6 +21,7 @@ module MergeRequests
         merge_request.update_project_counter_caches
         cleanup_environments(merge_request)
         abort_auto_merge(merge_request, 'merge request was closed')
+        cleanup_refs(merge_request)
       end
 
       merge_request

app/services/merge_requests/post_merge_service.rb

@@ -7,6 +7,8 @@ module MergeRequests
   # and execute all hooks and notifications
   #
   class PostMergeService < MergeRequests::BaseService
+    include RemovesRefs
+
     def execute(merge_request)
       merge_request.mark_as_merged
       close_issues(merge_request)
@@ -20,6 +22,7 @@ module MergeRequests
       delete_non_latest_diffs(merge_request)
       cancel_review_app_jobs!(merge_request)
       cleanup_environments(merge_request)
+      cleanup_refs(merge_request)
     end
 
     private

app/views/projects/merge_requests/_how_to_merge.html.haml

@@ -53,4 +53,4 @@
           %strong Tip:
           = succeed '.' do
             You can also checkout merge requests locally by
-            = link_to 'following these guidelines', help_page_path('user/project/merge_requests/reviewing_and_managing_merge_requests.md', anchor: "checkout-merge-requests-locally"), target: '_blank', rel: 'noopener noreferrer'
+            = link_to 'following these guidelines', help_page_path('user/project/merge_requests/reviewing_and_managing_merge_requests.md', anchor: "checkout-merge-requests-locally-through-the-head-ref"), target: '_blank', rel: 'noopener noreferrer'

app/workers/all_queues.yml

@@ -1484,6 +1484,14 @@
   :weight: 5
   :idempotent:
   :tags: []
+- :name: merge_request_cleanup_refs
+  :feature_category: :source_code_management
+  :has_external_dependencies:
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent: true
+  :tags: []
 - :name: merge_request_mergeability_check
   :feature_category: :source_code_management
   :has_external_dependencies:

app/workers/merge_request_cleanup_refs_worker.rb

+# frozen_string_literal: true
+
+class MergeRequestCleanupRefsWorker
+  include ApplicationWorker
+
+  feature_category :source_code_management
+  idempotent!
+
+  def perform(merge_request_id)
+    merge_request = MergeRequest.find_by_id(merge_request_id)
+
+    unless merge_request
+      logger.error("Failed to find merge request with ID: #{merge_request_id}")
+      return
+    end
+
+    result = ::MergeRequests::CleanupRefsService.new(merge_request).execute
+
+    return if result[:status] == :success
+
+    logger.error("Failed cleanup refs of merge request (#{merge_request_id}): #{result[:message]}")
+  end
+end

changelogs/unreleased/223156-remove-stale-mr-refs.yml

+---
+title: Clean up stale merge request HEAD ref
+merge_request: 41555
+author:
+type: performance

config/sidekiq_queues.yml

@@ -154,6 +154,8 @@
   - 2
 - - merge
   - 5
+- - merge_request_cleanup_refs
+  - 1
 - - merge_request_mergeability_check
   - 1
 - - metrics_dashboard_prune_old_annotations

doc/README.md

@@ -198,7 +198,7 @@ The following documentation relates to the DevOps **Create** stage:
 
 | Create topics - Merge Requests                                                                                                              | Description                                                                                                                           |
 |:--------------------------------------------------------------------------------------------------------------------------------------------|:--------------------------------------------------------------------------------------------------------------------------------------|
-| [Checking out merge requests locally](user/project/merge_requests/reviewing_and_managing_merge_requests.md#checkout-merge-requests-locally) | Tips for working with merge requests locally.                                                                                         |
+| [Checking out merge requests locally](user/project/merge_requests/reviewing_and_managing_merge_requests.md#checkout-merge-requests-locally-through-the-head-ref) | Tips for working with merge requests locally.                                                                                         |
 | [Cherry-picking](user/project/merge_requests/cherry_pick_changes.md)                                                                        | Use GitLab for cherry-picking changes.                                                                                                |
 | [Merge request thread resolution](user/discussions/index.md#moving-a-single-thread-to-a-new-issue)                                          | Resolve threads, move threads in a merge request to an issue, and only allow merge requests to be merged if all threads are resolved. |
 | [Merge requests](user/project/merge_requests/index.md)                                                                                      | Merge request management.                                                                                                             |

doc/user/project/merge_requests/reviewing_and_managing_merge_requests.md

@@ -284,15 +284,26 @@ the command line.
 NOTE: **Note:**
 This section might move in its own document in the future.
 
-### Checkout merge requests locally
+### Checkout merge requests locally through the `head` ref
 
 A merge request contains all the history from a repository, plus the additional
 commits added to the branch associated with the merge request. Here's a few
-tricks to checkout a merge request locally.
+ways to checkout a merge request locally.
 
 Please note that you can checkout a merge request locally even if the source
 project is a fork (even a private fork) of the target project.
 
+This relies on the merge request `head` ref (`refs/merge-requests/:iid/head`)
+that is available for each merge request. It allows checking out a merge
+request via its ID instead of its branch.
+
+[Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/223156) in GitLab
+13.4, 14 days after a merge request gets closed or merged, the merge request
+`head` ref will be deleted. This means that the merge request will not be available
+for local checkout via the merge request `head` ref anymore. The merge request
+can still be re-opened. Also, as long as the merge request's branch
+exists, you can still check out the branch as it won't be affected.
+
 #### Checkout locally by adding a Git alias
 
 Add the following alias to your `~/.gitconfig`:

spec/services/merge_requests/cleanup_refs_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe MergeRequests::CleanupRefsService do
+  describe '.schedule' do
+    let(:merge_request) { build(:merge_request) }
+
+    it 'schedules MergeRequestCleanupRefsWorker' do
+      expect(MergeRequestCleanupRefsWorker)
+        .to receive(:perform_in)
+        .with(described_class::TIME_THRESHOLD, merge_request.id)
+
+      described_class.schedule(merge_request)
+    end
+  end
+
+  describe '#execute' do
+    before do
+      # Need to re-enable this as it's being stubbed in spec_helper for
+      # performance reasons but is needed to run for this test.
+      allow(Gitlab::Git::KeepAround).to receive(:execute).and_call_original
+    end
+
+    subject(:result) { described_class.new(merge_request).execute }
+
+    shared_examples_for 'service that cleans up merge request refs' do
+      it 'creates keep around ref and deletes merge request refs' do
+        old_ref_head = ref_head
+
+        aggregate_failures do
+          expect(result[:status]).to eq(:success)
+          expect(kept_around?(old_ref_head)).to be_truthy
+          expect(ref_head).to be_nil
+        end
+      end
+
+      context 'when keep around ref cannot be created' do
+        before do
+          allow_next_instance_of(Gitlab::Git::KeepAround) do |keep_around|
+            expect(keep_around).to receive(:kept_around?).and_return(false)
+          end
+        end
+
+        it_behaves_like 'service that does not clean up merge request refs'
+      end
+    end
+
+    shared_examples_for 'service that does not clean up merge request refs' do
+      it 'does not delete merge request refs' do
+        aggregate_failures do
+          expect(result[:status]).to eq(:error)
+          expect(ref_head).to be_present
+        end
+      end
+    end
+
+    context 'when merge request is closed' do
+      let(:merge_request) { create(:merge_request, :closed) }
+
+      context "when closed #{described_class::TIME_THRESHOLD.inspect} ago" do
+        before do
+          merge_request.metrics.update!(latest_closed_at: described_class::TIME_THRESHOLD.ago)
+        end
+
+        it_behaves_like 'service that cleans up merge request refs'
+      end
+
+      context "when closed later than #{described_class::TIME_THRESHOLD.inspect} ago" do
+        before do
+          merge_request.metrics.update!(latest_closed_at: (described_class::TIME_THRESHOLD - 1.day).ago)
+        end
+
+        it_behaves_like 'service that does not clean up merge request refs'
+      end
+    end
+
+    context 'when merge request is merged' do
+      let(:merge_request) { create(:merge_request, :merged) }
+
+      context "when merged #{described_class::TIME_THRESHOLD.inspect} ago" do
+        before do
+          merge_request.metrics.update!(merged_at: described_class::TIME_THRESHOLD.ago)
+        end
+
+        it_behaves_like 'service that cleans up merge request refs'
+      end
+
+      context "when merged later than #{described_class::TIME_THRESHOLD.inspect} ago" do
+        before do
+          merge_request.metrics.update!(merged_at: (described_class::TIME_THRESHOLD - 1.day).ago)
+        end
+
+        it_behaves_like 'service that does not clean up merge request refs'
+      end
+    end
+
+    context 'when merge request is not closed nor merged' do
+      let(:merge_request) { create(:merge_request, :opened) }
+
+      it_behaves_like 'service that does not clean up merge request refs'
+    end
+  end
+
+  def kept_around?(commit)
+    Gitlab::Git::KeepAround.new(merge_request.project.repository).kept_around?(commit.id)
+  end
+
+  def ref_head
+    merge_request.project.repository.commit(merge_request.ref_path)
+  end
+end

spec/services/merge_requests/close_service_spec.rb

@@ -99,6 +99,12 @@ RSpec.describe MergeRequests::CloseService do
       described_class.new(project, user).execute(merge_request)
     end
 
+    it 'schedules CleanupRefsService' do
+      expect(MergeRequests::CleanupRefsService).to receive(:schedule).with(merge_request)
+
+      described_class.new(project, user).execute(merge_request)
+    end
+
     context 'current user is not authorized to close merge request' do
       before do
         perform_enqueued_jobs do

spec/services/merge_requests/post_merge_service_spec.rb

@@ -72,6 +72,12 @@ RSpec.describe MergeRequests::PostMergeService do
       subject
     end
 
+    it 'schedules CleanupRefsService' do
+      expect(MergeRequests::CleanupRefsService).to receive(:schedule).with(merge_request)
+
+      subject
+    end
+
     context 'when the merge request has review apps' do
       it 'cancels all review app deployments' do
         pipeline = create(:ci_pipeline,

spec/workers/merge_request_cleanup_refs_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe MergeRequestCleanupRefsWorker do
+  describe '#perform' do
+    context 'when merge request exists' do
+      let(:merge_request) { create(:merge_request) }
+      let(:job_args) { merge_request.id }
+
+      include_examples 'an idempotent worker' do
+        it 'calls MergeRequests::CleanupRefsService#execute' do
+          expect_next_instance_of(MergeRequests::CleanupRefsService, merge_request) do |svc|
+            expect(svc).to receive(:execute).and_call_original
+          end.twice
+
+          subject
+        end
+      end
+    end
+
+    context 'when merge request does not exist' do
+      it 'does not call MergeRequests::CleanupRefsService' do
+        expect(MergeRequests::CleanupRefsService).not_to receive(:new)
+
+        perform_multiple(1)
+      end
+    end
+  end
+end