Use preloaded violations in the security policy check

What does this MR do and why?

List endpoints already preload every scan result policy violation of a merge request, but the security policy violations mergeability check still queried the failed violations, the dismissals and the bypass policies again for each merge request.

Behind security_policy_violations_check_use_preloads:

  • failed violations come from the preloaded set,
  • the dismissal query is skipped when a merge request has no violations,
  • the bypass policy lookup is skipped when the project has no approval policies with bypass settings.

On GitLab.com this check takes about 8,200 seconds per hour (6.7%) of GET /groups/:id/merge_requests.

References

Screenshots or screen recordings

Not a UI change.

How to set up and validate locally

  1. Enable the flag in a Rails console:
    Feature.enable(:security_policy_violations_check_use_preloads)
  2. In a project with a merge request approval policy, open merge requests with and without policy violations.
  3. Call the group or project merge request list with the flag on and off and compare detailed_merge_status. They should be identical, including security_policy_violations for merge requests with failed violations that are not approved:
    curl --header "PRIVATE-TOKEN: <token>" "http://gdk.test:3000/api/v4/projects/<id>/merge_requests?state=opened"

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Detailed context for AI agents

Change (ee/app/services/merge_requests/mergeability/check_security_policy_violations_service.rb)

  • failed_violations?: when the flag is on and merge_request.scan_result_policy_violations is loaded (it is in with_api_entity_associations), use violations.any?(&:failed?) instead of failed_scan_result_policy_violations.any?. The association is unscoped and failed is the status enum value, so the result is the same.
  • policy_violations_dismissed?: returns false early when the preloaded violations are empty. The query it skips joins violations to dismissals, so it cannot match without violations.
  • security_policy_bypassed?: returns false when the project has no approval_policies.with_bypass_settings. The exists? SQL is the same for every merge request in a project, so on list pages it is a Rails query cache hit after the first merge request of each project (the load balancer enables the query cache on replicas in ensure_caching!). An earlier revision memoized it in Gitlab::SafeRequestStore; that was dropped because the query cache already dedupes it and the memo only saved building the relation. security_policies_with_bypass_settings is a subset of those policies, so an empty project set means no bypass.
  • Running violations already come from a preloaded association and are unchanged. The resync path (missing_policy_approval_rules?) is unchanged.
  • When the violations are not preloaded (single merge request reads, Sidekiq), the failed-violation and dismissal queries run as before. The bypass short-circuit applies whenever the flag is on, preloaded or not.
  • Flag off: master behaviour, apart from the Feature.enabled? call.

Queries removed per merge request (with policy approval rules)

  • 1 SELECT 1 ... scan_result_policy_violations ... status = 1 LIMIT 1
  • 1 dismissal JOIN query when there are no violations
  • 1 policy query plus its preload when the project has no bypass-enabled policies

Added: 1 EXISTS on the project's bypass-enabled approval policies per merge request, served from the query cache after the first merge request of each project.

Verification

  • ee/spec/services/merge_requests/mergeability/check_security_policy_violations_service_spec.rb: new with preloaded violations context using MergeRequest.with_api_entity_associations asserts the status and that the failed-violations, dismissal and bypass queries are not issued, a flag-off example that the failed query is issued, and a dismissed-violation example that keeps the dismissal query when the preloaded set is not empty. A mutation run without the preloaded paths fails two of them, and dropping the empty? from the dismissal guard fails the dismissal example.
  • A temporary break matrix (11 violation, dismissal and bypass states, flag on and off, preloaded and not) returned the same status as master in every case. The one difference it found (a bypass policy created later in the same request was not seen) came from the request-store memo, which has since been removed.

Production context

Measured on GitLab.com (Kibana, 2026-10-19) for GET /api/:version/groups/:id/merge_requests:

  • About 139k requests and 122k seconds of request time per hour. The per-MR mergeability checks behind detailed_merge_status take 40.6k of those seconds (33%), read from the per-MR application_json lines that share the request's correlation_id (mergeability.<check>.duration_s.values).
  • Seconds per hour by check: commits_status 11,174, approved 9,148, security_policy_violations 8,238, conflict_status 4,157, path_locks 2,100, lfs_file_locks 1,980, discussions_status 1,453, rebase_status 1,317.
  • Most of the load comes from bots polling state=opened&per_page=100 every few minutes: 53% of the endpoint's server time, with typical pages spanning 30 to 90 projects.

The investigation and the other changes from it are tracked in #631601. This is one of four independent drafts, each behind its own gitlab_com_derisk flag.

Edited by Marc Shaw

Merge request reports

Loading
Loading