Bound vulnerability consistency check sweeps to keyset windows

What does this MR do and why?

Vulnerabilities::ConsistencyChecks::TriggerParityCheck re-checks a project's vulnerability_reads against the source vulnerability_occurrences / vulnerabilities and repairs any drift (uuid, scanner_id, occurrence_id, has_issues, has_merge_request, and missing reads). It runs after every security-report ingestion, gated on the vulnerability_consistency_checks feature flag (rollout #592117).

On production it has been failing on gitlab-org/gitlab only, with PG::QueryCanceled: canceling statement due to statement timeout (144 failures over 7 days).

Root cause

Each fix_* method was driven by a whole-project scoping query and repaired the whole project in a single statement. On the largest project that statement could not stay under the statement timeout: the driving filters were not sargable, so a single statement's worth of work grew with the project's vulnerability_reads count.

This is not caused by vulnerabilities_across_contexts. vulnerability_reads has a unique index on vulnerability_id, so it is still one row per vulnerability. The trigger is simply that gitlab-org/gitlab's tables have grown past the point where an unbounded whole-project sweep completes within the statement timeout.

What changed

  • Bound each sweep to a keyset window. The check still does a full whole-project re-check, but it now enumerates vulnerability_reads in keyset windows over vulnerability_id (of: 1000) and bounds each corrective UPDATE with vulnerability_id BETWEEN <min> AND <max> (plus the existing project_id filter). The outer driver of every statement is therefore an index range scan on index_vulnerability_reads_on_project_id_and_vulnerability_id, so no single statement scans the whole project. create_missing_vulnerability_reads keysets over the Vulnerability primary key. This is the change that removes the timeout.
  • Key the link-table probes on vulnerability_id. The has_issues / has_merge_request repairs express their EXISTS against vulnerability_issue_links / vulnerability_merge_request_links on vulnerability_id (not vulnerability_occurrence_id), matching the DB triggers and the ingestion writer, which is the correct parity semantics under VAC (see thread discussion). vulnerability_id is already indexed on both link tables, so no new index is needed.
  • Per-project throttle. Ingestion runs on every pipeline, so on active projects a full sweep was enqueued constantly. The orchestrator now wraps only the async sweep checks in Gitlab::Utils::RedisThrottle.execute_every (a SET NX EX cooldown on Gitlab::Redis::SharedState), keyed per project, COOLDOWN_PERIOD = 24.hours. The synchronous HasVulnerabilitiesIsCorrectCheck runs on every ingestion, outside the throttle. A whole-project consistency sweep is a safety net for drift, so a daily cadence is plenty.
  • Failure isolation. fix! now runs every step even if one raises, then re-raises the first error, so a single timeout no longer prevents the other repairs from running while the job still visibly fails. The exception detail is routed through Gitlab::ErrorTracking.track_exception rather than logged as a per-step backtrace array.

The fix semantics, the repaired columns, the exec_update return-based counts, and all log('Fixed ...', count:) / log('Created missing ...', count:) message strings are unchanged (the rollout runbook watches those strings).

Database

There are no schema changes in this MR. The fix is a query-shape change (keyset windowing + keying the link-table EXISTS probes on the already-indexed vulnerability_id) plus a Redis throttle.

An earlier revision added two post-deploy indexes on vulnerability_issue_links (vulnerability_occurrence_id) and vulnerability_merge_request_links (vulnerability_occurrence_id). Those were dropped: the probes were re-keyed onto vulnerability_id, which is already covered by idx_vulnerability_issue_links_on_vulnerability_id_and_issue_id and unique_vuln_merge_request_link_vuln_id_and_mr_id, so no vulnerability_occurrence_id index is needed and no other caller reads those tables filtered by vulnerability_occurrence_id.

Re-validated plans (Database Lab, gitlab-production-sec, PG17, snapshot 2026-09-02)

Measured on gitlab-org/gitlab (project 278964, ~6,200 vulnerability_reads, ~7 windows of 1,000), for a single keyset window (vulnerability_id BETWEEN 36027627 AND 280708360), wrapped in BEGIN … ROLLBACK:

  • has_issues UPDATE: outer driver is the keyset range Index Scan using index_vulnerability_reads_on_project_id_and_vulnerability_id (1,000 rows); the EXISTS resolves as a correlated per-row Index Only Scan using idx_vulnerability_issue_links_on_vulnerability_id_and_issue_id. Execution ~7 ms warm (~730 ms cold, dominated by first-touch I/O), cost ≈ 13,313.
  • has_merge_request UPDATE (IS DISTINCT FROM): same keyset range driver; the EXISTS resolves as a correlated per-row Index Only Scan using unique_vuln_merge_request_link_vuln_id_and_mr_id. Execution ~10 ms warm, cost ≈ 16,738.

Note on the earlier revision's justification: with the vulnerability_id keying, the EXISTS is not planned as a hashed SubPlan / semijoin that hashes the whole link table. It is a correlated per-row index-only probe against the covering vulnerability_id index — cheap precisely because the index covers it. The per-window cost is therefore bounded by the keyset range (≈1,000 index-only probes), and each statement stays well under the statement timeout.

Sibling checks

The other orchestrator checks were reviewed and left unchanged:

  • HasVulnerabilitiesIsCorrectCheck: single-row read + update, no per-row full-project scan; runs synchronously on every ingestion, outside the throttle.
  • BackfillIssueLinkOccurrenceIdCheck / BackfillMergeRequestLinkOccurrenceIdCheck: batch over the link-table primary key with an index-usable IS NULL filter, and are VAC-gated (early-return now that VAC is at 100%).

Test plan

  • bundle exec rspec ee/spec/lib/vulnerabilities/consistency_checks/trigger_parity_check_spec.rb ee/spec/lib/vulnerabilities/consistency_checks/orchestrator_spec.rb ee/spec/lib/vulnerabilities/consistency_checks_spec.rb (all green locally).
  • Specs cover the bounded multi-window behaviour across every fix_* method and create_missing_vulnerability_reads (counts/semantics unchanged), cross-project isolation, the failure isolation, and the throttle scoping.

MR acceptance checklist

  • Database review of the rewritten has_issues / has_merge_request UPDATEs (now keyed on vulnerability_id) and the keyset windowing (no schema changes; re-validated plans above)
  • Confirmed fix semantics and log message strings unchanged
  • Re-check the consistency-check convergence on production after deploy before resuming the percentage rollout
Edited by Gregory Havenga

Merge request reports

Loading
Loading