Skip to content

Draft: Use direct SQL to generate internal ids

Patrick Bair requested to merge pb-generate-iid-in-database into master

What does this MR do?

Related issue: #325861 (closed)

Finish up implementation from !57412 (closed) that removes database round-trips and explicit locking when updating internal_ids. We make the assumption on generating a new value that most operations will modify existing records, so we prefer to attempt to update the row first.

Example of creation of object with internal_id, where no existing record exists (create_record!):

  Update InternalId (0.7ms)  UPDATE "internal_ids" SET "last_value" = ("internal_ids"."last_value" + 1) WHERE "internal_ids"."project_id" = 7 AND "internal_ids"."usage" = 0 RETURNING "last_value" /*application:console,line:/app/models/internal_id.rb:91:in `update_record!'*/
  TRANSACTION (0.1ms)  SAVEPOINT active_record_1 /*application:console,line:/app/models/concerns/atomic_internal_id.rb:211:in `block in project_init'*/
   (2.1ms)  SELECT MAX("issues"."iid") FROM "issues" WHERE "issues"."project_id" = 7 /*application:console,line:/app/models/concerns/atomic_internal_id.rb:211:in `block in project_init'*/
  InternalId Create (0.7ms)  INSERT INTO "internal_ids" ("project_id", "usage", "last_value") VALUES (7, 0, 1) RETURNING "id" /*application:console,line:/app/models/internal_id.rb:102:in `block in create_record!'*/

First attempts to update the existing record, and because none is found, it creates a new record. If the new record were to conflict with a concurrent insert, it retries the update.

Example of update of object with internal_id, where a record exists (update_record!):

 Update InternalId (0.3ms)  UPDATE "internal_ids" SET "last_value" = ("internal_ids"."last_value" + 1) WHERE "internal_ids"."project_id" = 7 AND "internal_ids"."usage" = 0 RETURNING "last_value" /*application:console,line:/app/models/internal_id.rb:91:in `update_record!'*/

It updates the record only.

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 Patrick Bair

Merge request reports