<!--IssueSummary start--> <details> <summary> Everyone can contribute. [Help move this issue forward](https://handbook.gitlab.com/handbook/marketing/developer-relations/engineering/community-contributors-workflows/#contributor-links) while earning points, leveling up and collecting rewards. </summary> - [Collaborate/take over this issue](https://contributors.gitlab.com/manage-issue?action=work&projectId=278964&issueIid=635534) </details> <!--IssueSummary end--> ### Summary `Group.mergeRequests` runs 4 queries per project when the query selects each merge request's `project`. The queries come from `ProjectPolicy` conditions evaluated when `Types::ProjectType` authorizes each project, and nothing on the group merge request path preloads what those conditions read. The `list_merge_requests` MCP tool hits this on every group call, because its group query selects `project { id fullPath }`. ### Steps to reproduce 1. In a GDK with an Ultimate license, create a group with two subgroups, each with a project that has an open merge request. 1. Enable the performance bar and run this in GraphiQL (`/-/graphql-explorer`): ```graphql query { group(fullPath: "<group>") { mergeRequests(includeSubgroups: true) { nodes { id project { id fullPath } } } } } ``` 1. Add a third subgroup with a project and a merge request, and rerun the query. The `pg` count goes up by 4. Also reproduced by the `'an MCP tool that avoids N+1 queries'` examples added in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/264415. For two extra projects, `ee/spec/services/ee/mcp/tools/merge_requests/list_merge_requests_service_spec.rb` carries `threshold: 8` in EE, and the FOSS-only example in `spec/services/mcp/tools/merge_requests/list_merge_requests_service_spec.rb` carries `threshold: 2`. ### Example Project Not applicable. Reproduced locally with factory data on `master` at 16456d8a707a. ### What is the current *bug* behavior? Each additional project in the result adds 4 queries, from `ee/app/policies/ee/project_policy.rb`: | Query | Condition | |---|---| | `project_features` | `merge_requests_disabled` → `ProjectPolicy#access_allowed_to?` | | `namespaces` (the project's group) | `ip_enforcement_prevents_access` → `subject.group` | | `find_namespaces_by_id` (root ancestor) | `needs_new_sso_session` → `SsoEnforcer.access_restricted?` → `EE::Group#root_saml_provider` → `root_ancestor` | | `saml_providers` (cached) | same chain → `root_ancestor.saml_provider` | In FOSS, each additional project adds 1 query: `project_features`, from CE `ProjectPolicy#access_allowed_to?`. The `project:` lookahead preload in `app/graphql/resolvers/concerns/merge_requests/look_ahead_preloads.rb` loads only `route` and `namespace: :route` on `target_project`. ### What is the expected *correct* behavior? The query count stays the same as projects are added. ### Relevant logs and/or screenshots <details> <summary>Measurement</summary> - Service-level spec calling `Mcp::Tools::MergeRequests::ListMergeRequestsService` with a group `full_path`. Each call runs with a fresh request store, Rails cache and BatchLoader executor, as `GraphqlHelpers#run_with_clean_state` does. - Warm-up call, then a control with one merge request in one project, then two more merge requests, each in its own project and subgroup. - 15 queries for the control, 23 after, counting cached queries. Each query was attributed with `QUERY_RECORDER_DEBUG=1`. </details> ### Output of checks Not reproduced on GitLab.com specifically. The code path is the same there. #### Results of GitLab environment info Not applicable. Reproduced in specs on `master`. #### Results of GitLab application Check Not applicable. ### Possible fixes Preload the target projects' policy dependencies in a `before_connection_authorization` block on `Resolvers::GroupMergeRequestsResolver`, the same way `Resolvers::GroupIssuesResolver` and `Resolvers::NamespaceProjectsResolver` do: ```diff --- a/app/graphql/resolvers/group_merge_requests_resolver.rb +++ b/app/graphql/resolvers/group_merge_requests_resolver.rb @@ -10,6 +10,12 @@ def self.issuable_collection_name alias_method :group, :object + before_connection_authorization do |nodes, current_user| + projects = nodes.map(&:target_project).uniq + ActiveRecord::Associations::Preloader.new(records: projects, associations: [:project_feature, :group]).call + ::Preloaders::GroupPolicyPreloader.new(projects.filter_map(&:group), current_user).execute + end + type Types::MergeRequestType.connection_type, null: true ``` With this change, both MCP examples (EE and FOSS) pass without a `threshold:`, and `spec/requests/api/graphql/group/merge_requests_spec.rb` plus the MCP service spec still pass. <details> <summary>Why not <code>Preloaders::ProjectPolicyPreloader</code></summary> The EE `ProjectPolicyPreloader` sets `root_saml_provider` from `ProjectRootAncestorPreloader`, but `ProjectRootAncestorPreloader#execute` returns early unless it gets an `ActiveRecord::Relation`. Connection nodes are an Array, so the `project.root_ancestor.saml_provider` loop in `EE::Preloaders::ProjectPolicyPreloader#execute` still runs 2 queries per project. `GroupPolicyPreloader` uses `GroupRootAncestorPreloader`, which works on arrays and preloads `saml_provider` on the root ancestors. </details> Implementation plan (weight 2): 1. Add the `before_connection_authorization` block above. 1. Add an N+1 example to `spec/requests/api/graphql/group/merge_requests_spec.rb` that selects `project { id fullPath }` across projects in different subgroups. 1. Delete `ee/spec/services/ee/mcp/tools/merge_requests/list_merge_requests_service_spec.rb`. In `spec/services/mcp/tools/merge_requests/list_merge_requests_service_spec.rb`, unwrap the N+1 example from its `context 'when CE edition'` block and remove its `threshold:` and `# TODO` line. `UserMergeRequestsResolverBase` (authored, assigned and review-requested lists) also spans projects. It wasn't measured. ### Patch release information for backports If the bug fix needs to be backported in a [patch release](https://handbook.gitlab.com/handbook/engineering/releases/patch-releases) to a version under [the maintenance policy](https://docs.gitlab.com/policy/maintenance/), please follow the steps on the [patch release runbook for GitLab engineers](https://gitlab.com/gitlab-org/release/docs/-/blob/master/general/patch/engineers.md). Refer to the [internal "Release Information" dashboard](https://dashboards.gitlab.net/d/delivery-release_info/delivery3a-release-information?orgId=1) for information about the next patch release, including the targeted versions, expected release date, and current status. #### High-severity bug remediation To remediate high-severity issues requiring an [internal release](https://handbook.gitlab.com/handbook/engineering/releases/internal-releases/) for single-tenant SaaS instances, refer to the [internal release process for engineers](https://gitlab.com/gitlab-org/release/docs/-/blob/master/general/internal-releases/engineers.md?ref_type=heads).