Skip to content

Create iid sequence for ci_pipelines with new projects

Marius Bobin requested to merge 423421-fix-race-cond into master

What does this MR do and why?

Create iid sequence for ci_pipelines with new projects

The initialization of the sequence is not atomic and due to race conditions some pipelines could reinitialize multiple times, resulting in attempts to insert rows with duplicate values. This fix moves the initialization from the first pipeline creation into the project creation, so the sequence will be already initialized when the first pipelines are created.

Changelog: fixed

Screenshots or screen recordings

Before

When the first pipeline is created, we have an update followed by an insert to generate the sequence. On edge cases, the first pipeline will not be one pipeline, but many created concurrently from Sidekiq. This can initialize the sequence more than once.

[11] pry(main)> FactoryBot.create(:ci_pipeline, project: project)
  Update InternalId (0.7ms)  UPDATE "internal_ids" SET "last_value" = ("internal_ids"."last_value" + 1) WHERE "internal_ids"."project_id" = 231 AND "internal_ids"."usage" = 5 RETURNING "last_value" /*application:console,db_config_name:main,console_hostname:rocket-sled.local,console_username:marius,line:/app/models/internal_id.rb:164:in `update_record!'*/
  Ci::Pipeline Maximum (2.5ms)  SELECT MAX("ci_pipelines"."iid") FROM "ci_pipelines" WHERE "ci_pipelines"."project_id" = 231 /*application:console,db_config_name:ci,console_hostname:rocket-sled.local,console_username:marius,line:/app/models/ci/pipeline.rb:66:in `block in <class:Pipeline>'*/
  Ci::Pipeline Count (0.2ms)  SELECT COUNT(*) FROM "ci_pipelines" WHERE "ci_pipelines"."project_id" = 231 /*application:console,db_config_name:ci,console_hostname:rocket-sled.local,console_username:marius,line:/app/models/ci/pipeline.rb:66:in `block in <class:Pipeline>'*/
  InternalId Insert (2.0ms)  INSERT INTO "internal_ids" ("project_id","namespace_id","usage","last_value") VALUES (231, NULL, 5, 1) ON CONFLICT  DO NOTHING RETURNING "id" /*application:console,db_config_name:main,console_hostname:rocket-sled.local,console_username:marius,line:/app/models/internal_id.rb:178:in `create_record!'*/
  Ci::Ref Load (0.5ms)  SELECT "ci_refs".* FROM "ci_refs" WHERE "ci_refs"."project_id" = 231 AND "ci_refs"."ref_path" = 'refs/heads/master' LIMIT 1 /*application:console,db_config_name:ci,console_hostname:rocket-sled.local,console_username:marius,line:/app/models/application_record.rb:79:in `safe_find_or_create_by'*/
  TRANSACTION (0.0ms)  BEGIN /*application:console,db_config_name:ci,console_hostname:rocket-sled.local,console_username:marius,line:/lib/gitlab/database/schema_cache_with_renamed_table.rb:21:in `primary_keys'*/
  Ci::Ref Create (0.4ms)  INSERT INTO "ci_refs" ("project_id", "ref_path", "lock_version") VALUES (231, 'refs/heads/master', 0) RETURNING "id" /*application:console,db_config_name:ci,console_hostname:rocket-sled.local,console_username:marius,line:/app/models/application_record.rb:87:in `block in safe_find_or_create_by'*/
  TRANSACTION (0.1ms)  COMMIT /*application:console,db_config_name:ci,console_hostname:rocket-sled.local,console_username:marius,line:/lib/gitlab/database.rb:423:in `commit'*/
  TRANSACTION (0.1ms)  BEGIN /*application:console,db_config_name:ci,console_hostname:rocket-sled.local,console_username:marius,line:<internal:kernel>:90:in `tap'*/
  Ci::Pipeline Create (1.7ms)  INSERT INTO "ci_pipelines" ("ref", "sha", "created_at", "updated_at", "project_id", "status", "source", "protected", "iid", "ci_ref_id", "partition_id", "lock_version") VALUES ('master', 'b83d6e391c22777fca1ed3012fce84f633d7fed0', '2023-08-30 13:58:30.060043', '2023-08-30 13:58:30.060043', 231, 'pending', 1, FALSE, 1, 244, 100, 0) RETURNING "id" /*application:console,db_config_name:ci,console_hostname:rocket-sled.local,console_username:marius,line:<internal:kernel>:90:in `tap'*/
  TRANSACTION (0.1ms)  COMMIT

After

The internal_ids record is created right after the project gets created, so the first pipelines will get the iid value from the increment call, without needing to initialize the sequence:

[18] pry(main)> FactoryBot.create(:ci_pipeline, project: project)
  Update InternalId (0.7ms)  UPDATE "internal_ids" SET "last_value" = ("internal_ids"."last_value" + 1) WHERE "internal_ids"."project_id" = 230 AND "internal_ids"."usage" = 5 RETURNING "last_value" /*application:console,db_config_name:main,console_hostname:rocket-sled.local,console_username:marius,line:/app/models/internal_id.rb:166:in `update_record!'*/
  Ci::Ref Load (0.4ms)  SELECT "ci_refs".* FROM "ci_refs" WHERE "ci_refs"."project_id" = 230 AND "ci_refs"."ref_path" = 'refs/heads/master' LIMIT 1 /*application:console,db_config_name:ci,console_hostname:rocket-sled.local,console_username:marius,line:/app/models/application_record.rb:79:in `safe_find_or_create_by'*/
  TRANSACTION (0.2ms)  BEGIN /*application:console,db_config_name:ci,console_hostname:rocket-sled.local,console_username:marius,line:/app/models/application_record.rb:87:in `block in safe_find_or_create_by'*/
  Ci::Ref Create (0.5ms)  INSERT INTO "ci_refs" ("project_id", "ref_path", "lock_version") VALUES (230, 'refs/heads/master', 0) RETURNING "id" /*application:console,db_config_name:ci,console_hostname:rocket-sled.local,console_username:marius,line:/app/models/application_record.rb:87:in `block in safe_find_or_create_by'*/
  TRANSACTION (0.2ms)  COMMIT /*application:console,db_config_name:ci,console_hostname:rocket-sled.local,console_username:marius,line:/lib/gitlab/database.rb:423:in `commit'*/
  TRANSACTION (0.1ms)  BEGIN /*application:console,db_config_name:ci,console_hostname:rocket-sled.local,console_username:marius,line:<internal:kernel>:90:in `tap'*/
  Ci::Pipeline Create (1.3ms)  INSERT INTO "ci_pipelines" ("ref", "sha", "created_at", "updated_at", "project_id", "status", "source", "protected", "iid", "ci_ref_id", "partition_id", "lock_version") VALUES ('master', 'b83d6e391c22777fca1ed3012fce84f633d7fed0', '2023-08-30 13:52:14.803045', '2023-08-30 13:52:14.803045', 230, 'pending', 1, FALSE, 1, 243, 100, 0) RETURNING "id" /*application:console,db_config_name:ci,console_hostname:rocket-sled.local,console_username:marius,line:<internal:kernel>:90:in `tap'*/
  TRANSACTION (0.2ms)  COMMIT

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #423421 (closed)

Edited by Marius Bobin

Merge request reports