Merge branch '324745-preload-issues-for-elasticsearch-indexing' into 'master'

Remove N+1 DB queries from indexing issues in Elasticsearch

See merge request gitlab-org/gitlab!57788

app/models/issue.rb

@@ -447,6 +447,10 @@ class Issue < ApplicationRecord
     issue_email_participants.pluck(IssueEmailParticipant.arel_table[:email].lower)
   end
 
+  def issue_assignee_user_ids
+    issue_assignees.pluck(:user_id)
+  end
+
   private
 
   # Ensure that the metrics association is safely created and respecting the unique constraint on issue_id

ee/changelogs/unreleased/324745-preload-issues-for-elasticsearch-indexing.yml

+---
+title: Remove N+1 DB queries from indexing issues in Elasticsearch
+merge_request: 57788
+author:
+type: performance

ee/lib/elastic/latest/issue_class_proxy.rb

@@ -28,6 +28,12 @@ module Elastic
         search(query_hash, options)
       end
 
+      # rubocop: disable CodeReuse/ActiveRecord
+      def preload_indexing_data(relation)
+        relation.includes(:issue_assignees, project: [:project_feature])
+      end
+      # rubocop: enable CodeReuse/ActiveRecord
+
       private
 
       # Builds an elasticsearch query that will select documents from a

ee/lib/elastic/latest/issue_instance_proxy.rb

@@ -12,7 +12,11 @@ module Elastic
           data[attr.to_s] = safely_read_attribute_for_elasticsearch(attr)
         end
 
-        data['assignee_id'] = safely_read_attribute_for_elasticsearch(:assignee_ids)
+        # Load them through the issue_assignees table since calling
+        # assignee_ids can't be easily preloaded and does
+        # unnecessary joins
+        data['assignee_id'] = safely_read_attribute_for_elasticsearch(:issue_assignee_user_ids)
+
         data['visibility_level'] = target.project.visibility_level
         data['issues_access_level'] = safely_read_project_feature_for_elasticsearch(:issues)
 

ee/spec/services/elastic/process_bookkeeping_service_spec.rb

@@ -221,7 +221,7 @@ RSpec.describe Elastic::ProcessBookkeepingService, :clean_gitlab_redis_shared_st
 
         described_class.track!(*notes)
 
-        control = ActiveRecord::QueryRecorder.new { described_class.new.execute }
+        control = ActiveRecord::QueryRecorder.new(skip_cached: false) { described_class.new.execute }
 
         3.times do
           notes << create(:note)
@@ -234,6 +234,20 @@ RSpec.describe Elastic::ProcessBookkeepingService, :clean_gitlab_redis_shared_st
 
         expect { described_class.new.execute }.not_to exceed_all_query_limit(control)
       end
+
+      it 'does not have N+1 queries for issues' do
+        issues = create_list(:issue, 2)
+
+        described_class.track!(*issues)
+
+        control = ActiveRecord::QueryRecorder.new(skip_cached: false) { described_class.new.execute }
+
+        issues += create_list(:issue, 3)
+
+        described_class.track!(*issues)
+
+        expect { described_class.new.execute }.not_to exceed_all_query_limit(control)
+      end
     end
 
     def expect_processing(*refs, failures: [])