How to avoid race condition in a rails model that records payments and running balance?

Question

I have a simple model, Payments, which has two fields amount and running_balance. When a new payment record is created, we look for the running_balance of its previous payment, say last_running_balance and save last_running_balance+amount as the running_balance of the current payment.

Here are our three failed attempts to implement the Payments model. For simplicity, assume the previous payment always exists and ids are increasing as payments are created.

Attempt 1:

class Payments < ActiveRecord::Base
    before_validation :calculate_running_balance
    private
    def calculate_running_balance
        p = Payment.last
        self.running_balance = p.running_balance + amount
    end
end

Attempt 2:

class Payments < ActiveRecord::Base
    after_create :calculate_running_balance
    private
    def calculate_running_balance
        p = Payment.where("id < ?", id).last
        update!(running_balance: p.running_balance + amount)
    end
end

Attemp 3:

class Payments < ActiveRecord::Base
    after_commit :calculate_running_balance
    private
    def calculate_running_balance
        p = Payment.where("id < ?", id).last
        update!(running_balance: p.running_balance + amount)
    end
end

These implementations may cause race conditions in the system as we are using sidekiq to create payments in the background. Suppose the last payment is payment 1. When two new payments, say payment 2 and payment 3 are created at the same time, their running_balance might be computed based on the running balance of payment 1 because it might be the case that when payment 3 is figuring out its running balance payment 2 has not been saved to the database yet.

In particular, I am interested in a fix that avoids the running condition. I am also keen on looking at other rails apps that implement similar payment systems.

Answer 1

Update: this is the first version, for an actually working approach, see below:

You can get rid of race conditions if you lock the last payment when calculating the last balance using pessimistic locking. For this to work you always need to wrap creating the payments with transaction block.

class Payments < ActiveRecord::Base
  before_create :calculate_running_balance

  private
  def calculate_running_balance
    last_payment = Payment.lock.last
    self.running_balance = last_payment.running_balance + amount
  end
end

# then, creating a payment must always be done in transaction
Payment.transaction do
  Payment.create!(amount: 100)
end

The first query to get the last Payment will also lock the record (and delay further querying it) for the duration of the transaction that wraps it, i.e. until the moment the transaction is fully committed and the new record created. If another query meanwhile also tries to read the locked last payment, it will have to wait until the first transaction is finished. So if you use a transaction in your sidekiq when creating the payment, you should be safe.

See the above-linked guide for more info.

Update: it's not that easy, this approach can lead to deadlocks

After some extensive testing, the problem seems to be more complex. If we lock just the "last" payment record (which Rails translate to SELECT * FROM payments ORDER BY id DESC LIMIT 1), then we may run into a deadlock.

Here I present the test that leads to deadlock, the actually working approach is further below.

In all tests below I'm working with a simple InnoDB table in MySQL. I created the simplest payments table with just the amount column added the first row and the accompanying model in Rails, like this:

# sql console
create table payments(id integer primary key auto_increment, amount integer) engine=InnoDB;
insert into payments(amount) values (100);

# app/models/payments.rb
class Payment < ActiveRecord::Base
end

Now, let's open two Rails consoles, start a long-running transaction with last record lock and new row insertion in the first one and another last row lock in the second console session:

# rails console 1
>> Payment.transaction { p = Payment.lock.last; sleep(10); Payment.create!(amount: (p.amount + 1));  }
D, [2016-03-11T21:26:36.049822 #5313] DEBUG -- :    (0.2ms)  BEGIN
D, [2016-03-11T21:26:36.051103 #5313] DEBUG -- :   Payment Load (0.4ms)  SELECT  `payments`.* FROM `payments`  ORDER BY `payments`.`id` DESC LIMIT 1 FOR UPDATE
D, [2016-03-11T21:26:46.053693 #5313] DEBUG -- :   SQL (1.0ms)  INSERT INTO `payments` (`amount`) VALUES (101)
D, [2016-03-11T21:26:46.054275 #5313] DEBUG -- :    (0.1ms)  ROLLBACK
ActiveRecord::StatementInvalid: Mysql2::Error: Deadlock found when trying to get lock; try restarting transaction: INSERT INTO `payments` (`amount`) VALUES (101)

# meanwhile in rails console 2
>> Payment.transaction { p = Payment.lock.last; }
D, [2016-03-11T21:26:37.483526 #8083] DEBUG -- :    (0.1ms)  BEGIN
D, [2016-03-11T21:26:46.053303 #8083] DEBUG -- :   Payment Load (8569.0ms)  SELECT  `payments`.* FROM `payments`  ORDER BY `payments`.`id` DESC LIMIT 1 FOR UPDATE
D, [2016-03-11T21:26:46.053887 #8083] DEBUG -- :    (0.1ms)  COMMIT
=> #<Payment id: 1, amount: 100>

The first transaction ended up with deadlock. One solution would be to use the code from the beginning of this answer but retry the whole transaction when a deadlock occurs.

Possible solution with retrying deadlocked transaction: (untested)

With taking the advantage of the method for retrying lock errors by @M.G.Palmer in this SO answer:

retry_lock_error do
  Payment.transaction 
    Payment.create!(amount: 100)
  end
end

When a deadlock occurs, the transaction is retried, i.e. a fresh last record is found and used.

Working solution with test

Another approach that I came across is to lock all records of the table. This can be done by locking the COUNT(*) clause and it seems to work consistently:

# rails console 1
>> Payment.transaction { Payment.lock.count; p = Payment.last; sleep(10); Payment.create!(amount: (p.amount + 1));}
D, [2016-03-11T23:36:14.989114 #5313] DEBUG -- :    (0.3ms)  BEGIN
D, [2016-03-11T23:36:14.990391 #5313] DEBUG -- :    (0.4ms)  SELECT COUNT(*) FROM `payments` FOR UPDATE
D, [2016-03-11T23:36:14.991500 #5313] DEBUG -- :   Payment Load (0.3ms)  SELECT  `payments`.* FROM `payments`  ORDER BY `payments`.`id` DESC LIMIT 1
D, [2016-03-11T23:36:24.993285 #5313] DEBUG -- :   SQL (0.6ms)  INSERT INTO `payments` (`amount`) VALUES (101)
D, [2016-03-11T23:36:24.996483 #5313] DEBUG -- :    (2.8ms)  COMMIT
=> #<Payment id: 2, amount: 101>

# meanwhile in rails console 2
>> Payment.transaction { Payment.lock.count; p = Payment.last; Payment.create!(amount: (p.amount + 1));}
D, [2016-03-11T23:36:16.271053 #8083] DEBUG -- :    (0.1ms)  BEGIN
D, [2016-03-11T23:36:24.993933 #8083] DEBUG -- :    (8722.4ms)  SELECT COUNT(*) FROM `payments` FOR UPDATE
D, [2016-03-11T23:36:24.994802 #8083] DEBUG -- :   Payment Load (0.2ms)  SELECT  `payments`.* FROM `payments`  ORDER BY `payments`.`id` DESC LIMIT 1
D, [2016-03-11T23:36:24.995712 #8083] DEBUG -- :   SQL (0.2ms)  INSERT INTO `payments` (`amount`) VALUES (102)
D, [2016-03-11T23:36:25.000668 #8083] DEBUG -- :    (4.3ms)  COMMIT
=> #<Payment id: 3, amount: 102>

By looking at the timestamps you can see that the second transaction waited for the first one to finish and the second insert already "knew" about the first one.

So the final solution that I propose is the following:

class Payments < ActiveRecord::Base
  before_create :calculate_running_balance

  private
  def calculate_running_balance
    Payment.lock.count # lock all rows by pessimistic locking
    last_payment = Payment.last # now we can freely select the last record
    self.running_balance = last_payment.running_balance + amount
  end
end

# then, creating a payment must always be done in transaction
Payment.transaction do
  Payment.create!(amount: 100)
end