Merge branch 'test_update_merge_request_worker_performance_3' into 'master'

Preload data used in the updating of a merge request

See merge request gitlab-org/gitlab!53802

app/models/merge_request.rb

@@ -317,6 +317,8 @@ class MergeRequest < ApplicationRecord
   scope :preload_author, -> { preload(:author) }
   scope :preload_approved_by_users, -> { preload(:approved_by_users) }
   scope :preload_metrics, -> (relation) { preload(metrics: relation) }
+  scope :preload_project_and_latest_diff, -> { preload(:source_project, :latest_merge_request_diff) }
+  scope :preload_latest_diff_comment, -> { preload(latest_merge_request_diff: :merge_request_diff_commits) }
   scope :with_web_entity_associations, -> { preload(:author, :target_project) }
 
   scope :with_auto_merge_enabled, -> do

app/services/merge_requests/refresh_service.rb

@@ -74,7 +74,8 @@ module MergeRequests
     def post_merge_manually_merged
       commit_ids = @commits.map(&:id)
       merge_requests = @project.merge_requests.opened
-        .preload(:latest_merge_request_diff)
+        .preload_project_and_latest_diff
+        .preload_latest_diff_comment
         .where(target_branch: @push.branch_name).to_a
         .select(&:diff_head_commit)
         .select do |merge_request|
@@ -116,11 +117,14 @@ module MergeRequests
     # Note: we should update merge requests from forks too
     def reload_merge_requests
       merge_requests = @project.merge_requests.opened
-        .by_source_or_target_branch(@push.branch_name).to_a
+        .by_source_or_target_branch(@push.branch_name)
+        .preload_project_and_latest_diff
 
-      merge_requests += merge_requests_for_forks.to_a
+      merge_requests_from_forks = merge_requests_for_forks
+        .preload_project_and_latest_diff
 
-      filter_merge_requests(merge_requests).each do |merge_request|
+      merge_requests_array = merge_requests.to_a + merge_requests_from_forks.to_a
+      filter_merge_requests(merge_requests_array).each do |merge_request|
         if branch_and_project_match?(merge_request) || @push.force_push?
           merge_request.reload_diff(current_user)
           # Clear existing merge error if the push were directed at the

changelogs/unreleased/test_update_merge_request_worker_performance_3.yml

+---
+title: Preload certain data used in the updating of a merge request
+merge_request: 53802
+author:
+type: performance

spec/services/merge_requests/refresh_service_spec.rb

@@ -72,6 +72,21 @@ RSpec.describe MergeRequests::RefreshService do
         allow(NotificationService).to receive(:new) { notification_service }
       end
 
+      context 'query count' do
+        it 'does not execute a lot of queries' do
+          # Hardcoded the query limit since the queries can also be reduced even
+          # if there are the same number of merge requests (e.g. by preloading
+          # associations). This should also fail in case additional queries are
+          # added elsewhere that affected this service.
+          #
+          # The limit is based on the number of queries executed at the current
+          # state of the service. As we reduce the number of queries executed in
+          # this service, the limit should be reduced as well.
+          expect { refresh_service.execute(@oldrev, @newrev, 'refs/heads/master') }
+            .not_to exceed_query_limit(260)
+        end
+      end
+
       it 'executes hooks with update action' do
         refresh_service.execute(@oldrev, @newrev, 'refs/heads/master')
         reload_mrs