Skip to content

Add deduplicated column to security findings and arrange indices

What does this MR do?

This MR adds a new boolean column into security_findings table called deduplicated along with a compound index defined on scan_id and deduplicated field. Since PostgreSQL can use the first component of the index individually, the index on scan_id becomes unnecessary therefore removed.

Related to #238156

Migration Outputs

Adding the new column

rake db:migrate:up VERSION=20200914155135

== 20200914155135 AddDeduplicatedFlagIntoSecurityFindingsTable: migrating =====
-- add_column(:security_findings, :deduplicated, :boolean, {:default=>false, :null=>false})
   -> 0.0030s
== 20200914155135 AddDeduplicatedFlagIntoSecurityFindingsTable: migrated (0.0031s)

rake db:migrate:down VERSION=20200914155135

== 20200914155135 AddDeduplicatedFlagIntoSecurityFindingsTable: reverting =====
-- remove_column(:security_findings, :deduplicated, :boolean, {:default=>false, :null=>false})
   -> 0.0015s
== 20200914155135 AddDeduplicatedFlagIntoSecurityFindingsTable: reverted (0.0035s)
Adding the compound index on scan_id and deduplicated

rake db:migrate:up VERSION=20200914183227

== 20200914183227 AddIndexOnDeduplicatedColumnOfSecurityFindings: migrating ===
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:security_findings, [:scan_id, :deduplicated], {:name=>"index_security_scans_on_scan_id_and_deduplicated", :algorithm=>:concurrently})
   -> 0.0036s
-- add_index(:security_findings, [:scan_id, :deduplicated], {:name=>"index_security_scans_on_scan_id_and_deduplicated", :algorithm=>:concurrently})
   -> 0.0059s
== 20200914183227 AddIndexOnDeduplicatedColumnOfSecurityFindings: migrated (0.0099s)

rake db:migrate:down VERSION=20200914183227

== 20200914183227 AddIndexOnDeduplicatedColumnOfSecurityFindings: reverting ===
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:security_findings, [:scan_id, :deduplicated], {:name=>"index_security_scans_on_scan_id_and_deduplicated", :algorithm=>:concurrently})
   -> 0.0037s
-- remove_index(:security_findings, {:name=>"index_security_scans_on_scan_id_and_deduplicated", :algorithm=>:concurrently, :column=>[:scan_id, :deduplicated]})
   -> 0.0045s
== 20200914183227 AddIndexOnDeduplicatedColumnOfSecurityFindings: reverted (0.0087s)
Removing the index on scan_id

rake db:migrate:up VERSION=20200914184212

== 20200914184212 RemoveIndexOnSecurityFindingsScanId: migrating ==============
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:security_findings, :scan_id, {:algorithm=>:concurrently})
   -> 0.0035s
-- remove_index(:security_findings, {:algorithm=>:concurrently, :column=>:scan_id})
   -> 0.0062s
== 20200914184212 RemoveIndexOnSecurityFindingsScanId: migrated (0.0101s) =====

rake db:migrate:down VERSION=20200914184212

== 20200914184212 RemoveIndexOnSecurityFindingsScanId: reverting ==============
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:security_findings, :scan_id, {:algorithm=>:concurrently})
   -> 0.0036s
-- add_index(:security_findings, :scan_id, {:algorithm=>:concurrently})
   -> 0.0030s
== 20200914184212 RemoveIndexOnSecurityFindingsScanId: reverted (0.0070s) =====

The security_findings table has around 13M records.

EXPLAIN SELECT id FROM security_findings;
Index Only Scan using security_findings_pkey on public.security_findings  (cost=0.43..355794.95 rows=13378409 width=8) (actual time=5.866..17347.980 rows=13375978 loops=1)
   Heap Fetches: 1958834
   Buffers: shared hit=70851 read=61816 dirtied=25217
   I/O Timings: read=12203.358

https://explain.depesz.com/s/dNOd

Screenshots

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 Mehmet Emin INAC

Merge request reports