Fix timeout in approval checks by querying committer emails via SQL

Marc Shawrequested to merge
fix/avoid-expensive-commit-instantiation-in-approval-checks into master

Summary

  • The approval mergeability check (committers_to_filter_from_approvers, committer_ids_to_filter_from_approvers) was calling committers() which instantiates all MergeRequestDiffCommit Ruby objects, preloads associations, and converts each to a Commit — just to extract committer emails. For MRs with thousands of commits, this caused 60s+ CPU timeouts.
  • Introduces committer_emails_from_diff which queries committer emails directly via an Arel-built UNION query (merge_request_diff_commits → merge_request_commits_metadata → merge_request_diff_commit_users), bypassing all Ruby object instantiation.
  • Only the approval-specific methods use the new query path. The general committers() method is left unchanged for other callers.
  • Gated behind the approval_committer_emails_from_diff feature flag (gitlab_com_derisk, default disabled).

Why the SQL-only approach is safe

The approval callers invoke committers(with_merge_commits: true, include_author_when_signed: true). These parameters have the following effects:

  1. with_merge_commits: true — This skips the without_merge_commits filter entirely (CommitCollection#committers_emails line 172 returns early), so all commits are included. Our SQL query also includes all commits — equivalent behavior.
  2. include_author_when_signed: true — This causes committer_emails_for to check commit.signature&.verified_system? and return author_email instead of committer_email when true. However, Commit.from_hash creates commits without Gitaly backing, so raw_signature_type returns nil → signature returns nil → verified_system? is never reached. The code always falls through to committer_email. Our SQL query extracts the same committer emails directly.
  3. Maps committer emails to User records — this is the only meaningful work. The new query extracts the same emails directly from the database and passes them to User.by_any_email, producing identical results without instantiating thousands of intermediate Ruby objects.

SQL Queries

Edited by Marc Shaw

Merge request reports