Merge branch 'dm-process-commit-worker-n+1' into 'master'

Look up upstream commits once before queuing ProcessCommitWorkers

Closes #65464

See merge request gitlab-org/gitlab-ce!31871

2 files changed, 25 insertions, 17 deletions

diff --git a/app/services/git/branch_hooks_service.rb b/app/services/git/branch_hooks_service.rb
index 431a5aedf2e..d2b037a680c 100644
--- a/app/services/git/branch_hooks_service.rb
+++ b/app/services/git/branch_hooks_service.rb
@@ -83,8 +83,16 @@ module Git
 
     # Schedules processing of commit messages
     def enqueue_process_commit_messages
-      limited_commits.each do |commit|
-        next unless commit.matches_cross_reference_regex?
+      referencing_commits = limited_commits.select(&:matches_cross_reference_regex?)
+
+      upstream_commit_ids = upstream_commit_ids(referencing_commits)
+
+      referencing_commits.each do |commit|
+        # Avoid reprocessing commits that already exist upstream if the project
+        # is a fork. This will prevent duplicated/superfluous system notes on
+        # mentionables referenced by a commit that is pushed to the upstream,
+        # that is then also pushed to forks when these get synced by users.
+        next if upstream_commit_ids.include?(commit.id)
 
         ProcessCommitWorker.perform_async(
           project.id,
@@ -142,5 +150,18 @@ module Git
     def branch_name
       strong_memoize(:branch_name) { Gitlab::Git.ref_name(params[:ref]) }
     end
+
+    def upstream_commit_ids(commits)
+      set = Set.new
+
+      upstream_project = project.fork_source
+      if upstream_project
+        upstream_project
+          .commits_by(oids: commits.map(&:id))
+          .each { |commit| set << commit.id }
+      end
+
+      set
+    end
   end
 end
diff --git a/app/workers/process_commit_worker.rb b/app/workers/process_commit_worker.rb
index 3efb5343a96..f6ebe4ab006 100644
--- a/app/workers/process_commit_worker.rb
+++ b/app/workers/process_commit_worker.rb
@@ -2,7 +2,8 @@
 
 # Worker for processing individual commit messages pushed to a repository.
 #
-# Jobs for this worker are scheduled for every commit that is being pushed. As a
+# Jobs for this worker are scheduled for every commit that contains mentionable
+# references in its message and does not exist in the upstream project. As a
 # result of this the workload of this worker should be kept to a bare minimum.
 # Consider using an extra worker if you need to add any extra (and potentially
 # slow) processing of commits.
@@ -19,7 +20,6 @@ class ProcessCommitWorker
     project = Project.find_by(id: project_id)
 
     return unless project
-    return if commit_exists_in_upstream?(project, commit_hash)
 
     user = User.find_by(id: user_id)
 
@@ -77,17 +77,4 @@ class ProcessCommitWorker
 
     Commit.from_hash(hash, project)
   end
-
-  private
-
-  # Avoid reprocessing commits that already exist in the upstream
-  # when project is forked. This will also prevent duplicated system notes.
-  def commit_exists_in_upstream?(project, commit_hash)
-    upstream_project = project.fork_source
-
-    return false unless upstream_project
-
-    commit_id = commit_hash.with_indifferent_access[:id]
-    upstream_project.commit(commit_id).present?
-  end
 end