Commit dfd4406f authored by Marc Shaw's avatar Marc Shaw

Preload data used in the updating of a merge request

Merge Request: gitlab.com/gitlab-org/gitlab/-/merge_requests/53536
Issue: gitlab.com/gitlab-org/gitlab/-/issues/218410
parent b382088c
...@@ -317,6 +317,8 @@ class MergeRequest < ApplicationRecord ...@@ -317,6 +317,8 @@ class MergeRequest < ApplicationRecord
scope :preload_author, -> { preload(:author) } scope :preload_author, -> { preload(:author) }
scope :preload_approved_by_users, -> { preload(:approved_by_users) } scope :preload_approved_by_users, -> { preload(:approved_by_users) }
scope :preload_metrics, -> (relation) { preload(metrics: relation) } 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_web_entity_associations, -> { preload(:author, :target_project) }
scope :with_auto_merge_enabled, -> do scope :with_auto_merge_enabled, -> do
......
...@@ -74,7 +74,8 @@ module MergeRequests ...@@ -74,7 +74,8 @@ module MergeRequests
def post_merge_manually_merged def post_merge_manually_merged
commit_ids = @commits.map(&:id) commit_ids = @commits.map(&:id)
merge_requests = @project.merge_requests.opened 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 .where(target_branch: @push.branch_name).to_a
.select(&:diff_head_commit) .select(&:diff_head_commit)
.select do |merge_request| .select do |merge_request|
...@@ -116,11 +117,14 @@ module MergeRequests ...@@ -116,11 +117,14 @@ module MergeRequests
# Note: we should update merge requests from forks too # Note: we should update merge requests from forks too
def reload_merge_requests def reload_merge_requests
merge_requests = @project.merge_requests.opened 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? if branch_and_project_match?(merge_request) || @push.force_push?
merge_request.reload_diff(current_user) merge_request.reload_diff(current_user)
# Clear existing merge error if the push were directed at the # Clear existing merge error if the push were directed at the
......
---
title: Preload certain data used in the updating of a merge request
merge_request: 53802
author:
type: performance
...@@ -72,6 +72,21 @@ RSpec.describe MergeRequests::RefreshService do ...@@ -72,6 +72,21 @@ RSpec.describe MergeRequests::RefreshService do
allow(NotificationService).to receive(:new) { notification_service } allow(NotificationService).to receive(:new) { notification_service }
end 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 it 'executes hooks with update action' do
refresh_service.execute(@oldrev, @newrev, 'refs/heads/master') refresh_service.execute(@oldrev, @newrev, 'refs/heads/master')
reload_mrs reload_mrs
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment