Fetch pipeline refs by name in CI runner

This reverts af43dd6a with the aim of
reducing Gitaly server resouce usage when fetching Git data for CI
builds. Instead of fetching the pipeline ref by SHA, we go back to
fetching it by name 'refs/pipelines/1234'. This causes `git fetch` to
use the 'ref-prefix' option [1] when fetching refs. At the time of
af43dd6a, the server side
implementation of 'ref-prefix' was very inefficient. This got fixed in
Git by [2].

Currently each CI fetch causes a full ref walk on the Gitaly server.
By going back to fetching by ref, this changes to a filtered refwalk
which is much faster. Especially on repositories with many (server
side) refs, such as gitlab-org/gitlab.

[1]: https://gitlab.com/gitlab-org/gitlab-git/-/blob/v2.33.0/Documentation/technical/protocol-v2.txt#L191-193
[2]: https://gitlab.com/gitlab-org/gitlab-git/-/commit/b3970c702cb0acc0551d88a5f34ad4ad2e2a6d39

app/presenters/ci/build_runner_presenter.rb

@@ -105,10 +105,7 @@ module Ci
     end
 
     def refspec_for_persistent_ref
-      # Use persistent_ref.sha because it sometimes causes 'git fetch' to do
-      # less work. See
-      # https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/746.
-      "+#{pipeline.persistent_ref.sha}:#{pipeline.persistent_ref.path}"
+      "+#{pipeline.persistent_ref.path}:#{pipeline.persistent_ref.path}"
     end
 
     def persistent_ref_exist?

spec/presenters/ci/build_runner_presenter_spec.rb

@@ -173,11 +173,7 @@ RSpec.describe Ci::BuildRunnerPresenter do
 
     it 'returns the correct refspecs' do
       is_expected.to contain_exactly("+refs/heads/#{build.ref}:refs/remotes/origin/#{build.ref}",
-                                     "+#{pipeline.sha}:refs/pipelines/#{pipeline.id}")
-    end
-
-    it 'uses a SHA in the persistent refspec' do
-      expect(subject[0]).to match(%r{^\+[0-9a-f]{40}:refs/pipelines/[0-9]+$})
+                                     "+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}")
     end
 
     context 'when ref is tag' do
@@ -185,7 +181,7 @@ RSpec.describe Ci::BuildRunnerPresenter do
 
       it 'returns the correct refspecs' do
         is_expected.to contain_exactly("+refs/tags/#{build.ref}:refs/tags/#{build.ref}",
-                                       "+#{pipeline.sha}:refs/pipelines/#{pipeline.id}")
+                                       "+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}")
       end
 
       context 'when GIT_DEPTH is zero' do
@@ -196,7 +192,7 @@ RSpec.describe Ci::BuildRunnerPresenter do
         it 'returns the correct refspecs' do
           is_expected.to contain_exactly('+refs/tags/*:refs/tags/*',
                                          '+refs/heads/*:refs/remotes/origin/*',
-                                         "+#{pipeline.sha}:refs/pipelines/#{pipeline.id}")
+                                         "+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}")
         end
       end
     end
@@ -212,7 +208,7 @@ RSpec.describe Ci::BuildRunnerPresenter do
 
       it 'returns the correct refspecs' do
         is_expected
-          .to contain_exactly("+#{pipeline.sha}:refs/pipelines/#{pipeline.id}")
+          .to contain_exactly("+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}")
       end
 
       context 'when GIT_DEPTH is zero' do
@@ -222,7 +218,7 @@ RSpec.describe Ci::BuildRunnerPresenter do
 
         it 'returns the correct refspecs' do
           is_expected
-            .to contain_exactly("+#{pipeline.sha}:refs/pipelines/#{pipeline.id}",
+            .to contain_exactly("+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}",
                                 '+refs/heads/*:refs/remotes/origin/*',
                                 '+refs/tags/*:refs/tags/*')
         end
@@ -232,7 +228,7 @@ RSpec.describe Ci::BuildRunnerPresenter do
         let(:merge_request) { create(:merge_request, :with_legacy_detached_merge_request_pipeline) }
 
         it 'returns the correct refspecs' do
-          is_expected.to contain_exactly("+#{pipeline.sha}:refs/pipelines/#{pipeline.id}",
+          is_expected.to contain_exactly("+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}",
                                          "+refs/heads/#{build.ref}:refs/remotes/origin/#{build.ref}")
         end
       end
@@ -250,7 +246,7 @@ RSpec.describe Ci::BuildRunnerPresenter do
 
       it 'exposes the persistent pipeline ref' do
         is_expected
-          .to contain_exactly("+#{pipeline.sha}:refs/pipelines/#{pipeline.id}",
+          .to contain_exactly("+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}",
                               "+refs/heads/#{build.ref}:refs/remotes/origin/#{build.ref}")
       end
     end

spec/requests/api/ci/runner/jobs_request_post_spec.rb

@@ -156,7 +156,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
               'sha' => job.sha,
               'before_sha' => job.before_sha,
               'ref_type' => 'branch',
-              'refspecs' => ["+#{pipeline.sha}:refs/pipelines/#{pipeline.id}",
+              'refspecs' => ["+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}",
                              "+refs/heads/#{job.ref}:refs/remotes/origin/#{job.ref}"],
               'depth' => project.ci_default_git_depth }
           end
@@ -291,7 +291,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
 
                 expect(response).to have_gitlab_http_status(:created)
                 expect(json_response['git_info']['refspecs'])
-                  .to contain_exactly("+#{pipeline.sha}:refs/pipelines/#{pipeline.id}",
+                  .to contain_exactly("+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}",
                                       '+refs/tags/*:refs/tags/*',
                                       '+refs/heads/*:refs/remotes/origin/*')
               end
@@ -359,7 +359,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
 
                 expect(response).to have_gitlab_http_status(:created)
                 expect(json_response['git_info']['refspecs'])
-                  .to contain_exactly("+#{pipeline.sha}:refs/pipelines/#{pipeline.id}",
+                  .to contain_exactly("+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}",
                                       '+refs/tags/*:refs/tags/*',
                                       '+refs/heads/*:refs/remotes/origin/*')
               end