Rails/ActiveRecord Pessimistic Locking - Do I need to reload after obtaining a lock?

activerecord, concurrency, delayed-job, locking, ruby-on-rails-3

Solution

Both approaches are valid and will work. In version 1 `reload` is not necessary however - locking automatically reloads the record, so you can remove it.

And then if you check the source code for `with_lock` and `lock!`, you will find your two versions are 100% equivalent:

def lock!(lock = true)
  reload(:lock => lock) if persisted?
  self
end

def with_lock(lock = true)
  transaction do
    lock!(lock)
    yield
  end
end

Using `with_lock` would be most simple and preferred:

# Obtain row-lock
with_lock do
  # Check status after lock/reload
  return unless status == :new

  delay.settle_credit_card
  update_attribute(:status, :pending)
end

(note: you can safely drop `self` from method calls)

Problem

I have a Transaction model in Rails representing a financial transaction which will be charged to a credit card. When a transaction is created, it's status is `:new`. When I attempt to settle a charge (which will occur in DelayedJob) I update the status to `:pending`. Any subsequent calls to `charge` will be ignored if status is not `:new`. Naive Version (not concerned with concurrency): ``` # Trigger a card to be charged def charge_transaction return unless status == :new self.transaction do self.delay.settle_credit_card self.update_attribute(:status, :pending) end end # Actually do the charging (in a delayed worker) def settle_credit_card # ... Interact with our payment gateway end ``` Since this is a load_balanced web app, I want to make sure we are taking concurrency into account and not creating duplicate charges (due to concurrent requests). I understand the benefits of optimistic locking, but in this case I don't mind having this critical region because simultaneous attempts to charge (or update a transaction in any way) should be an exceptional case. Here's an attempt at using pessimistic row-level locking Concurrent Version (option 1) ``` # Trigger a card to be charged def charge_transaction # Obtain row-lock self.with_lock do self.reload # Reload once lock is obtained - necessary? # Check status after lock/reload return unless status == :new self.delay.settle_credit_card self.update_attribute(:status, :pending) end end ``` Concurrent Version (option 2) ``` # Trigger a card to be charged def charge_transaction # Begin transaction without lock self.transaction do self.reload(lock: true) # Reload and obtain lock # Check status after lock/reload return unless status == :new self.delay.settle_credit_card self.update_attribute(:status, :pending) end end ``` Are either (or both) of these approaches valid? Is the explicit reload necessary once obtaining the lock (to ensure transaction object is current) or will Rails do that automatically when obtaining the lock? If both approaches are valid, which is preferable? Thanks a lot!

Original source