Merge branch '215592-avoid-n-1-queries-on-project-search' into 'master'

Resolve "Avoid N+1 queries on project search"

See merge request gitlab-org/gitlab!30346

app/controllers/search_controller.rb

@@ -5,6 +5,10 @@ class SearchController < ApplicationController
   include SearchHelper
   include RendersCommits
 
+  SCOPE_PRELOAD_METHOD = {
+    projects: :with_web_entity_associations
+  }.freeze
+
   around_action :allow_gitaly_ref_name_caching
 
   skip_before_action :authenticate_user!
@@ -28,7 +32,7 @@ class SearchController < ApplicationController
     @scope = search_service.scope
     @show_snippets = search_service.show_snippets?
     @search_results = search_service.search_results
-    @search_objects = search_service.search_objects
+    @search_objects = search_service.search_objects(preload_method)
 
     render_commits if @scope == 'commits'
     eager_load_user_status if @scope == 'users'
@@ -64,6 +68,10 @@ class SearchController < ApplicationController
 
   private
 
+  def preload_method
+    SCOPE_PRELOAD_METHOD[@scope.to_sym]
+  end
+
   def search_term_valid?
     unless search_service.valid_query_length?
       flash[:alert] = t('errors.messages.search_chars_too_long', count: SearchService::SEARCH_CHAR_LIMIT)

app/models/project.rb

@@ -509,6 +509,7 @@ class Project < ApplicationRecord
     left_outer_joins(:pages_metadatum)
       .where(project_pages_metadata: { project_id: nil })
   end
+
   scope :with_api_entity_associations, -> {
     preload(:project_feature, :route, :tags,
             group: :ip_restrictions, namespace: [:route, :owner])
@@ -528,6 +529,10 @@ class Project < ApplicationRecord
   # Used by Projects::CleanupService to hold a map of rewritten object IDs
   mount_uploader :bfg_object_map, AttachmentUploader
 
+  def self.with_web_entity_associations
+    preload(:project_feature, :route, :creator, :group, namespace: [:route, :owner])
+  end
+
   def self.eager_load_namespace_and_owner
     includes(namespace: :owner)
   end

ee/app/models/ee/project.rb

@@ -191,6 +191,8 @@ module EE
     end
 
     class_methods do
+      extend ::Gitlab::Utils::Override
+
       def search_by_visibility(level)
         where(visibility_level: ::Gitlab::VisibilityLevel.string_options[level])
       end
@@ -205,6 +207,11 @@ module EE
         # see https://gitlab.com/gitlab-org/gitlab/-/merge_requests/24063#note_282435524 for details
         joins(:service_desk_setting).find_by('service_desk_settings.project_key' => key)
       end
+
+      override :with_web_entity_associations
+      def with_web_entity_associations
+        super.preload(:compliance_framework_setting)
+      end
     end
 
     def can_store_security_reports?

ee/changelogs/unreleased/215592-fix-n-1-queries-on-project-web-search.yml

+---
+title: Fix N+1 queries for Elastic Web Search projects scope
+merge_request: 30346
+author:
+type: performance

ee/lib/gitlab/elastic/search_results.rb

@@ -29,7 +29,7 @@ module Gitlab
 
         case scope
         when 'projects'
-          eager_load(projects, page, per_page, preload_method, [:route, :namespace, :compliance_framework_setting])
+          eager_load(projects, page, per_page, preload_method, [:route, :namespace])
         when 'issues'
           eager_load(issues, page, per_page, preload_method, project: [:route, :namespace], labels: [], timelogs: [], assignees: [])
         when 'merge_requests'

ee/spec/requests/search_controller_spec.rb

@@ -5,7 +5,7 @@ require 'spec_helper'
 describe SearchController, type: :request do
   let_it_be(:user) { create(:user) }
   let_it_be(:group) { create(:group) }
-  let(:project) { create(:project, :public, :repository, :wiki_repo, name: 'awesome project', group: group) }
+  let_it_be(:project) { create(:project, :public, :repository, :wiki_repo, name: 'awesome project', group: group) }
 
   before_all do
     login_as(user)
@@ -20,6 +20,7 @@ describe SearchController, type: :request do
       before do
         stub_ee_application_setting(elasticsearch_search: true, elasticsearch_indexing: true)
       end
+
       context 'for merge_request scope' do
         before do
           create(:merge_request, target_branch: 'feature_1', source_project: project)
@@ -28,6 +29,7 @@ describe SearchController, type: :request do
           create(:merge_request, target_branch: 'feature_4', source_project: project)
           ensure_elasticsearch_index!
         end
+
         it 'avoids N+1 queries' do
           control = ActiveRecord::QueryRecorder.new(skip_cached: false) { send_search_request(scope: 'merge_requests', search: '*') }
 
@@ -40,7 +42,30 @@ describe SearchController, type: :request do
 
           # some N+1 queries still exist
           expect { send_search_request(scope: 'merge_requests', search: '*') }
-              .not_to exceed_all_query_limit(control.count + 2)
+            .not_to exceed_all_query_limit(control)
+        end
+      end
+
+      context 'for project scope' do
+        before do
+          create(:project, :public)
+          ensure_elasticsearch_index!
+        end
+
+        it 'avoids N+1 queries' do
+          control = ActiveRecord::QueryRecorder.new(skip_cached: false) { send_search_request(scope: 'project', search: '*') }
+
+          create_list(:project, 3, :public)
+
+          ensure_elasticsearch_index!
+
+          # some N+1 queries still exist
+          # each project requires 3 extra queries
+          #   - one count for forks
+          #   - one count for open MRs
+          #   - one count for open Issues
+          expect { send_search_request(scope: 'project', search: '*') }
+            .not_to exceed_all_query_limit(control).with_threshold(9)
         end
       end
     end