Merge branch 'sh-optimize-mr-commit-sha-lookup' into 'master'

Optimize merge request refresh by using the database to check commit SHAs

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

3 files changed, 21 insertions, 5 deletions

diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 7eef08aa6a3..735d9fba966 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -353,6 +353,15 @@ class MergeRequest < ActiveRecord::Base
     end
   end
 
+  # Returns true if there are commits that match at least one commit SHA.
+  def includes_any_commits?(shas)
+    if persisted?
+      merge_request_diff.commits_by_shas(shas).exists?
+    else
+      (commit_shas & shas).present?
+    end
+  end
+
   # Calls `MergeWorker` to proceed with the merge process and
   # updates `merge_jid` with the MergeWorker#jid.
   # This helps tracking enqueued and ongoing merge jobs.
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index 02c6b650f33..bb6ff8921df 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -140,6 +140,12 @@ class MergeRequestDiff < ActiveRecord::Base
     merge_request_diff_commits.map(&:sha)
   end
 
+  def commits_by_shas(shas)
+    return [] unless shas.present?
+
+    merge_request_diff_commits.where(sha: shas)
+  end
+
   def diff_refs=(new_diff_refs)
     self.base_commit_sha = new_diff_refs&.base_sha
     self.start_commit_sha = new_diff_refs&.start_sha
diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb
index f01872b205e..53768ff2cbe 100644
--- a/app/services/merge_requests/refresh_service.rb
+++ b/app/services/merge_requests/refresh_service.rb
@@ -87,11 +87,8 @@ module MergeRequests
       filter_merge_requests(merge_requests).each do |merge_request|
         if branch_and_project_match?(merge_request) || @push.force_push?
           merge_request.reload_diff(current_user)
-        else
-          mr_commit_ids = merge_request.commit_shas
-          push_commit_ids = @commits.map(&:id)
-          matches = mr_commit_ids & push_commit_ids
-          merge_request.reload_diff(current_user) if matches.any?
+        elsif merge_request.includes_any_commits?(push_commit_ids)
+          merge_request.reload_diff(current_user)
         end
 
         merge_request.mark_as_unchecked
@@ -104,6 +101,10 @@ module MergeRequests
     end
     # rubocop: enable CodeReuse/ActiveRecord
 
+    def push_commit_ids
+      @push_commit_ids ||= @commits.map(&:id)
+    end
+
     def branch_and_project_match?(merge_request)
       merge_request.source_project == @project &&
         merge_request.source_branch == @push.branch_name