Pessimistic locking in Activerecord - with_lock is not working as expected-CodePudding

I'm trying to implement create_or_update method for User model, which will either create a new record or update the existing one.

def create_or_update_user(external_user_id, data)
  user = User.find_or_initialize_by(external_user_id: external_user_id)
  user.title = data[:title].downcase
  # and so on - here we assign other attributes from data
  user.save! if user.changed?
end

The pitfall here is that this table is being updated concurrently by different processes and when they are trying to modify user with the same external_user_id then race condition happens and ActiveRecord::RecordNotUnique is raised. I tried to use database lock to solve this problem, but it didn't work as expected - exception is still raised sometimes.

The table structure looks like this:

create_table :users do |t|
    t.integer :external_user_id, index: { unique: true }
    # ...
end

updated method - what I'm doing wrong here?:

def create_or_update_user(external_user_id, data)
  user = User.find_or_initialize_by(external_user_id: external_user_id)
  user.with_lock do
    user.title = data[:title].downcase
    # and so on - here we assign other attributes from data
    user.save! if user.changed?
  end
end

I can't use upsert, because I need model callbacks.

It probably can be fixed by adding

rescue ActiveRecord::RecordNotUnique
  retry
end

but I want to use locks for best performance cause the race conditions is not a rare in this part of code.

UPD: added a gist to reproduce this race condition https://gist.github.com/krelly/520a2397f64269f96489c643a7346d0f

CodePudding user response：

If I'm understanding correctly, the error is likely still occurring in cases where two threads attempt to create a new User for an external id that doesn't exist.

In this situation, thread #1 would attempt to acquire a row lock on a row that does not exist - no locking would actually occur.

Thread #2 would be able to make the same find_or_initialize query while thread #1 is still building the record, and thus thread #2 would hit a uniqueness violation once it commits.

As you've already guessed, the only simple solution to this problem would be a rescue retry. Thread #2 would just try again, and it would update the record created by thread #1.

An alternative solution would be an advisory lock - a lock with application-specific behavior. In this case, you would be saying that only one thread at a time can run create_or_update_user with a specific external ID.

Rails doesn't support advisory locks natively, but the linked article contains an example of how you would do it with Postgres. There's also the gem with_advisory_locks