Eliminate N+1 performance issues in MergeRequest.pipelines

This requires the use of the CachingArrayResolver due to the way
MergeRequest.pipelines is implemented.

This is OK to use since the cardinality of `MergeRequest.pipelines` is
low (collections with more than 20 elements are rare). We need to set
an appropriate max page size though, so we set that here to 500
pipelines for any one MR, which is very conservative, since in
gitlab-org/gitlab> the 99th percentile is 39 pipelines per MR.

app/graphql/resolvers/merge_request_pipelines_resolver.rb

@@ -5,14 +5,32 @@ module Resolvers
   class MergeRequestPipelinesResolver < BaseResolver
     # The GraphQL type here gets defined in this include
     include ::ResolvesPipelines
+    include ::CachingArrayResolver
 
     alias_method :merge_request, :object
 
+    # Return at most 500 pipelines for each MR.
+    # Merge requests generally have many fewer pipelines than this.
+    def self.field_options
+      super.merge(max_page_size: 500)
+    end
+
     def resolve(**args)
       return unless project
 
-      resolve_pipelines(project, args)
-        .merge(merge_request.all_pipelines)
+      super
+    end
+
+    def query_for(args)
+      resolve_pipelines(project, args).merge(merge_request.all_pipelines)
+    end
+
+    def model_class
+      ::Ci::Pipeline
+    end
+
+    def query_input(**args)
+      args
     end
 
     def project

app/graphql/types/merge_request_type.rb

@@ -115,7 +115,7 @@ module Types
           description: 'The pipeline running on the branch HEAD of the merge request'
     field :pipelines,
           null: true,
-          description: 'Pipelines for the merge request',
+          description: 'Pipelines for the merge request. Note: for performance reasons, no more than the most recent 500 pipelines will be returned.',
           resolver: Resolvers::MergeRequestPipelinesResolver
 
     field :milestone, Types::MilestoneType, null: true,

changelogs/unreleased/ajk-quality-triage-ops-532.yml

+---
+title: Eliminate N+1 performance issues in MergeRequest.pipelines in GraphQL API
+merge_request: 47784
+author:
+type: fixed

doc/api/graphql/reference/gitlab_schema.graphql

@@ -13188,7 +13188,8 @@ type MergeRequest implements CurrentUserTodos & Noteable {
   ): UserConnection
 
   """
-  Pipelines for the merge request
+  Pipelines for the merge request. Note: for performance reasons, no more than
+  the most recent 500 pipelines will be returned.
   """
   pipelines(
     """

doc/api/graphql/reference/gitlab_schema.json

@@ -36392,7 +36392,7 @@
             },
             {
               "name": "pipelines",
-              "description": "Pipelines for the merge request",
+              "description": "Pipelines for the merge request. Note: for performance reasons, no more than the most recent 500 pipelines will be returned.",
               "args": [
                 {
                   "name": "status",

doc/api/graphql/reference/index.md

@@ -2043,7 +2043,7 @@ Autogenerated return type of MarkAsSpamSnippet.
 | `milestone` | Milestone | The milestone of the merge request |
 | `notes` | NoteConnection! | All notes on this noteable |
 | `participants` | UserConnection | Participants in the merge request |
-| `pipelines` | PipelineConnection | Pipelines for the merge request |
+| `pipelines` | PipelineConnection | Pipelines for the merge request. Note: for performance reasons, no more than the most recent 500 pipelines will be returned. |
 | `project` | Project! | Alias for target_project |
 | `projectId` | Int! | ID of the merge request project |
 | `rebaseCommitSha` | String | Rebase commit SHA of the merge request |

spec/graphql/resolvers/merge_request_pipelines_resolver_spec.rb

@@ -24,7 +24,7 @@ RSpec.describe Resolvers::MergeRequestPipelinesResolver do
   end
 
   def resolve_pipelines
-    resolve(described_class, obj: merge_request, ctx: { current_user: current_user })
+    sync(resolve(described_class, obj: merge_request, ctx: { current_user: current_user }))
   end
 
   it 'resolves only MRs for the passed merge request' do

spec/graphql/types/merge_request_type_spec.rb

@@ -39,6 +39,12 @@ RSpec.describe GitlabSchema.types['MergeRequest'] do
     expect(described_class).to have_graphql_fields(*expected_fields)
   end
 
+  describe '#pipelines' do
+    subject { described_class.fields['pipelines'] }
+
+    it { is_expected.to have_attributes(max_page_size: 500) }
+  end
+
   describe '#diff_stats_summary' do
     subject { GitlabSchema.execute(query, context: { current_user: current_user }).as_json }
 

spec/requests/api/graphql/project/merge_request/pipelines_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'Query.project.mergeRequests.pipelines' do
+  include GraphqlHelpers
+
+  let_it_be(:project) { create(:project, :public, :repository) }
+  let_it_be(:author) { create(:user) }
+  let_it_be(:merge_requests) do
+    %i[with_diffs with_image_diffs conflict].map do |trait|
+      create(:merge_request, trait, author: author, source_project: project)
+    end
+  end
+
+  describe '.count' do
+    let(:query) do
+      <<~GQL
+      query($path: ID!, $first: Int) {
+        project(fullPath: $path) {
+          mergeRequests(first: $first) {
+            nodes {
+              iid
+              pipelines {
+                count
+              }
+            }
+          }
+        }
+      }
+      GQL
+    end
+
+    def run_query(first = nil)
+      post_graphql(query, current_user: author, variables: { path: project.full_path, first: first })
+    end
+
+    before do
+      merge_requests.each do |mr|
+        shas = mr.all_commits.limit(2).pluck(:sha)
+
+        shas.each do |sha|
+          create(:ci_pipeline, :success, project: project, ref: mr.source_branch, sha: sha)
+        end
+      end
+    end
+
+    it 'produces correct results' do
+      run_query(2)
+
+      p_nodes = graphql_data_at(:project, :merge_requests, :nodes)
+
+      expect(p_nodes).to all(match('iid' => be_present, 'pipelines' => match('count' => 2)))
+    end
+
+    it 'is scalable', :request_store, :use_clean_rails_memory_store_caching do
+      # warm up
+      run_query
+
+      expect { run_query(2) }.to(issue_same_number_of_queries_as { run_query(1) }.ignoring_cached_queries)
+    end
+  end
+end