Commit bf638d88 authored by Matthias Käppler's avatar Matthias Käppler

Merge branch '276944-reduce-lcp-to-2-5s-for-sourcecode_mr_list' into 'master'

Fix N+1 Issue with loading approval rules

See merge request gitlab-org/gitlab!52364
parents 931ad1c6 c5907053
......@@ -9,7 +9,7 @@ module EE
def preload_for_collection
@preload_for_collection ||= case collection_type
when 'MergeRequest'
super.push(:approval_rules)
super + [approval_rules: [:users, :group_users], approval_project_rules: [:users, :group_users]]
when 'Issue'
super.push(*issue_preloads)
else
......
---
title: Resolve N+1 issue loading approval rules on the MR list
merge_request: 52364
author:
type: performance
......@@ -60,39 +60,6 @@ RSpec.describe Projects::MergeRequestsController do
sign_in(viewer)
end
describe 'GET index' do
def get_merge_requests
get :index,
params: {
namespace_id: project.namespace.to_param,
project_id: project,
state: 'opened'
}
end
context 'when filtering by opened state' do
context 'with opened merge requests' do
render_views
it 'avoids N+1' do
other_user = create(:user)
create(:merge_request, :unique_branches, target_project: project, source_project: project)
create_list(:approval_merge_request_rule, 5, merge_request: merge_request, users: [user, other_user], approvals_required: 2)
control_count = ActiveRecord::QueryRecorder.new { get_merge_requests }.count
create_list(:approval, 10)
create_list(:merge_request, 20, :unique_branches, target_project: project, source_project: project).each do |mr|
create(:approval_merge_request_rule, merge_request: merge_request, users: [user, other_user], approvals_required: 2)
end
expect do
get_merge_requests
end.not_to exceed_query_limit(control_count)
end
end
end
end
describe 'PUT update' do
let_it_be_with_reload(:merge_request) do
create(:merge_request_with_diffs, source_project: project, author: author)
......
......@@ -43,4 +43,27 @@ RSpec.describe Projects::MergeRequestsController do
end
end
end
describe 'GET #index' do
def get_index
get project_merge_requests_path(project, state: 'opened')
end
it 'avoids N+1' do
other_user = create(:user)
create(:merge_request, :unique_branches, target_project: project, source_project: project)
create_list(:approval_project_rule, 5, project: project, users: [user, other_user], approvals_required: 2)
create_list(:approval_merge_request_rule, 5, merge_request: merge_request, users: [user, other_user], approvals_required: 2)
control_count = ActiveRecord::QueryRecorder.new { get_index }.count
create_list(:approval, 10)
create(:approval_project_rule, project: project, users: [user, other_user], approvals_required: 2)
create_list(:merge_request, 20, :unique_branches, target_project: project, source_project: project).each do |mr|
create(:approval_merge_request_rule, merge_request: mr, users: [user, other_user], approvals_required: 2)
end
expect { get_index }.not_to exceed_query_limit(control_count)
end
end
end
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