Use calculated status for DastSiteProfile query (!44133) · Merge requests · GitLab.org / GitLab

Philip Cunningham requested to merge replace-stubbed-dast-site-validation-status-245214 into master Oct 02, 2020

What does this MR do?

replaces the stubbed validationStatus with the one via associated dast_site_validation.

Issue(s)

Database

we're amending an existing scope to eager load from another, related, table (dast_site_validations) and adding a limit when filtering by the pkey.

Before

By `project_id`

SELECT "dast_site_profiles".* FROM "dast_site_profiles" WHERE "dast_site_profiles"."project_id" = 19936172;
SELECT "dast_sites".* FROM "dast_sites" WHERE "dast_sites"."id" IN (105, 106, 107, 108, 7, 13, 154, 1);

 Index Scan using index_dast_site_profiles_on_project_id_and_name on public.dast_site_profiles  (cost=0.14..4.89 rows=14 width=51) (actual time=11.140..11.145 rows=12 loops=1)
   Index Cond: (dast_site_profiles.project_id = 19936172)
   Buffers: shared hit=3 read=2
   I/O Timings: read=11.081

Time: 11.308 ms
  - planning: 0.129 ms
  - execution: 11.179 ms
    - I/O read: 11.081 ms
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 3 (~24.00 KiB) from the buffer pool
  - reads: 2 (~16.00 KiB) from the OS file cache, including disk I/O
  - dirtied: 0
  - writes: 0

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

 Index Scan using dast_sites_pkey on public.dast_sites  (cost=0.14..6.06 rows=8 width=70) (actual time=2.980..3.974 rows=8 loops=1)
   Index Cond: (dast_sites.id = ANY ('{105,106,107,108,7,13,154,1}'::bigint[]))
   Buffers: shared hit=11 read=3
   I/O Timings: read=3.888

Time: 4.171 ms
  - planning: 0.165 ms
  - execution: 4.006 ms
    - I/O read: 3.888 ms
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 11 (~88.00 KiB) from the buffer pool
  - reads: 3 (~24.00 KiB) from the OS file cache, including disk I/O
  - dirtied: 0
  - writes: 0

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

By `id`

SELECT "dast_site_profiles".* FROM "dast_site_profiles" WHERE "dast_site_profiles"."id" = 2;
SELECT "dast_sites".* FROM "dast_sites" WHERE "dast_sites"."id" = 1;

 Index Scan using dast_site_profiles_pkey on public.dast_site_profiles  (cost=0.14..3.16 rows=1 width=51) (actual time=1.197..1.199 rows=1 loops=1)
   Index Cond: (dast_site_profiles.id = 2)
   Buffers: shared hit=3 read=2
   I/O Timings: read=1.124

Time: 1.373 ms
  - planning: 0.143 ms
  - execution: 1.230 ms
    - I/O read: 1.124 ms
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 3 (~24.00 KiB) from the buffer pool
  - reads: 2 (~16.00 KiB) from the OS file cache, including disk I/O
  - dirtied: 0
  - writes: 0

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

 Index Scan using dast_sites_pkey on public.dast_sites  (cost=0.14..3.16 rows=1 width=70) (actual time=0.025..0.026 rows=1 loops=1)
   Index Cond: (dast_sites.id = 1)
   Buffers: shared hit=2

Time: 0.160 ms
  - planning: 0.112 ms
  - execution: 0.048 ms
    - I/O read: 0.000 ms
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 2 (~16.00 KiB) from the buffer pool
  - reads: 0 from the OS file cache, including disk I/O
  - dirtied: 0
  - writes: 0

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

After

By `project_id`

note: only third select is different. please the others above.

SELECT "dast_site_profiles".* FROM "dast_site_profiles" WHERE "dast_site_profiles"."project_id" = 19936172;
SELECT "dast_sites".* FROM "dast_sites" WHERE "dast_sites"."id" IN (105, 106, 107, 108, 7, 13, 154, 1);
SELECT "dast_site_validations".* FROM "dast_site_validations" WHERE "dast_site_validations"."id" = 2;

 Index Scan using dast_site_validations_pkey on public.dast_site_validations  (cost=0.15..3.17 rows=1 width=162) (actual time=0.010..0.010 rows=0 loops=1)
   Index Cond: (dast_site_validations.id = 2)
   Buffers: shared hit=1

Time: 0.274 ms
  - planning: 0.235 ms
  - execution: 0.039 ms
    - I/O read: 0.000 ms
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 1 (~8.00 KiB) from the buffer pool
  - reads: 0 from the OS file cache, including disk I/O
  - dirtied: 0
  - writes: 0

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

By `id`

note: only first and third selects are different. please other above.

SELECT "dast_site_profiles".* FROM "dast_site_profiles" WHERE "dast_site_profiles"."id" = 2 LIMIT 1;
SELECT "dast_sites".* FROM "dast_sites" WHERE "dast_sites"."id" = 1;
SELECT "dast_site_validations".* FROM "dast_site_validations" WHERE "dast_site_validations"."id" = 2;

 Limit  (cost=0.14..3.16 rows=1 width=51) (actual time=0.023..0.023 rows=1 loops=1)
   Buffers: shared hit=2
   ->  Index Scan using dast_site_profiles_pkey on public.dast_site_profiles  (cost=0.14..3.16 rows=1 width=51) (actual time=0.021..0.022 rows=1 loops=1)
         Index Cond: (dast_site_profiles.id = 2)
         Buffers: shared hit=2

Time: 0.161 ms
  - planning: 0.105 ms
  - execution: 0.056 ms
    - I/O read: 0.000 ms
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 2 (~16.00 KiB) from the buffer pool
  - reads: 0 from the OS file cache, including disk I/O
  - dirtied: 0
  - writes: 0

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

 Index Scan using dast_site_validations_pkey on public.dast_site_validations  (cost=0.15..3.17 rows=1 width=162) (actual time=0.010..0.010 rows=0 loops=1)
   Index Cond: (dast_site_validations.id = 2)
   Buffers: shared hit=1

Time: 0.274 ms
  - planning: 0.235 ms
  - execution: 0.039 ms
    - I/O read: 0.000 ms
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 1 (~8.00 KiB) from the buffer pool
  - reads: 0 from the OS file cache, including disk I/O
  - dirtied: 0
  - writes: 0

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

Notes

i have not used Project=278964 because it has no dast_site_profiles and thus doesn't make sense to use for analysis
i have opted to use Project=19936172 instead because it does have data
previous finder analysis can be found here

Projections

currently we see few records in these tables (DastSiteProfile.count = 143 and DastSiteValidation.count = 1 on gitlab.com), when we have 15M projects for gitlab.com, it means a fraction of those projects will have multiple dast_site_profiles

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process.
Tested in all supported browsers
Informed Infrastructure department of a default or new setting change, if applicable per definition of done

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 Oct 05, 2020 by Philip Cunningham

Use calculated status for DastSiteProfile query

What does this MR do?

Issue(s)

Database

Before

By project_id

By id

After

By project_id

By id

Notes

Projections

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

Merge request reports

By `project_id`

By `id`

By `project_id`

By `id`