From b9fe40b05fae29b7280374665647bd94b8052858 Mon Sep 17 00:00:00 2001
From: Terri Chu <tchu@gitlab.com>
Date: Mon, 29 Mar 2021 04:55:47 +0000
Subject: [PATCH] Add preloading to fix N+1 queries in merge requests search
[RUN ALL RSPEC]
---
app/models/merge_request.rb | 2 +-
.../fix-n-1-for-merge-requests-scope.yml | 5 +++++
ee/app/models/ee/merge_request.rb | 2 +-
.../search/elastic/global_search_spec.rb | 19 ++++++++++++++-----
4 files changed, 21 insertions(+), 7 deletions(-)
create mode 100644 changelogs/unreleased/fix-n-1-for-merge-requests-scope.yml
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 834d28f1e3c7..ed4d19988ab1 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -358,7 +358,7 @@ def public_merge_status
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)
diff --git a/changelogs/unreleased/fix-n-1-for-merge-requests-scope.yml b/changelogs/unreleased/fix-n-1-for-merge-requests-scope.yml
new file mode 100644
index 000000000000..0b473f728935
--- /dev/null
+++ b/changelogs/unreleased/fix-n-1-for-merge-requests-scope.yml
@@ -0,0 +1,5 @@
+---
+title: Preload additional data to fix N+1 queries for merge request search
+merge_request: 57284
+author:
+type: performance
diff --git a/ee/app/models/ee/merge_request.rb b/ee/app/models/ee/merge_request.rb
index f95f4b2fd824..544933c1f7e6 100644
--- a/ee/app/models/ee/merge_request.rb
+++ b/ee/app/models/ee/merge_request.rb
@@ -82,7 +82,7 @@ def merge_requests_disable_committers_approval?
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)
diff --git a/ee/spec/features/search/elastic/global_search_spec.rb b/ee/spec/features/search/elastic/global_search_spec.rb
index d5b108e12d23..e7837cf95256 100644
--- a/ee/spec/features/search/elastic/global_search_spec.rb
+++ b/ee/spec/features/search/elastic/global_search_spec.rb
@@ -5,6 +5,7 @@
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 @@
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 @@
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 @@
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 @@
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
--
GitLab