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
- Enable the flag in a Rails console:
Feature.enable(:security_policy_violations_check_use_preloads) - In a project with a merge request approval policy, open merge requests with and without policy violations.
- Call the group or project merge request list with the flag on and off and compare
detailed_merge_status. They should be identical, includingsecurity_policy_violationsfor 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 andmerge_request.scan_result_policy_violationsis loaded (it is inwith_api_entity_associations), useviolations.any?(&:failed?)instead offailed_scan_result_policy_violations.any?. The association is unscoped andfailedis thestatusenum 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 noapproval_policies.with_bypass_settings. Theexists?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 inensure_caching!). An earlier revision memoized it inGitlab::SafeRequestStore; that was dropped because the query cache already dedupes it and the memo only saved building the relation.security_policies_with_bypass_settingsis 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: newwith preloaded violationscontext usingMergeRequest.with_api_entity_associationsasserts 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 theempty?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_statustake 40.6k of those seconds (33%), read from the per-MRapplication_jsonlines that share the request'scorrelation_id(mergeability.<check>.duration_s.values). - Seconds per hour by check:
commits_status11,174,approved9,148,security_policy_violations8,238,conflict_status4,157,path_locks2,100,lfs_file_locks1,980,discussions_status1,453,rebase_status1,317. - Most of the load comes from bots polling
state=opened&per_page=100every 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.