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:
- twice per MR security report comparison job
enabledSecurityScansGraphQL field that the MR widget pollsProcessPipelineCompletionWorker
References
Related #628409
Notes
- Production Sidekiq logs,
ReactiveCaching::LowUrgencyWorkerjobs forSecurity::MergeRequestSecurityReportGenerationServicewithjson.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_countbetween 62 and 1366 - 1
enabledSecurityScansGraphQL request on a gitlab-org/gitlab pipeline: 324 CI database queries, 0.70 s CPU, 59 MB allocatedjson.correlation_id: a3adfa92d8fd68b2-BRU - GDK, pipeline with 16 finished builds: current code 22 queries (16
Ci::JobDefinitionloads), 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 1New (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
configJSONB from TOAST storage, becauseEXPLAINdoes 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.