Remove unnecessary committer batch-load from mergeability fields

The detailedMergeStatus and mergeable GraphQL fields on MergeRequest time out after 30s on merge requests with several thousand commits, when the project has "Prevent approvals by users who add commits" enabled. The cause is a dead EE wrapper (lazy_committers) that eagerly preloads every diff commit through an O(n^2) association load before the field even resolves, work that's no longer needed since committer filtering for approvals moved to committer_emails_from_diff. This MR deletes that wrapper, the mergeable / detailed_merge_status overrides, and the now-unused add_committers_to_batch_loader alias, letting both fields fall back to the plain CE resolvers, and extends the existing N+1 specs for both fields to the case where committers cannot approve.

Related to https://gitlab.com/gitlab-org/gitlab/-/issues/630822

Detailed context for AI agents

Symptom

Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/630822

Customer's MR widget never loads. The GraphQL query for MergeRequest.detailedMergeStatus hits the 30s server timeout: "Timeout on MergeRequest.detailedMergeStatus". Profiling showed ~35s CPU, 0 Gitaly calls, ~54 DB queries, ~983k objects allocated, and no per-check mergeability log lines at all (i.e. DetailedMergeStatusService never ran).

Root cause

ee/app/graphql/ee/types/merge_request_type.rb wraps both the mergeable and detailed_merge_status fields in a private lazy_committers helper. When object.merge_requests_disable_committers_approval? is true, it eagerly calls object.commits.add_committers_to_batch_loader(with_merge_commits: true) before returning a Gitlab::Graphql::Lazy.

object.commits routes through MergeRequestDiff#load_commits -> merge_request_diff_commits.with_users, which preloads the instance-dependent belongs_to :merge_request_commits_metadata association. Rails builds one loader per row, so the preload cost is O(commits^2). Measured locally on GDK:

  • 500 commits: 0.3s
  • 1,000 commits: 1.1s
  • 2,000 commits: 3.6s
  • 3,000 commits: 8.1s
  • 5,000 commits: 23.1s

StackProf showed 92% of samples inside ActiveRecord's Preloader::Association::LoaderRecords#load_records.

Why the timeout is attributed to the field itself, with no per-check logs

graphql-ruby's Timeout checks the deadline in begin_execute_field, and the runtime checks again in after_lazy when syncing a lazy field's result. The eager commit load burns the whole time budget, so the deadline check trips during the lazy sync, before DetailedMergeStatusService (the per-check mergeability logic) ever gets to run. By contrast, mergeabilityChecks has no such wrapper and returns in ~1s on gitlab.com for the same MR.

Why the wrapper is dead code

lazy_committers was added in 2024 (commit 9e7ef142, "Reduce N+1 queries when approvals by committers are not allowed") to batch User.by_any_email lookups consumed via CommitCollection#committers(lazy: true).

Since !223603 (merged) and its follow-ups (!243852 (closed), !243855 (merged)), the approval committer filter (MergeRequest#committers_to_filter_from_approvers / committer_ids_to_filter_from_approvers) reads committer_users_from_diff -> committer_emails_from_diff, which:

  • runs a SQL query when the approval_committer_emails_from_diff flag is on (100% rolled out on gitlab.com since 2026-06-30), or
  • falls back to preload_commits_metadata + commits.committer_emails otherwise.

Neither path reads the BatchLoader that lazy_committers pre-warms, so the eager load produces objects nobody consumes. The only remaining caller of committers(lazy: true) is MergeRequests::MergeAuditEventService, which is unaffected by this change.

Change

  • Delete lazy_committers and the mergeable / detailed_merge_status field overrides in ee/app/graphql/ee/types/merge_request_type.rb. Both fields fall back to the CE resolvers (method: :mergeable? and Types::MergeRequestType#detailed_merge_status).
  • Delete the now-unused alias_method :add_committers_to_batch_loader, :committers_lazy in app/models/commit_collection.rb. committers_lazy itself is untouched (still used by MergeAuditEventService).
  • Add a when committers cannot approve context to the #mergeable and #detailed_merge_status describes in ee/spec/graphql/ee/types/merge_request_type_spec.rb, re-running the existing field with approval rules related check N+1 shared example with merge_requests_disable_committers_approval: true. With the query threshold forced to 0, the committer-disabled contexts show the same per-field deltas as the baseline contexts (+2 / +2 / +4 across the three N+1 scenarios), so the setting adds no per-MR queries once the wrapper is gone. This replaces an earlier spec that only asserted #commits was never called, per reviewer feedback.
  • Not changed: applying the same context to approved, approvalState and mergeabilityChecks fails, because the committer-email query and User.by_any_email run once per MR there. Those fields never had lazy_committers, so that N+1 is pre-existing and out of scope here.

No feature flag: behaviour is identical on both sides of approval_committer_emails_from_diff, since neither path ever depended on the deleted eager load.

Verification

GDK repro: project with a 5,000-commit MR and merge_requests_disable_committers_approval enabled.

  • Real GraphQL request { iid detailedMergeStatus }: ~49s with a timeout error before the fix.
  • In-process GitlabSchema.execute for the same query: 22-29s before, 0.04s after, same NOT_APPROVED result both times.

Specs:

  • ee/spec/graphql/ee/types/merge_request_type_spec.rb: #mergeable and #detailed_merge_status describes green (32 examples, including the new committers-cannot-approve N+1 contexts); full file green before the spec rework (67 examples).
  • spec/models/commit_collection_spec.rb: 31 examples green.
  • Rubocop clean.
  • !257894 (merged) (Draft) removes the same quadratic preload from the lookahead preloads used by list resolvers (mergeRequests { nodes }). It does not cover the single mergeRequest(iid:) field, since lookahead preloads only apply when a nodes selection exists, and it leaves lazy_committers in place. The two MRs are complementary, not overlapping.
  • The underlying O(n^2) preload of the instance-dependent merge_request_commits_metadata association is out of scope for this MR and is tracked separately in https://gitlab.com/gitlab-org/gitlab/-/work_items/629382 and #608151 (closed).
  • Customer workaround until this deploys: disable "Prevent approvals by users who add commits" on the project, or reduce the commit count (squash/rebase). Diff size is irrelevant to the timeout.
  • Not in scope: fixing the quadratic preload at the association level; any frontend changes to the MR widget query.
Edited by Marc Shaw

Merge request reports

Loading
Loading