How to handle race conditions on a complex custom validation in Rails?

Question

People are making reservations using our Rails web app.

We get high traffic when reservations are made available, and sometimes two visitors get a valid reservation when there was only one left.

My validation is a bit complex, but here's a simplified version which gives an idea of part of what is being checked:

validate :time_availability

def time_availability
  if Reservation.where(date: date, arrival_time: arrival_time).count >= ReservationMax.for(date, arrival_time)
    errors.add(:arrival_time, "This time is not available")
  end
end

How would you make sure that two simultaneous requests don't both save when one of them saving should make the other invalid?

Answer 1

I solved it in a pretty interesting way which does not require any locking, rolling back, or extra transaction wrapping, but uses a unique index at the database level.

I added a column to the Reservation model called seat_number and added a unique index on date, arrival_time, and seat_number:

class AddSeatNumberToReservations < ActiveRecord::Migration[5.0]
  def change
    add_column :reservations, :seat_number, :integer
    Reservation.update_all("seat_number=id") // so that existing reservations have a unique seat_number
    add_index :reservations, [:date, :arrival_time, :seat_number], unique: true
  end
end

Then I use an around_save to set the seat_number based on how many reservations already exist for that date and time, and rescue from ActiveRecord::RecordNotUnique and retry the save with a new seat number

class Reservation < ApplicationRecord
  around_save :check_for_race_condition

  def check_for_race_condition
    seat_count = Reservation.where(date: date, arrival_time: arrival_time).count
    begin
      self.seat_number = seat_count + 1
      yield
    rescue ActiveRecord::RecordNotUnique
      if seat_count + 1 >= ReservationMax.for(date, arrival_time)
        errors.add(:arrival_time, "This time is not available")
      else
        seat_count += 1
        retry
      end
    end
  end
end

It works beautifully.

In my multi-thread test, I set the db pool to 15, the reservation max (for a specific date and time) to 9, and run 14 simultaneous threads (apart from the main thread) trying to save a reservation for that date and time. The result is 9 reservations with seat_numbers 1 through 9, in order, and the remaining 5 gracefully return the reservation with the correct error.

Answer 2

Not tested yet, but I have an idea. It came to me after commenting with coorasse on his answer about database indexes.

If there was another model for recording when a date and time were full, then before_saveing the Reservation I could create that record and the database would take care of uniqueness.

Migration for the new model and table:

class CreateFullReservationTimes < ActiveRecord::Migration[5.0]                                                                   
  def change
    create_table :full_reservation_times do |t|
      t.date :date
      t.datetime :arrival_time

      t.index [:date, :arrival_time], unique: true

      t.timestamps
    end
  end
end

Add before_save to the Reservation model:

class Reservation < ApplicationRecord
  before_save :add_full_reservation_time, if: :arrival_time_will_be_full?

  def add_full_reservation_time
    begin
      FullReservationTime.create!(date: date, arrival_time: arrival_time)                                                         
    rescue ActiveRecord::RecordNotUnique
      errors.add(:arrival_time, "This time is not available")
      throw :abort
    end
  end

  def arrival_time_will_be_full?
    Reservation.one_or_none_left_at?(date, arrival_time)                                                                                     
  end
end

That way, if there's one (or no) reservations left before saveing, it tries to create a FullReservationTime (and if there's a race condition for two simultaneous "last reservations" only one will succeed due to ActiveRecord::RecordNotUniqe

---

UPDATE

I'm testing this using multiple threads to try to save multiple records at the same time.

If I start with one reservation left, the test passes and only one reservation is saved!

If I start with two reservations left, and run 3 simultaneous threads, it fails because they all save before there's time to check if there's one or none left.

I tried the method from max's answer (wrapping in a transaction, performing the validation after the save, and raising an ActiveRecord::Rollback if the validation fails, and the test did not pass. All threads successfully save the record...

Answer 3

I'm not sure a model validation will work here due to the potential race condition - instead you need to wrap it in a transaction and do it backwards:

date, arrival_time = @reservation.date, @reservation.arrival_time
Reservation.transaction do
  @reservation.save!
  unless Reservation.where(date: date, arrival_time:arrival_time).count >= ReservationMax.for(date, arrival_time)
    raise ActiveRecord::Rollback, "This time is not available"
  end
end

if @reservation.persisted?
  redirect_to @reservation
else
  redirect_to :somewhere_else
end

This creates a pessimistic save and only commits the write if the "validation" succeeds. This removes the potential race condition between the validation being run and the actual insert being performed.

Answer 4

You need to encapsulate everything in a Transaction if you need to do more operations:

def create
  Reservation.transaction do
    reservation = Reservation.new(parsed_params)
    if reservation.save
      #do other things within the transaction
    else
      #...
    end
  end
end

Please read also: http://api.rubyonrails.org/classes/ActiveRecord/Locking/Pessimistic.html

and for optimistic locking: http://api.rubyonrails.org/classes/ActiveRecord/Validations/ClassMethods.html#method-i-validates_uniqueness_of-label-Concurrency+and+integrity

your code should raise an StatementInvalid exception if you have an index.