Commit b9fe40b0 authored by Terri Chu's avatar Terri Chu Committed by Dylan Griffith

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

parent bb2bcbd3
...@@ -358,7 +358,7 @@ class MergeRequest < ApplicationRecord ...@@ -358,7 +358,7 @@ class MergeRequest < ApplicationRecord
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_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 :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 scope :with_auto_merge_enabled, -> do
with_state(:opened).where(auto_merge_enabled: true) 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 ...@@ -82,7 +82,7 @@ module EE
class_methods do class_methods do
# This is an ActiveRecord scope in CE # This is an ActiveRecord scope in CE
def with_api_entity_associations def with_api_entity_associations
super.preload(:blocking_merge_requests) super.preload(:blocking_merge_requests, target_project: [group: :saml_provider])
end end
def sort_by_attribute(method, *args, **kwargs) def sort_by_attribute(method, *args, **kwargs)
......
...@@ -5,6 +5,7 @@ require 'spec_helper' ...@@ -5,6 +5,7 @@ require 'spec_helper'
RSpec.describe 'Global elastic search', :elastic, :sidekiq_inline do RSpec.describe 'Global elastic search', :elastic, :sidekiq_inline do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:project) { create(:project, :repository, :wiki_repo, namespace: user.namespace) } let(:project) { create(:project, :repository, :wiki_repo, namespace: user.namespace) }
let(:projects) { create_list(:project, 5, :public, :repository, :wiki_repo) }
before do before do
stub_ee_application_setting(elasticsearch_search: true, elasticsearch_indexing: true) stub_ee_application_setting(elasticsearch_search: true, elasticsearch_indexing: true)
...@@ -22,7 +23,11 @@ RSpec.describe 'Global elastic search', :elastic, :sidekiq_inline do ...@@ -22,7 +23,11 @@ RSpec.describe 'Global elastic search', :elastic, :sidekiq_inline do
control_count = ActiveRecord::QueryRecorder.new { visit path }.count control_count = ActiveRecord::QueryRecorder.new { visit path }.count
expect(page).to have_css('.search-results') # Confirm there are search results to prevent false positives 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! ensure_elasticsearch_index!
...@@ -40,7 +45,9 @@ RSpec.describe 'Global elastic search', :elastic, :sidekiq_inline do ...@@ -40,7 +45,9 @@ RSpec.describe 'Global elastic search', :elastic, :sidekiq_inline do
let(:object) { :issue } let(:object) { :issue }
let(:creation_args) { { project: project, title: 'initial' } } let(:creation_args) { { project: project, title: 'initial' } }
let(:path) { search_path(search: 'initial', scope: 'issues') } 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' it_behaves_like 'an efficient database result'
end end
...@@ -59,8 +66,8 @@ RSpec.describe 'Global elastic search', :elastic, :sidekiq_inline do ...@@ -59,8 +66,8 @@ RSpec.describe 'Global elastic search', :elastic, :sidekiq_inline do
context 'searching merge requests' do context 'searching merge requests' do
let(:object) { :merge_request } let(:object) { :merge_request }
let(:creation_traits) { [:sequence_source_branch] } let(:creation_traits) { [:unique_branches, :unique_author] }
let(:creation_args) { { source_project: project, title: 'initial' } } let(:creation_args) { { title: 'initial', source_project: project } }
let(:path) { search_path(search: '*', scope: 'merge_requests') } let(:path) { search_path(search: '*', scope: 'merge_requests') }
let(:query_count_multiplier) { 0 } let(:query_count_multiplier) { 0 }
...@@ -71,7 +78,9 @@ RSpec.describe 'Global elastic search', :elastic, :sidekiq_inline do ...@@ -71,7 +78,9 @@ RSpec.describe 'Global elastic search', :elastic, :sidekiq_inline do
let(:object) { :milestone } let(:object) { :milestone }
let(:creation_args) { { project: project } } let(:creation_args) { { project: project } }
let(:path) { search_path(search: '*', scope: 'milestones') } 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' it_behaves_like 'an efficient database result'
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