Merge branch 'dm-large-push-performance' into 'master'

Load and process at most 100 commits when pushing into default branch

Closes #35531

See merge request !13132

4 files changed, 29 insertions, 16 deletions

diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb
index bb7680c5054..ada2b64a3a6 100644
--- a/app/services/git_push_service.rb
+++ b/app/services/git_push_service.rb
@@ -45,6 +45,7 @@ class GitPushService < BaseService
     elsif push_to_existing_branch?
       # Collect data for this git push
       @push_commits = @project.repository.commits_between(params[:oldrev], params[:newrev])
+
       process_commit_messages
 
       # Update the bare repositories info/attributes file using the contents of the default branches
@@ -66,15 +67,21 @@ class GitPushService < BaseService
 
   def update_caches
     if is_default_branch?
-      paths = Set.new
+      if push_to_new_branch?
+        # If this is the initial push into the default branch, the file type caches
+        # will already be reset as a result of `Project#change_head`.
+        types = []
+      else
+        paths = Set.new
 
-      @push_commits.each do |commit|
-        commit.raw_deltas.each do |diff|
-          paths << diff.new_path
+        @push_commits.last(PROCESS_COMMIT_LIMIT).each do |commit|
+          commit.raw_deltas.each do |diff|
+            paths << diff.new_path
+          end
         end
-      end
 
-      types = Gitlab::FileDetector.types_in_paths(paths.to_a)
+        types = Gitlab::FileDetector.types_in_paths(paths.to_a)
+      end
     else
       types = []
     end
@@ -92,7 +99,7 @@ class GitPushService < BaseService
   def process_commit_messages
     default = is_default_branch?
 
-    push_commits.last(PROCESS_COMMIT_LIMIT).each do |commit|
+    @push_commits.last(PROCESS_COMMIT_LIMIT).each do |commit|
       if commit.matches_cross_reference_regex?
         ProcessCommitWorker
           .perform_async(project.id, current_user.id, commit.to_hash, default)
@@ -111,7 +118,7 @@ class GitPushService < BaseService
 
     EventCreateService.new.push(@project, current_user, build_push_data)
     Ci::CreatePipelineService.new(@project, current_user, build_push_data).execute(:push)
-    
+
     SystemHookPushWorker.perform_async(build_push_data.dup, :push_hooks)
     @project.execute_hooks(build_push_data.dup, :push_hooks)
     @project.execute_services(build_push_data.dup, :push_hooks)
@@ -131,7 +138,10 @@ class GitPushService < BaseService
   end
 
   def process_default_branch
-    @push_commits = project.repository.commits(params[:newrev])
+    @push_commits_count = project.repository.commit_count_for_ref(params[:ref])
+
+    offset = [@push_commits_count - PROCESS_COMMIT_LIMIT, 0].max
+    @push_commits = project.repository.commits(params[:newrev], offset: offset, limit: PROCESS_COMMIT_LIMIT)
 
     # Ensure HEAD points to the default branch in case it is not master
     project.change_head(branch_name)
@@ -160,7 +170,8 @@ class GitPushService < BaseService
       params[:oldrev],
       params[:newrev],
       params[:ref],
-      push_commits)
+      @push_commits,
+      commits_count: @push_commits_count)
   end
 
   def push_to_existing_branch?
diff --git a/changelogs/unreleased/dm-large-push-performance.yml b/changelogs/unreleased/dm-large-push-performance.yml
new file mode 100644
index 00000000000..f5fe1bd3b28
--- /dev/null
+++ b/changelogs/unreleased/dm-large-push-performance.yml
@@ -0,0 +1,4 @@
+---
+title: Improve performance of large (initial) push into default branch
+merge_request:
+author:
diff --git a/lib/gitlab/data_builder/push.rb b/lib/gitlab/data_builder/push.rb
index 8c8729b6557..5c5f507d44d 100644
--- a/lib/gitlab/data_builder/push.rb
+++ b/lib/gitlab/data_builder/push.rb
@@ -24,11 +24,11 @@ module Gitlab
       #   total_commits_count: Fixnum
       # }
       #
-      def build(project, user, oldrev, newrev, ref, commits = [], message = nil)
+      def build(project, user, oldrev, newrev, ref, commits = [], message = nil, commits_count: nil)
         commits = Array(commits)
 
         # Total commits count
-        commits_count = commits.size
+        commits_count ||= commits.size
 
         # Get latest 20 commits ASC
         commits_limited = commits.last(20)
diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb
index 4023c33aa59..3fb677b65be 100644
--- a/spec/services/git_push_service_spec.rb
+++ b/spec/services/git_push_service_spec.rb
@@ -658,8 +658,7 @@ describe GitPushService, services: true do
     end
 
     it 'only schedules a limited number of commits' do
-      allow(service).to receive(:push_commits)
-        .and_return(Array.new(1000, double(:commit, to_hash: {}, matches_cross_reference_regex?: true)))
+      service.push_commits = Array.new(1000, double(:commit, to_hash: {}, matches_cross_reference_regex?: true))
 
       expect(ProcessCommitWorker).to receive(:perform_async).exactly(100).times
 
@@ -667,8 +666,7 @@ describe GitPushService, services: true do
     end
 
     it "skips commits which don't include cross-references" do
-      allow(service).to receive(:push_commits)
-        .and_return([double(:commit, to_hash: {}, matches_cross_reference_regex?: false)])
+      service.push_commits = [double(:commit, to_hash: {}, matches_cross_reference_regex?: false)]
 
       expect(ProcessCommitWorker).not_to receive(:perform_async)