Moving nullify from before_destroy to destroy
What does this MR do and why?
From the previous MR. Update All on all pipelines was being called before batching.
Unfortunately, it doesn't look like the order of executions was correct, as the error is still happening: https://sentry.gitlab.net/gitlab/gitlabcom/issues/4103394/events/latest/
This is to hopefully change that, so the query does not timeout anymore
Screenshots or screen recordings
New Database Queries
[1] pry(main)> Ci::PipelineSchedule.last.destroy
Ci::PipelineSchedule Load (2.1ms) SELECT "ci_pipeline_schedules".* FROM "ci_pipeline_schedules" ORDER BY "ci_pipeline_schedules"."id" DESC LIMIT 1 /*application:console,db_config_name:ci,console_hostname:MaxPro.local,console_username:maxfan,line:(pry):1:in `__pry__'*/
Ci::Pipeline Update All (2.2ms) UPDATE "ci_pipelines" SET "pipeline_schedule_id" = NULL, "lock_version" = COALESCE("lock_version", 0) + 1 WHERE "ci_pipelines"."id" IN (SELECT "ci_pipelines"."id" FROM "ci_pipelines" WHERE "ci_pipelines"."pipeline_schedule_id" = 29 LIMIT 100) /*application:console,db_config_name:ci,console_hostname:MaxPro.local,console_username:maxfan,line:/app/models/concerns/batch_nullify_dependent_associations.rb:21:in `block (2 levels) in nullify_dependent_associations_in_batches'*/
TRANSACTION (0.1ms) BEGIN /*application:console,db_config_name:ci,console_hostname:MaxPro.local,console_username:maxfan,line:/app/models/ci/pipeline_schedule.rb:89:in `destroy'*/
Ci::Pipeline Update All (0.2ms) UPDATE "ci_pipelines" SET "pipeline_schedule_id" = NULL, "lock_version" = COALESCE("lock_version", 0) + 1 WHERE "ci_pipelines"."pipeline_schedule_id" = 29 /*application:console,db_config_name:ci,console_hostname:MaxPro.local,console_username:maxfan,line:/app/models/ci/pipeline_schedule.rb:89:in `destroy'*/
Ci::PipelineSchedule Destroy (0.7ms) DELETE FROM "ci_pipeline_schedules" WHERE "ci_pipeline_schedules"."id" = 29 /*application:console,db_config_name:ci,console_hostname:MaxPro.local,console_username:maxfan,line:/app/models/ci/pipeline_schedule.rb:89:in `destroy'*/
TRANSACTION (0.1ms) COMMIT /*application:console,db_config_name:ci,console_hostname:MaxPro.local,console_username:maxfan,line:/lib/gitlab/database.rb:380:in `commit'*/
Previous Database Queries
[28] pry(main)> Ci::PipelineSchedule.last.destroy
Ci::PipelineSchedule Load (1.7ms) SELECT "ci_pipeline_schedules".* FROM "ci_pipeline_schedules" ORDER BY "ci_pipeline_schedules"."id" DESC LIMIT 1 /*application:console,db_config_name:ci,console_hostname:MaxPro.local,console_username:maxfan,line:(pry):28:in `__pry__'*/
TRANSACTION (0.2ms) BEGIN /*application:console,db_config_name:ci,console_hostname:MaxPro.local,console_username:maxfan,line:/lib/gitlab/database/schema_cache_with_renamed_table.rb:25:in `columns'*/
Ci::Pipeline Update All (1.5ms) UPDATE "ci_pipelines" SET "pipeline_schedule_id" = NULL, "lock_version" = COALESCE("lock_version", 0) + 1 WHERE "ci_pipelines"."pipeline_schedule_id" = 23 /*application:console,db_config_name:ci,console_hostname:MaxPro.local,console_username:maxfan,line:(pry):28:in `__pry__'*/
Ci::Pipeline Update All (0.7ms) UPDATE "ci_pipelines" SET "pipeline_schedule_id" = NULL, "lock_version" = COALESCE("lock_version", 0) + 1 WHERE "ci_pipelines"."id" IN (SELECT "ci_pipelines"."id" FROM "ci_pipelines" WHERE "ci_pipelines"."pipeline_schedule_id" = 23 LIMIT 100) /*application:console,db_config_name:ci,console_hostname:MaxPro.local,console_username:maxfan,line:/app/models/concerns/batch_nullify_dependent_associations.rb:21:in `block (2 levels) in nullify_dependent_associations_in_batches'*/
Ci::PipelineSchedule Destroy (0.5ms) DELETE FROM "ci_pipeline_schedules" WHERE "ci_pipeline_schedules"."id" = 23 /*application:console,db_config_name:ci,console_hostname:MaxPro.local,console_username:maxfan,line:(pry):28:in `__pry__'*/
TRANSACTION (0.2ms) COMMIT /*application:console,db_config_name:ci,console_hostname:MaxPro.local,console_username:maxfan,line:/lib/gitlab/database.rb:380:in `commit'*/
The Key Differences are
-
Ci::Pipeline Update All
with batching is called beforeCi::Pipeline Update All
total -
Update All
call is no longer in the same transaction block
How to set up and validate locally
- Create a pipelineSchedule
http://172.16.123.1:3000/root/maxci/-/pipeline_schedules
- In the console
destroy
the schedule and see the queries
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Related to #328596 (closed)
Edited by Max Fan