Skip to content

Add locking to pipeline add job service

Furkan Ayhan requested to merge 326791-locking-pipeline-add-job into master

What does this MR do?

We saw some repeated requests in small frequencies, so we decided to add a lock mechanism to Ci::Pipelines::AddJobService.

This MR adds an exclusive lock to the service based on the "ci:pipelines:#{pipeline.id}:add-job" key because Ci::Pipeline can be our reference when the requests try to add different jobs in small frequencies.

It's behind a feature flag ci_pipeline_add_job_with_lock (#337628 (closed)).

Related to #326791 (closed)

Screenshots or Screencasts (strongly suggested)

Example;

Ci::Pipelines::AddJobService.new(pipeline).execute!(job) do |j|
  puts Time.now
  sleep 3
  puts Time.now
end

First request;

Project Load (3.7ms)  SELECT "projects".* FROM "projects" WHERE "projects"."id" = 20 LIMIT 1 /*application:console,line:/app/services/ci/pipelines/add_job_service.rb:46:in `assign_pipeline_attributes'*/
2021-08-04 14:37:30 +0300
2021-08-04 14:37:33 +0300
  CommitStatus Update All (0.9ms)  UPDATE "ci_builds" SET "retried" = TRUE, "processed" = TRUE WHERE "ci_builds"."commit_id" = 96 AND ("ci_builds"."retried" = FALSE OR "ci_builds"."retried" IS NULL) AND "ci_builds"."name" = 'deploy' AND "ci_builds"."id" != 816 /*application:console,line:/app/models/commit_status.rb:293:in `update_older_statuses_retried!'*/

Second request;

Project Load (3.4ms)  SELECT "projects".* FROM "projects" WHERE "projects"."id" = 20 LIMIT 1 /*application:console,line:/app/services/ci/pipelines/add_job_service.rb:46:in `assign_pipeline_attributes'*/
2021-08-04 14:37:33 +0300
2021-08-04 14:37:36 +0300
  CommitStatus Update All (0.8ms)  UPDATE "ci_builds" SET "retried" = TRUE, "processed" = TRUE WHERE "ci_builds"."commit_id" = 96 AND ("ci_builds"."retried" = FALSE OR "ci_builds"."retried" IS NULL) AND "ci_builds"."name" = 'deploy' AND "ci_builds"."id" != 816 /*application:console,line:/app/models/commit_status.rb:293:in `update_older_statuses_retried!'*/

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

Does this MR contain changes to processing or storing of credentials or tokens, authorization and authentication methods or other items described in the security review guidelines? If not, then delete this Security section.

  • 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 Furkan Ayhan

Merge request reports