CI: use commit SHA in persistent refspec

A Git fetch refspec normally looks like 'refs/foo/bar:refs/qux/baz'.
In English that means: resolve 'refs/foo/bar' on the remote server,
fetch the object it points to, and create a local reference
'refs/qux/baz' that points to that thing.

However, since Git 1.8.3 (released in 2013) it is also allowed to
write '$COMMIT_SHA:refs/qux/baz'. What that does is: fetch object
$COMMIT_SHA from the server, and create a local 'refs/qux/baz' that
points to that thing.

This commit changes the refspec used for "persistent ref" CI fetches.
We already know what commit SHA persistent refs point to so there is
no need to have Git resolve the persistent ref again during a CI
build.

app/presenters/ci/build_runner_presenter.rb

@@ -93,7 +93,22 @@ module Ci
     end
 
     def refspec_for_persistent_ref
-      "+#{persistent_ref_path}:#{persistent_ref_path}"
+      #
+      # End-to-end test coverage for CI fetching seems to not be strong, so we
+      # are using a feature flag here to close the confidence gap. My (JV)
+      # confidence about the change is very high but if something is wrong
+      # with it after all, this would cause all CI jobs on gitlab.com to fail.
+      #
+      # The roll-out will be tracked in
+      # https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/746.
+      #
+      if Feature.enabled?(:scalability_ci_fetch_sha, type: :ops)
+        # Use persistent_ref.sha because it 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}"
+      else
+        "+#{pipeline.persistent_ref.path}:#{pipeline.persistent_ref.path}"
+      end
     end
 
     def persistent_ref_exist?
@@ -107,10 +122,6 @@ module Ci
       pipeline.persistent_ref.exist?
     end
 
-    def persistent_ref_path
-      pipeline.persistent_ref.path
-    end
-
     def git_depth_variable
       strong_memoize(:git_depth_variable) do
         variables&.find { |variable| variable[:key] == 'GIT_DEPTH' }

changelogs/unreleased/jv-ci-refspec-sha.yml

+---
+title: 'CI: use commit SHA in persistent refspec'
+merge_request: 51208
+author:
+type: performance

config/feature_flags/ops/scalability_ci_fetch_sha.yml

+---
+name: scalability_ci_fetch_sha
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/51208
+rollout_issue_url: https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/746
+milestone: '13.8'
+type: ops
+group: team::Scalability
+default_enabled: false

doc/ci/pipelines/index.md

@@ -89,9 +89,9 @@ This table lists the refspecs injected for each pipeline type:
 
 | Pipeline type                                                      | Refspecs                                                                                       |
 |---------------                                                     |----------------------------------------                                                        |
-| Pipeline for Branches                                              | `+refs/pipelines/<id>:refs/pipelines/<id>` and `+refs/heads/<name>:refs/remotes/origin/<name>` |
-| pipeline for Tags                                                  | `+refs/pipelines/<id>:refs/pipelines/<id>` and `+refs/tags/<name>:refs/tags/<name>`            |
-| [Pipeline for Merge Requests](../merge_request_pipelines/index.md) | `+refs/pipelines/<id>:refs/pipelines/<id>`                                                     |
+| Pipeline for Branches                                              | `+<sha>:refs/pipelines/<id>` and `+refs/heads/<name>:refs/remotes/origin/<name>` |
+| pipeline for Tags                                                  | `+<sha>:refs/pipelines/<id>` and `+refs/tags/<name>:refs/tags/<name>`            |
+| [Pipeline for Merge Requests](../merge_request_pipelines/index.md) | `+<sha>:refs/pipelines/<id>`                                                     |
 
 The refs `refs/heads/<name>` and `refs/tags/<name>` exist in your
 project repository. GitLab generates the special ref `refs/pipelines/<id>` during a

spec/presenters/ci/build_runner_presenter_spec.rb

@@ -189,7 +189,21 @@ 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}",
-                                     "+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}")
+                                     "+#{pipeline.sha}:refs/pipelines/#{pipeline.id}")
+    end
+
+    it 'uses a SHA in the persistent refspec' do
+      expect(subject[0]).to match(/^\+[0-9a-f]{40}:refs\/pipelines\/[0-9]+$/)
+    end
+
+    context 'when the scalability_ci_fetch_sha feature flag is disabled' do
+      before do
+        stub_feature_flags(scalability_ci_fetch_sha: false)
+      end
+
+      it 'fetches the ref by name' do
+        expect(subject[0]).to eq("+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}")
+      end
     end
 
     context 'when ref is tag' do
@@ -197,7 +211,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}",
-                                       "+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}")
+                                       "+#{pipeline.sha}:refs/pipelines/#{pipeline.id}")
       end
 
       context 'when GIT_DEPTH is zero' do
@@ -208,7 +222,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/*',
-                                         "+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}")
+                                         "+#{pipeline.sha}:refs/pipelines/#{pipeline.id}")
         end
       end
     end
@@ -224,7 +238,7 @@ RSpec.describe Ci::BuildRunnerPresenter do
 
       it 'returns the correct refspecs' do
         is_expected
-          .to contain_exactly("+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}")
+          .to contain_exactly("+#{pipeline.sha}:refs/pipelines/#{pipeline.id}")
       end
 
       context 'when GIT_DEPTH is zero' do
@@ -234,7 +248,7 @@ RSpec.describe Ci::BuildRunnerPresenter do
 
         it 'returns the correct refspecs' do
           is_expected
-            .to contain_exactly("+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}",
+            .to contain_exactly("+#{pipeline.sha}:refs/pipelines/#{pipeline.id}",
                                 '+refs/heads/*:refs/remotes/origin/*',
                                 '+refs/tags/*:refs/tags/*')
         end
@@ -244,7 +258,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("+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}",
+          is_expected.to contain_exactly("+#{pipeline.sha}:refs/pipelines/#{pipeline.id}",
                                           "+refs/heads/#{build.ref}:refs/remotes/origin/#{build.ref}")
         end
       end
@@ -262,7 +276,7 @@ RSpec.describe Ci::BuildRunnerPresenter do
 
       it 'exposes the persistent pipeline ref' do
         is_expected
-          .to contain_exactly("+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}",
+          .to contain_exactly("+#{pipeline.sha}: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' => ["+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}",
+              'refspecs' => ["+#{pipeline.sha}:refs/pipelines/#{pipeline.id}",
                              "+refs/heads/#{job.ref}:refs/remotes/origin/#{job.ref}"],
               'depth' => project.ci_default_git_depth }
           end
@@ -284,7 +284,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("+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}",
+                  .to contain_exactly("+#{pipeline.sha}:refs/pipelines/#{pipeline.id}",
                                       '+refs/tags/*:refs/tags/*',
                                       '+refs/heads/*:refs/remotes/origin/*')
               end
@@ -346,7 +346,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("+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}",
+                  .to contain_exactly("+#{pipeline.sha}:refs/pipelines/#{pipeline.id}",
                                       '+refs/tags/*:refs/tags/*',
                                       '+refs/heads/*:refs/remotes/origin/*')
               end