Use locked value from Ci::Pipeline for Ci::JobArtifact
What does this MR do and why?
- Makes
Ci::JobArtifacts::CreateService
use the value ofCi::Pipeline#locked
for new instances ofCi::JobArtifact
- Makes
Ci::UnlockArtifactsService
update instances ofCi::JobArtifact
with the value ofCi::Pipeline#locked
for pipelines being unlocked
This is being done as part of the work in #327281 (closed) to improve the cleanup of expired and unlocked job artifacts so that we do not continue to accumulate more.
The first step was completed in !68893 (merged), which added a locked
column to Ci::JobArtifact
and a partial index for it.
This is the second step, to cause this column to be updated with the value from the parent pipeline when the job artifacts are created, and when the pipeline is unlocked.
Relates to #327281 (closed)
Feature Flag
ci_update_unlocked_job_artifacts
: #343465 (closed)
Rollout to production will begin on 2021-11-03, targeting 100% by 2021-11-04.
Database
Ci::UnlockArtifactsService
to also update ci_job_artifacts
for any updated ci_pipelines
Added update query in Explain is for worst case scenario where there were 100 (batch size) pipelines for a single Ci::Ref
needing to be unlocked.
https://console.postgres.ai/shared/6d07df0f-3dbb-4f4f-92ca-927b0aa5d32c
Query:
UPDATE
"ci_job_artifacts"
SET
"locked" = 0
WHERE
"ci_job_artifacts"."job_id" IN (
SELECT
"ci_builds"."id"
FROM
"ci_builds"
WHERE
"ci_builds"."type" = 'Ci::Build'
AND "ci_builds"."commit_id" IN (4886, 4895, 3893, 4469, 4400, 1612, 2990, 2252, 4737, 6941, 9793, 2970, 9193, 7472, 4620, 621, 2904, 416, 4194, 7177, 1396, 5153, 4224, 6885, 909, 161, 4446, 213, 4442, 6590, 9544, 6984, 1341, 7761, 4153, 599, 814, 5965, 2539, 4816, 9689, 1862, 2315, 6607, 2518, 7010, 206, 5585, 1839, 5982, 1831, 3340, 5898, 7774, 5927, 5268, 6125, 3886, 6238, 2059, 8335, 7619, 455, 3497, 5124, 2745, 5860, 2488, 5698, 9381, 3768, 2401, 4683, 6249, 4951, 3935, 9803, 2990, 1701, 8364, 9095, 4605, 4605, 3668, 6423, 2014, 3805, 8547, 7799, 3641, 1265, 8106, 9637, 5594, 6027, 1662, 8035, 7175, 9060, 9739))
RETURNING ("ci_job_artifacts"."id");
Statistics:
Time: 446.838 ms
- planning: 25.925 ms
- execution: 420.913 ms
- I/O read: 405.492 ms
- I/O write: 0.000 ms
Shared buffers:
- hits: 4770 (~37.30 MiB) from the buffer pool
- reads: 506 (~4.00 MiB) from the OS file cache, including disk I/O
- dirtied: 277 (~2.20 MiB)
- writes: 1 (~8.00 KiB)
Explain:
ModifyTable on public.ci_job_artifacts (cost=1.28..1005111.10 rows=40987 width=164) (actual time=28.762..420.735 rows=145 loops=1)
Buffers: shared hit=4770 read=506 dirtied=277 written=1
I/O Timings: read=405.492 write=0.000
-> Nested Loop (cost=1.28..1005111.10 rows=40987 width=164) (actual time=13.075..291.709 rows=145 loops=1)
Buffers: shared hit=1049 read=312
I/O Timings: read=287.532 write=0.000
-> Index Scan using index_ci_builds_on_commit_id_and_status_and_type on public.ci_builds (cost=0.70..58726.17 rows=49658 width=14) (actual time=6.882..196.908 rows=147 loops=1)
Index Cond: ((ci_builds.commit_id = ANY ('{4886,4895,3893,4469,4400,1612,2990,2252,4737,6941,9793,2970,9193,7472,4620,621,2904,416,4194,7177,1396,5153,4224,6885,909,161,4446,213,4442,6590,9544,6984,1341,7761,4153,599,814,5965,2539,4816,9689,1862,2315,6607,2518,7010,206,5585,1839,5982,1831,3340,5898,7774,5927,5268,6125,3886,6238,2059,8335,7619,455,3497,5124,2745,5860,2488,5698,9381,3768,2401,4683,6249,4951,3935,9803,2990,1701,8364,9095,4605,4605,3668,6423,2014,3805,8547,7799,3641,1265,8106,9637,5594,6027,1662,8035,7175,9060,9739}'::integer[])) AND ((ci_builds.type)::text = 'Ci::Build'::text))
Buffers: shared hit=441 read=187
I/O Timings: read=195.077 write=0.000
-> Index Scan using index_ci_job_artifacts_on_job_id_and_file_type on public.ci_job_artifacts (cost=0.58..18.90 rows=16 width=144) (actual time=0.640..0.641 rows=1 loops=147)
Index Cond: (ci_job_artifacts.job_id = ci_builds.id)
Buffers: shared hit=608 read=125
I/O Timings: read=92.455 write=0.000
How to set up and validate locally
- Cause job artifacts to be created
- Verify they received a
locked
value other thanunknown
or
- Execute
::Ci::UnlockArtifactsService.new(pipeline.project, pipeline.user).execute(pipeline.ci_ref, pipeline)
for a pipeline with builds with job artifacts - Verify the job artifacts
locked
value changed tounlocked
MR acceptance checklist
These checklists encourage us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
Quality
-
Quality checklist confirmed
- I have self-reviewed this MR per code review guidelines.
- For the code that that this change impacts, I believe that the automated tests (Testing Guide) validate functionality that is highly important to users (including consideration of all test levels). If the existing automated tests do not cover this functionality, I have added the necessary additional tests or I have added an issue to describe the automation testing gap and linked it to this MR.
- I have considered the technical aspects of the impact of this change on both gitlab.com hosted customers and self-hosted customers.
- I have considered the impact of this change on the front-end, back-end, and database portions of the system where appropriate and applied frontend, backend and database labels accordingly.
- I have tested this MR in all supported browsers, or determiend that this testing is not needed.
- I have confirmed that this change is backwards compatible across updates, or I have decided that this does not apply.
- I have properly separated EE content from FOSS, or this MR is FOSS only. (Where should EE code go?)
- If I am introducing a new expectation for existing data, I have confirmed that existing data meets this expectation or I have made this expectation optional rather than required.
Performance, reliability, and availability
-
Performance, reliability, and availability checklist confirmed
- I am confident that this MR does not harm performance, or I have asked a reviewer to help assess the performance impact. (Merge request performance guidelines)
- I have added information for database reviewers in the MR description, or I have decided that it is unnecessary. (Does this MR have database-related changes?)
- I have considered the availability and reliability risks of this change. I have also considered the scalability risk based on future predicted growth
- I have considered the performance, reliability and availability impacts of this change on large customers who may have significantly more data than the average customer.
Documentation
-
Documentation checklist confirmed
- I have included changelog trailers, or I have decided that they are not needed. (Does this MR need a changelog?)
- I have added/updated documentation, or I have decided that documentation changes are not needed for this MR. (Is documentation required?)
Security
-
Security checklist confirmed
- I have confirmed that if this MR contains changes to processing or storing of credentials or tokens, authorization, and authentication methods, or other items described in the security review guidelines, I have added the label security and I have
@
-mentioned@gitlab-com/gl-security/appsec
.
Deployment
-
Deployment checklist confirmed
- I have considered using a feature flag for this change because the change may be high risk. If I decided to use a feature flag, I plan to test the change in staging before I test it in production, and I have considered rolling it out to a subset of production customers before doing rolling it out to all customers. When to use a feature flag
- I have informed the Infrastructure department of a default setting or new setting change per definition of done, or decided that this is not needed.
Related to #327281 (closed)