Commit ff3c4c7e authored by Dylan Griffith's avatar Dylan Griffith

Merge branch 'fix-n+1-for-merge-requests-scope' into 'master'

Add preloading to fix N+1 queries in merge requests search [RUN ALL RSPEC]

See merge request gitlab-org/gitlab!57284
parents a3cad689 b9fe40b0
......@@ -359,7 +359,7 @@ class MergeRequest < ApplicationRecord
scope :preload_metrics, -> (relation) { preload(metrics: relation) }
scope :preload_project_and_latest_diff, -> { preload(:source_project, :latest_merge_request_diff) }
scope :preload_latest_diff_commit, -> { 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: [:project_feature, group: [:route, :parent], namespace: :route]) }
scope :with_auto_merge_enabled, -> do
with_state(:opened).where(auto_merge_enabled: true)
......
---
title: Preload additional data to fix N+1 queries for merge request search
merge_request: 57284
author:
type: performance
......@@ -82,7 +82,7 @@ module EE
class_methods do
# This is an ActiveRecord scope in CE
def with_api_entity_associations
super.preload(:blocking_merge_requests)
super.preload(:blocking_merge_requests, target_project: [group: :saml_provider])
end
def sort_by_attribute(method, *args, **kwargs)
......
......@@ -5,6 +5,7 @@ require 'spec_helper'
RSpec.describe 'Global elastic search', :elastic, :sidekiq_inline do
let(:user) { create(:user) }
let(:project) { create(:project, :repository, :wiki_repo, namespace: user.namespace) }
let(:projects) { create_list(:project, 5, :public, :repository, :wiki_repo) }
before do
stub_ee_application_setting(elasticsearch_search: true, elasticsearch_indexing: true)
......@@ -22,7 +23,11 @@ RSpec.describe 'Global elastic search', :elastic, :sidekiq_inline do
control_count = ActiveRecord::QueryRecorder.new { visit path }.count
expect(page).to have_css('.search-results') # Confirm there are search results to prevent false positives
create_list(object, 10, *creation_traits, creation_args)
projects.each do |project|
creation_args[:source_project] = project if creation_args.key?(:source_project)
creation_args[:project] = project if creation_args.key?(:project)
create(object, *creation_traits, creation_args)
end
ensure_elasticsearch_index!
......@@ -40,7 +45,9 @@ RSpec.describe 'Global elastic search', :elastic, :sidekiq_inline do
let(:object) { :issue }
let(:creation_args) { { project: project, title: 'initial' } }
let(:path) { search_path(search: 'initial', scope: 'issues') }
let(:query_count_multiplier) { 0 }
# N+1 queries still exist and will be fixed per
# https://gitlab.com/gitlab-org/gitlab/-/issues/230712
let(:query_count_multiplier) { 1 }
it_behaves_like 'an efficient database result'
end
......@@ -59,8 +66,8 @@ RSpec.describe 'Global elastic search', :elastic, :sidekiq_inline do
context 'searching merge requests' do
let(:object) { :merge_request }
let(:creation_traits) { [:sequence_source_branch] }
let(:creation_args) { { source_project: project, title: 'initial' } }
let(:creation_traits) { [:unique_branches, :unique_author] }
let(:creation_args) { { title: 'initial', source_project: project } }
let(:path) { search_path(search: '*', scope: 'merge_requests') }
let(:query_count_multiplier) { 0 }
......@@ -71,7 +78,9 @@ RSpec.describe 'Global elastic search', :elastic, :sidekiq_inline do
let(:object) { :milestone }
let(:creation_args) { { project: project } }
let(:path) { search_path(search: '*', scope: 'milestones') }
let(:query_count_multiplier) { 0 }
# N+1 queries still exist and will be fixed per
# https://gitlab.com/gitlab-org/gitlab/-/issues/325887
let(:query_count_multiplier) { 1 }
it_behaves_like 'an efficient database result'
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