Skip to content

Batch-load vulnerability findings by UUID

Matthias Käppler requested to merge 323059-batch-load-findings-by-uuid into master

What does this MR do?

Related to #323059 (closed)

This prevents an N+1 where the StoreReportService would previously query for a Finding by UUID one-by-one only to fall back on a different query/creation when not found.

Instead, we now pre-load findings into a Hash by UUID, and only fall back in case we haven't already found it.

Improvements

When storing SAST reports, as measured by StoreReportServiceSpec:

  • SQL calls before this change: 1181
  • SQL calls after this change: 1149

Difference: 32

This math checks out, since there were 33 findings processed by this spec, which all caused one query-by-uuid before, now it is one query for all findings i.e. 33 queries saved, one added, = 32 queries saved.

Impact

The impact of this depends on a number of factors:

  • how many findings we are able to pre-fetch by UUID; if there are 0, this improvement is useless
  • how many findings there are per report; the more findings, the higher the impact
  • how many reports there are per build; the more reports, the higher the impact

For example, assuming that:

  • there are 3 builds
  • with 10 reports
  • with 50 findings each
  • and 50% being found by UUID

Then the reduction in queries is 3 * 10 * 50 * 0.5 = 750

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Matthias Käppler

Merge request reports