Preload associations in Ci::Pipeline#cancel_running
What does this MR do?
This will help to reduce N+1 queries for
- Projects::PipelinesController#cancel
- Pipeline::Cancel GraphQL endpointv
- CancelPendingPipelines
- API :id/pipelines/:pipeline_id/cancel endpoint
- destroying merge trains
- All commit statuses need
project
andpipeline
preloads. - Only CI Builds need
deployment
andtaggings
.
Remaining queries:
-
UPDATE
query for each builds. (we may unify the UPDATE) -
Ci::BuildRunnerSession.where(build: build).delete_all
for running builds. -
validate :name_uniqueness_across_types, unless: :importing?
query.
The method Ci::Pipeline#cancel_running
method is too complicated and it should be in a service. I'll look into it in future works =>
Future work drafts:
- Find all methods calling cancel_running
- TODO: Ci::BuildRunnerSession.where(build: build).delete_all
- TODO: validate :name_uniqueness_across_types, unless: :importing?
- TODO: Try to have only one UPDATE SQL query
moving to a service
# rubocop: disable CodeReuse/ServiceClass
def cancel_running(retries: nil, &block)
Ci::CancelPipelineService.new.execute(self, retries, block)
end
# rubocop: enable CodeReuse/ServiceClass
# frozen_string_literal: true
module Ci
class CancelPipelineService
COMMIT_STATUS_RELATIONS = %i(project, pipeline)
CI_BUILD_RELATIONS = %i(deployment, taggings)
# rubocop: disable CodeReuse/ActiveRecord
def execute(pipeline, max_retries, &block)
Gitlab::OptimisticLocking.retry_lock(pipeline.cancelable_statuses, max_retries, name: 'ci_pipeline_cancel_running') do |cancelables|
cancelables.find_in_batches do |batch|
ActiveRecord::Associations::Preloader.new.preload(batch, COMMIT_STATUS_RELATIONS)
ActiveRecord::Associations::Preloader.new.preload(batch.select { |job| job.is_a?(Ci::Build) }, CI_BUILD_RELATIONS)
batch.each do |job|
yield(job) if block
job.cancel
end
end
end
end
# rubocop: enable CodeReuse/ActiveRecord
end
end
Related to #21098 (closed)
Screenshots (strongly suggested)
-
Projects::PipelinesController#cancel
callsCi::Pipeline#cancel_running
-
Ci::Pipeline#cancel_running
fetches jobs with the statuses of[:running, :waiting_for_resource, :preparing, :pending, :created, :scheduled]
;
SELECT "ci_builds".*
FROM "ci_builds"
WHERE "ci_builds"."commit_id" = 641
AND "ci_builds"."status" IN ('running', 'waiting_for_resource', 'preparing', 'pending', 'created', 'scheduled')
ORDER BY "ci_builds"."id" ASC
- and runs the
CommitStatus#cancel
method to each of them.
Ci::Pipeline Load (0.4ms) SELECT "ci_pipelines".* FROM "ci_pipelines" WHERE "ci_pipelines"."id" = 641 LIMIT 1
Ci::Build Update (0.5ms) UPDATE "ci_builds" SET "status" = 'canceled', "finished_at" = '2021-04-02 09:51:30.158203', "processed" = FALSE, "updated_at" = '2021-04-02 09:51:30.160335', "lock_version" = 1 WHERE "ci_builds"."id" = 5000 AND "ci_builds"."lock_version" = 0
ActsAsTaggableOn::Tagging Load (0.3ms) SELECT "taggings".* FROM "taggings" WHERE "taggings"."taggable_id" = 5000 AND "taggings"."taggable_type" = 'CommitStatus'
Project Load (0.5ms) SELECT "projects".* FROM "projects" WHERE "projects"."id" = 86 LIMIT 1
Deployment Load (0.3ms) SELECT "deployments".* FROM "deployments" WHERE "deployments"."deployable_id" = 5000 AND "deployments"."deployable_type" = 'CommitStatus' LIMIT 1
Now, it only runs an UPDATE
query for each of them.
Ci::Pipeline Load (0.4ms) SELECT "ci_pipelines".* FROM "ci_pipelines" WHERE "ci_pipelines"."id" = 644 LIMIT 1
(0.1ms) BEGIN
CommitStatus Load (0.7ms) SELECT "ci_builds".* FROM "ci_builds" WHERE "ci_builds"."commit_id" = 644 AND "ci_builds"."status" IN ('running', 'waiting_for_resource', 'preparing', 'pending', 'created', 'scheduled') ORDER BY "ci_builds"."id" ASC LIMIT 1000
Deployment Load (0.5ms) SELECT "deployments".* FROM "deployments" WHERE "deployments"."deployable_type" = 'CommitStatus' AND "deployments"."deployable_id" IN (5110, 5111, 5112, 5113, 5114, 5115, 5116, 5117, 5118, 5119, 5120, 5121, 5122, 5123, 5124, 5125, 5126, 5127, 5128, 5129, 5130, 5131, 5132, 5133, 5134, 5135, 5136, 5137, 5138, 5139, 5140, 5141, 5142, 5143, 5144, 5145, 5146)
ActsAsTaggableOn::Tagging Load (0.4ms) SELECT "taggings".* FROM "taggings" WHERE "taggings"."taggable_type" = 'CommitStatus' AND "taggings"."taggable_id" IN (5110, 5111, 5112, 5113, 5114, 5115, 5116, 5117, 5118, 5119, 5120, 5121, 5122, 5123, 5124, 5125, 5126, 5127, 5128, 5129, 5130, 5131, 5132, 5133, 5134, 5135, 5136, 5137, 5138, 5139, 5140, 5141, 5142, 5143, 5144, 5145, 5146)
Project Load (0.9ms) SELECT "projects".* FROM "projects" WHERE "projects"."id" = 86
Ci::Pipeline Load (0.4ms) SELECT "ci_pipelines".* FROM "ci_pipelines" WHERE "ci_pipelines"."id" = 644
Ci::Build Update (0.6ms) UPDATE "ci_builds" SET "status" = 'canceled', "finished_at" = '2021-04-02 10:26:28.803761', "processed" = FALSE, "updated_at" = '2021-04-02 10:26:28.805487', "lock_version" = 1 WHERE "ci_builds"."id" = 5110 AND "ci_builds"."lock_version" = 0
Ci::Build Update (0.5ms) UPDATE "ci_builds" SET "status" = 'canceled', "finished_at" = '2021-04-02 10:26:28.808801', "processed" = FALSE, "updated_at" = '2021-04-02 10:26:28.809361', "lock_version" = 1 WHERE "ci_builds"."id" = 5111 AND "ci_builds"."lock_version" = 0
...
Does this MR meet the acceptance criteria?
Conformity
-
📋 Does this MR need a changelog?-
I have included a changelog entry. -
I have not included a changelog entry because _____.
-
-
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
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 by Dmytro Zaporozhets (DZ)