Check only unfinished builds in all_security_jobs_complete?

What does this MR do and why?

The method all_security_jobs_complete? loads every build of the pipeline and its child pipelines and calls security_job? on each. security_jobs? reads the job definition with one query per build. On large pipelines this meant hundreds of queries and hundreds of megabytes per call, twice per MR security report job.

Behind the new feature flag optimized_all_security_jobs_complete_check, we now try to optimize this by switching from "all security jobs are finished" to the logically equivalent "no unfinished build is a security job".

This code runs often:

  1. twice per MR security report comparison job
  2. enabledSecurityScans GraphQL field that the MR widget polls
  3. ProcessPipelineCompletionWorker

References

Related #628409

Notes

  • Production Sidekiq logs, ReactiveCaching::LowUrgencyWorker jobs for Security::MergeRequestSecurityReportGenerationService with json.cpu_s >= 30, last 2 days: about 16,000 jobs, almost all gitlab-org/gitlab. 30 - 120 s CPU, 1-5 GB memory, json.db_ci_replica_count between 62 and 1366
  • 1 enabledSecurityScans GraphQL request on a gitlab-org/gitlab pipeline: 324 CI database queries, 0.70 s CPU, 59 MB allocated json.correlation_id: a3adfa92d8fd68b2-BRU
  • GDK, pipeline with 16 finished builds: current code 22 queries (16 Ci::JobDefinition loads), new code 6 queries.
  • Postgres.ai on the CI database, gitlab-org/gitlab master pipeline 2812872255: the current query returns 90 rows for the parent pipeline (152 pages read); the new query returns 0 rows, 61 buffers.

Database

The method loads the pipeline hierarchy first, so pipeline ids and partition_id are literals. With child pipelines the condition is commit_id IN (...), without children commit_id = id. The queries below are for gitlab-org/gitlab master pipeline 2664258420 (623 jobs, 39 child pipelines, stuck with 532 unfinished builds, partition 113). The ... in the IN list stands for the remaining child pipeline ids.

Current: builds query, then one job definition query per build
SELECT "p_ci_builds".*
FROM "p_ci_builds"
WHERE "p_ci_builds"."type" = 'Ci::Build'
  AND ("p_ci_builds"."retried" = FALSE OR "p_ci_builds"."retried" IS NULL)
  AND "p_ci_builds"."commit_id" IN (2664258420, 2664369127, ...)
  AND "p_ci_builds"."partition_id" = 113
  AND "p_ci_builds"."status" != 'manual'

Then, for every returned build (security_job? reads the job definition):

SELECT "p_ci_job_definitions".*
FROM "p_ci_job_definitions"
INNER JOIN "p_ci_job_definition_instances" ON "p_ci_job_definitions"."id" = "p_ci_job_definition_instances"."job_definition_id"
WHERE "p_ci_job_definition_instances"."job_id" = <build id>
  AND "p_ci_job_definition_instances"."partition_id" = 113
  AND "p_ci_job_definitions"."partition_id" = 113
LIMIT 1
New (flag on): builds query limited to unfinished builds, then one preload query
SELECT "p_ci_builds".*
FROM "p_ci_builds"
WHERE "p_ci_builds"."type" = 'Ci::Build'
  AND ("p_ci_builds"."retried" = FALSE OR "p_ci_builds"."retried" IS NULL)
  AND "p_ci_builds"."commit_id" IN (2664258420, 2664369127, ...)
  AND "p_ci_builds"."partition_id" = 113
  AND "p_ci_builds"."status" != 'manual'
  AND "p_ci_builds"."status" IN ('created', 'waiting_for_resource', 'preparing', 'waiting_for_callback', 'pending', 'running', 'canceling', 'manual', 'scheduled')

Then one query for all returned builds (preload(:job_definition)):

SELECT "p_ci_job_definition_instances".*, "p_ci_job_definitions".*
FROM "p_ci_job_definition_instances"
LEFT OUTER JOIN "p_ci_job_definitions"
  ON "p_ci_job_definitions"."partition_id" IS NOT NULL
 AND "p_ci_job_definitions"."id" = "p_ci_job_definition_instances"."job_definition_id"
 AND "p_ci_job_definitions"."partition_id" = "p_ci_job_definition_instances"."partition_id"
WHERE "p_ci_job_definition_instances"."partition_id" = 113
  AND "p_ci_job_definitions"."partition_id" = 113
  AND "p_ci_job_definition_instances"."job_id" IN (<ids of the returned builds>)

manual appears in the IN list and is excluded again by != 'manual'. This comes from chaining the existing without_status(:manual) and incomplete scopes and is a no-op for the planner.

Query plans

Database Lab, CI database. Each plan ran after a reset, so shared buffers were cold. The batched "all 1440" query is a stand-in for the 1440 single queries of the current code, because Joe cannot loop.

Pipeline Query Rows Buffers (hits + reads) Execution Plan
2664258420, running (stuck), 40 pipelines Current: builds 1440 1153 (51 + 1102) 30 ms plan
Current: one job definition (x 1440) 1 23 (6 + 17) 14 ms plan
Current: job definitions of all 1440 builds, batched 1440 15560 (13991 + 1569) 521 ms plan
New: builds 532 283 (33 + 250) 28 ms plan
New: preload 532 5603 (5262 + 341) 294 ms plan
2812872255, finished, no children Current: builds 90 152 pages 255 ms plan
New: builds 0 66 7 ms plan

What this shows:

  • The new builds query uses the (commit_id, status, type) index, so the status filter is part of the index condition and finished builds are never visited. The current query uses (commit_id, type, ref) and filters status afterwards.
  • Running pipeline: current code runs 1441 queries and loads 1440 builds plus 1440 job definitions. New code runs 2 queries and loads 532 builds plus 532 job definitions.
  • Finished pipeline: the new builds query returns nothing, the preload does not run, and no job definitions are loaded. This is the common case for the Sidekiq job.
  • The batched plans do not include reading the config JSONB from TOAST storage, because EXPLAIN does not send rows to a client. The real page count of the job definition queries is higher for both current and new.

How to set up and validate locally

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Lorenz van Herwaarden

Merge request reports

Loading
Loading