Share group and policy configurations across approver batches

What does this MR do and why?

Security::ScanResultPolicies::AddApproversToRulesWorker processes up to 100 projects per job but loops per project, and each iteration reloaded state that's actually shared across the job: every project's group and namespace associations and all_security_orchestration_policy_configurations returned fresh instances, so state memoized on those instances (group ancestor ids, the organization policy setting, each configuration's namespace settings used for the approval-policy limit, the namespace plan) was recomputed per project instead of reused across siblings in the same group or namespace. Project#compliance_framework_ids also queried compliance_framework_settings once per policy per configuration for each project.

This MR:

  • Changes the worker to iterate projects(event).with_namespace.with_group.with_compliance_framework_settings.find_each (all three are existing Project scopes).
    • with_group: projects in the same group share one group instance.
    • with_namespace: Project#licensed_feature_available? (via Security::PolicyAvailability.available? in process_project?) reads project.namespace, a separate association from group, so each project loaded its namespace fresh. On GitLab.com (namespace plan checks enabled) that also repeated the root_ancestor, gitlab_subscriptions and plans lookups per project. Projects in the same namespace now share one instance with its memoized plan.
    • with_compliance_framework_settings: PolicyScopeChecker#applicable_for_compliance_framework? calls Project#compliance_framework_ids (a compliance_framework_settings.pluck(:framework_id)) once per policy per configuration for each project. With the association preloaded, pluck reads the loaded records instead of querying. In the heavy scenario below this was 125 of 264 queries for 10 projects, the biggest remaining repeat.
  • Adds an optional configurations_by_id: keyword (default {}) to Security::ScanResultPolicies::AddApproversToRulesService. The worker builds one hash per job and passes it in; the service maps each project's configurations onto the first instance seen for that id.

Local measurement (uncommitted QueryRecorder benchmark: group > subgroup > subgroup, 100 authorized users, an open MR with approval rules per project, Rails cache enabled as in production). Per additional project is the difference between batches of 2 and 10 projects, savepoints excluded:

Scenario Before this MR First commit only This MR
Self-managed, 2 configurations, 3 unscoped policies 14.8 8.8 9.0
GitLab.com plan checks, 2 configurations, 3 policies scoped by group / compliance framework 20.8 14.8 9.0
GitLab.com plan checks, 3 configurations × 5 policies scoped by group and compliance framework 31.5 24.5 8.0

Exact duplicate queries per additional project dropped to 1 in all three scenarios (from 7.0 / 11.0 / 22.0). For a single project in the heavy scenario: 51 queries with 15 exact duplicates before, 37 with 2 after. Inserted approver rows were identical before and after in every scenario. Per-project cost no longer grows with the number of policies or configurations.

Remaining per-project queries (about 8), not addressed here: the configurations query plus its namespace preload (with_project_and_namespace in Project#security_orchestration_policies_for_scope), the username lookup, one approval-rules query per configuration, and the two inserts.

The local setup still does not reproduce production's ~83.6 queries per project, so the production impact has to be confirmed in Sidekiq logs (db_main_count, db_main_cached_count for this worker).

Reviewable LOC: 24 (plus 55 in tests)

Context for LLM agents

Rejected alternatives:

  • Resolving usernames once per batch, as the issue suggests: project.team.users.id_in(user_ids) joins project_authorizations on the project id, so the generated SQL differs per project — it is not a duplicate query, and the membership filter is still needed per project.
  • Adding :group to the with_scan_result_policy_reads scope: that scope is also used by dispatch?, which runs on the publisher side, and would add a query there for no benefit.
  • Caching all_security_orchestration_policy_configurations per parent-group set to remove the remaining duplicate: it changes a Project method with many callers, to save one query per project.
  • Memoizing Project#compliance_framework_ids: preloading in the worker removes the queries without changing a method with other callers.

Invariants:

  • configurations_by_id is shared only within a single job, never across jobs.
  • Configuration instances memoize per-project results (strong_memoize_with keyed by project) and must not outlive the job.
  • Behavior is unchanged: the same set of valid configurations is used per project.

References

Related to https://gitlab.com/gitlab-org/gitlab/-/work_items/629415. The issue stays open for the remaining per-project configuration query and for verification in production Sidekiq logs.

Screenshots or screen recordings

Not applicable: backend only.

Before/after query counts per additional project (local measurement as described above):

Scenario Before this MR First commit only This MR
Self-managed, 2 configurations, 3 unscoped policies 14.8 8.8 9.0
GitLab.com plan checks, 2 configurations, 3 policies scoped by group / compliance framework 20.8 14.8 9.0
GitLab.com plan checks, 3 configurations × 5 policies scoped by group and compliance framework 31.5 24.5 8.0

How to set up and validate locally

Run the worker and service specs:

bin/rspec ee/spec/workers/security/scan_result_policies/add_approvers_to_rules_worker_spec.rb ee/spec/services/security/scan_result_policies/add_approvers_to_rules_service_spec.rb

47 examples, 0 failures.

In ee/spec/workers/security/scan_result_policies/add_approvers_to_rules_worker_spec.rb, the example "with multiple projects sharing a policy configuration" uses a policy scoped to a compliance framework assigned to both projects. It asserts users are added to each project's rule. The "loads group and configuration state once for the batch" example asserts organizations, security_policy_settings, namespace_settings and project_compliance_framework_settings are each queried once. The project_compliance_framework_settings assertion fails without the preload and passes with it. Existing expectations were updated for the new configurations_by_id: constructor kwarg.

To see the effect after deploy: compare db_main_count and db_main_cached_count per job for this worker in Sidekiq logs.

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.

Edited by Alan (Maciej) Paruszewski

Merge request reports

Loading
Loading