Skip to content

[Devops Adoption] Fix security scan query timeouts

Pavel Shutsin requested to merge 335190-fix-security-scan-query-timeout into master

What does this MR do?

Tries to fix query timeout which occurred for security scan query.

Migration output

== 20210708124229 AddSecurityScansCreatedAtIndex: reverting ===================
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:security_scans, :created_at, {:algorithm=>:concurrently})
   -> 0.0102s
-- remove_index(:security_scans, {:algorithm=>:concurrently, :column=>:created_at})
   -> 0.0102s
== 20210708124229 AddSecurityScansCreatedAtIndex: reverted (0.0225s) ==========

== 20210708124229 AddSecurityScansCreatedAtIndex: migrating ===================
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:security_scans, :created_at, {:algorithm=>:concurrently})
   -> 0.0082s
-- add_index(:security_scans, :created_at, {:algorithm=>:concurrently})
   -> 0.0110s
== 20210708124229 AddSecurityScansCreatedAtIndex: migrated (0.0220s) ==========

Index creation takes about 20 seconds in #database-lab so I think it's safe to keep it in regular migration

Query explains

Technically we can improve more by adding index to ci_builds but it's extremely big table and adding indexes there takes really lots of time (around 14h in #database-lab). So I feel like the effort and index costs doesn't worth it compared to 1 monthly query improvement. New query returns small number of records so filtering out shouldn't be a dramatic loss of performance.

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

Does this MR contain changes to processing or storing of credentials or tokens, authorization and authentication methods or other items described in the security review guidelines? If not, then delete this Security section.

  • 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

Related to #335190 (closed)

Edited by Pavel Shutsin

Merge request reports