Eliminate LEFT JOIN in FindingsFinder by adding overridden_severity column to security_findings

Summary

The Security::FindingsFinder query performs an expensive LEFT JOIN to vulnerability_occurrences solely to check for severity overrides. This JOIN is a significant contributor to high CPU utilization in ReactiveCachingWorker jobs processing security findings (see gitlab-com/request-for-help#4212).

By adding a scanner_reported_severity column to security_findings and updating the ingestion logic, we can eliminate this JOIN entirely.

Problem

The current query in ee/app/finders/security/findings_finder.rb (lines 50-56):

lateral_relation = Security::Finding
  .left_joins_vulnerability_finding
  .where('"security_findings"."scan_id" = "security_scans"."id"')
  .where(
    'COALESCE("vulnerability_occurrences"."severity", "security_findings"."severity") = "severities"."severity"'
  )

The LEFT JOIN exists because:

  • security_findings.severity = original severity from scanner
  • vulnerability_occurrences.severity = potentially overridden severity by user
  • The COALESCE prefers the overridden severity when it exists

This JOIN to vulnerability_occurrences (a large, frequently-updated table) is expensive.

Proposal

Add a scanner_reported_severity column to store the original scanner value, and update severity to always hold the effective value (user override if exists, otherwise scanner value).

Revised Approach (per @minac feedback)

  • severity = the effective severity (user-overridden value if one exists, otherwise scanner value)
  • scanner_reported_severity = original value from the scan report

This eliminates the need for COALESCE since severity always contains the effective value.

Schema Change 

add_column :security_findings, :scanner_reported_severity, :smallint

Storage Impact

< 1% increase in table size (2 bytes per row for smallint column).

Code Changes

1. StoreFindingsService - Lookup overrides during ingestion 

# ee/app/services/security/store_findings_service.rb

def store_finding_batch(batch)
  overrides = lookup_severity_overrides(batch.map(&:uuid)) if Feature.enabled?(:security_findings_effective_severity, project)
  
  batch.map { |finding| finding_data(finding, overrides&.dig(finding.uuid)) }
       .then { import_batch(_1) }
  # ...
end

def finding_data(report_finding, overridden_severity = nil)
  scanner_severity = report_finding.severity
  {
    # ... existing fields ...
    scanner_reported_severity: scanner_severity,
    severity: overridden_severity || scanner_severity,  # effective value
  }
end

def lookup_severity_overrides(uuids)
  return {} if uuids.empty?

  Vulnerabilities::Finding
    .where(uuid: uuids)
    .joins(vulnerability: :severity_overrides)
    .distinct
    .pluck(:uuid, :severity)
    .to_h
end

2. FindingsFinder - Remove the JOIN 

# ee/app/finders/security/findings_finder.rb

# Before (with JOIN and COALESCE):
.left_joins_vulnerability_finding
.where('COALESCE("vulnerability_occurrences"."severity", "security_findings"."severity") = "severities"."severity"')

# After (no JOIN, no COALESCE):
.where('"security_findings"."severity" = "severities"."severity"')

Implementation Phases

Phase 1: Schema and Model Changes

  • Add scanner_reported_severity column to security_findings (migration)
  • Add feature flag: security_findings_effective_severity

Phase 2: Write Path - Ingestion Update

  • Update StoreFindingsService#store_finding_batch to lookup existing overrides
  • Update StoreFindingsService#finding_data to set both columns
  • Add specs for override lookup during ingestion

Phase 3: Read Path Optimization

  • Update FindingsFinder to remove LEFT JOIN (behind feature flag)
  • Remove COALESCE, filter directly on severity
  • Add specs for finder optimization

Phase 4: Rollout and Validation

  • Enable feature flag on staging
  • Monitor query performance
  • Enable on GitLab.com percentage rollout
  • Remove feature flag after validation

Key Design Decisions

No Backfill Migration Needed

The security_findings table has 30-90 day retention. Old data without the new column will naturally cycle out. Any project that runs a new pipeline will get fresh data with both columns populated correctly.

Eventual Consistency is Acceptable

When a user overrides a severity:

  1. The override is recorded in vulnerability_severity_overrides
  2. vulnerabilities.severity and vulnerability_occurrences.severity are updated
  3. Existing ephemeral security_findings records are NOT updated
  4. The next pipeline run picks up the override during ingestion

This means users must re-run the pipeline to see severity changes reflected in the MR security widget. This is acceptable because:

  • The MR widget shows findings from the current pipeline
  • Re-running the pipeline is a natural workflow action
  • This significantly simplifies the implementation

Works with Branch-Context-Aware UUIDs

Ongoing work is upgrading UUID generation to factor in branch/tracked context (see !220835 (merged), !220866, !220869). The severity override lookup by UUID will naturally scope to the correct tracked context.

Expected Results

Query plan improvement:

# Before (with JOIN):
Nested Loop Left Join
  -> Index Scan on security_findings
  -> Index Scan on vulnerability_occurrences  <- EXPENSIVE

# After (no JOIN):
Index Scan on security_findings  <- MUCH FASTER

Expected 50-80% reduction in query time for the FindingsFinder lateral query.

Related

/cc @minac @rwells

Edited by 🤖 GitLab Bot 🤖