Skip to content

Preload associations in Ci::Pipeline#cancel_running

Furkan Ayhan requested to merge 21098-reducing-cancel-sql-queries into master

What does this MR do?

This will help to reduce N+1 queries for

  • All commit statuses need project and pipeline preloads.
  • Only CI Builds need deployment and taggings.

Remaining queries:

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 calls Ci::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

Availability and Testing

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)

Merge request reports