Skip to content

Reduce subtransactions in safe_find_or_create_by

What does this MR do?

Reduces subtransactions in safe_find_or_create_by by only opening the transaction when attempting to create a record.

Old code:

  1. Open transaction
  2. Find record
  3. If not found, create record
  4. If create not unique, repeat steps 2 and 3

New code:

  1. Find record
  2. If not found, open transaction and create record
  3. If create not unique, repeat step 1

Note that there is a slight behavior change where we do not attempt to create the record twice. I think it's not expected to do that anyway and it's more of a convenience of using retry on the block.

Sample when record does not exist:

[2] pry(main)> UserInteractedProject.transaction { UserInteractedProject.safe_find_or_create_by(user_id: 1, project_id: 1) }
  TRANSACTION (0.3ms)  BEGIN /*application:console,line:/data/cache/bundle-2.7.2/ruby/2.7.0/gems/activerecord-6.1.3.2/lib/active_record/connection_adapters/postgresql/database_statements.rb:109:in `begin_db_transaction'*/
  UserInteractedProject Load (0.7ms)  SELECT "user_interacted_projects".* FROM "user_interacted_projects" WHERE "user_interacted_projects"."user_id" = $1 AND "user_interacted_projects"."project_id" = $2 LIMIT $3 /*application:console,line:/data/cache/bundle-2.7.2/ruby/2.7.0/gems/activerecord-6.1.3.2/lib/active_record/connection_adapters/postgresql/database_statements.rb:53:in `exec_query'*/  [["user_id", 1], ["project_id", 1], ["LIMIT", 1]]
  Feature::FlipperGate Load (0.5ms)  SELECT "feature_gates".* FROM "feature_gates" WHERE "feature_gates"."feature_key" = $1 /*application:console,line:/data/cache/bundle-2.7.2/ruby/2.7.0/gems/activerecord-6.1.3.2/lib/active_record/connection_adapters/postgresql/database_statements.rb:53:in `exec_query'*/  [["feature_key", "active_record_subtransactions_counter"]]
  TRANSACTION (0.2ms)  SAVEPOINT active_record_1 /*application:console,line:/data/cache/bundle-2.7.2/ruby/2.7.0/gems/activerecord-6.1.3.2/lib/active_record/connection_adapters/abstract/savepoints.rb:11:in `create_savepoint'*/
  UserInteractedProject Create (9.6ms)  INSERT INTO "user_interacted_projects" ("user_id", "project_id") VALUES ($1, $2) /*application:console,line:/data/cache/bundle-2.7.2/ruby/2.7.0/gems/activerecord-6.1.3.2/lib/active_record/connection_adapters/postgresql/database_statements.rb:53:in `exec_query'*/  [["user_id", 1], ["project_id", 1]]
  TRANSACTION (0.2ms)  RELEASE SAVEPOINT active_record_1 /*application:console,line:/data/cache/bundle-2.7.2/ruby/2.7.0/gems/activerecord-6.1.3.2/lib/active_record/connection_adapters/abstract/savepoints.rb:19:in `release_savepoint'*/
  TRANSACTION (1.2ms)  COMMIT /*application:console,line:/data/cache/bundle-2.7.2/ruby/2.7.0/gems/activerecord-6.1.3.2/lib/active_record/connection_adapters/postgresql/database_statements.rb:119:in `commit_db_transaction'*/
=> #<UserInteractedProject:0x00007eff895a2c30 user_id: 1, project_id: 1>

Sample when record exists:

[3] pry(main)> UserInteractedProject.transaction { UserInteractedProject.safe_find_or_create_by(user_id: 1, project_id: 1) }
  TRANSACTION (0.3ms)  BEGIN /*application:console,line:/data/cache/bundle-2.7.2/ruby/2.7.0/gems/activerecord-6.1.3.2/lib/active_record/connection_adapters/postgresql/database_statements.rb:109:in `begin_db_transaction'*/
  UserInteractedProject Load (0.5ms)  SELECT "user_interacted_projects".* FROM "user_interacted_projects" WHERE "user_interacted_projects"."user_id" = $1 AND "user_interacted_projects"."project_id" = $2 LIMIT $3 /*application:console,line:/data/cache/bundle-2.7.2/ruby/2.7.0/gems/activerecord-6.1.3.2/lib/active_record/connection_adapters/postgresql/database_statements.rb:53:in `exec_query'*/  [["user_id", 1], ["project_id", 1], ["LIMIT", 1]]
  TRANSACTION (0.3ms)  COMMIT /*application:console,line:/data/cache/bundle-2.7.2/ruby/2.7.0/gems/activerecord-6.1.3.2/lib/active_record/connection_adapters/postgresql/database_statements.rb:119:in `commit_db_transaction'*/
=> #<UserInteractedProject:0x00007eff8867bd38 user_id: 1, project_id: 1>

We can see that it's a single SELECT without any subtransaction.

Screenshots or Screencasts (strongly suggested)

How to setup and validate locally (strongly suggested)

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 Heinrich Lee Yu

Merge request reports