Forum: Ruby on Rails concurrency issue

Announcement (2017-05-07): www.ruby-forum.com is now read-only since I unfortunately do not have the time to support and maintain the forum any more. Please see rubyonrails.org/community and ruby-lang.org/en/community for other Rails- und Ruby-related community platforms.
unknown (Guest)
on 2007-05-15 00:32
(Received via mailing list)
Hi,

On my site, if a lot of people place orders at the same time, the
available quantity left in stock for a product seems to get wrong
every so often.

Here's my execute_purchase method, which is a before_filter on the
Order model:

    def execute_purchase
      Product.transaction do
        product.lock!
        if product.quantity >= quantity # enough in stock
          if credit_card_needed?
            response = authorize_payment
            if response.success?
              product.update_attribute(:quantity, product.quantity -
self.quantity)
            else
              raise FulfillmentError, response.message
            end
          end
        else
          raise FulfillmentError, "not enough in stock"
        end
      end
    rescue FulfillmentError => e
      errors.add_to_base e.message
      return false
    end

So, I enter the transaction.  Lock the product.  I see if there's
enough in stock.  If there's enough in stock, then I authorize the
payment -- if that's successful, then I decrement the available
quantity.   Since I'm in a transaction and I've locked the record,
this method should be fine, right?

So, what happens is sometimes we sell more than we have.  So, the
product quantity isn't always updated when an order happens.
Paolo N. (Guest)
on 2007-05-15 00:50
(Received via mailing list)
I'm just trying to guess so forgive me if I'm saying something stupid

Let's think to two almost simultaneous request A and B

this is the timing

1) request A land on your page and load the product (quantity = 3) and
A wants to by 1
2) request B land on your page and load the product (quantity = 3) and
B wants to buy 1
3) request A execute the transaction setting quantity = 3 -1 => 2
4) request B execute the transaction setting quantity = 3 - 1 => 2

That's how you end up with still 2 available even if you've sold 2
items.

update_attribute doesn't run validations so in this case lock_version
doesn't protect you.

Paolo
unknown (Guest)
on 2007-05-15 00:54
(Received via mailing list)
Shouldn't the
   product.lock!
line give me a lock on that row?  So, the other requests need to wait
until the transaction is over with.  Then the product should get
reloaded with current data.

Right?
Paolo N. (Guest)
on 2007-05-15 01:07
(Received via mailing list)
Yes, you lock the row, but the point is at that point it

On 14/05/07, removed_email_address@domain.invalid 
<removed_email_address@domain.invalid> wrote:
>
> Shouldn't the
>    product.lock!
> line give me a lock on that row?  So, the other requests need to wait
> until the transaction is over with.

The above is correct, it waits, but what at this point product is has
been already loaded and it contains quantity 3

>  Then the product should get
> reloaded with current data.

The above is incorrect

If you don't reload explicitly the ActiveRecord object it doesn't
reload itself, it will keep forever a STALE image of the database.

Paolo
Paolo N. (Guest)
on 2007-05-15 01:23
(Received via mailing list)
Sorry for the mess in the previous message, cut and paste error.

Basically when you instantiate an AR object it loads the data at that
moment and never updates until you call the .reload method.
An AR object is not aware of what happens to its image in the database.

Paolo
Paolo N. (Guest)
on 2007-05-15 02:22
(Received via mailing list)
I just noticed you decrease the quantity only when credit_card_needed?

Is that right?

Paolo
Jean-Christophe M. (Guest)
on 2007-05-15 02:24
(Received via mailing list)
Hi,

What about doing preventive stock update:

Le 14 mai 07, à 22:31, removed_email_address@domain.invalid a écrit :
> Here's my execute_purchase method, which is a before_filter on the
> Order model:
>
>     def execute_purchase
>       Product.transaction do
>         if product.quantity >= quantity # enough in stock
>           if credit_card_needed?

UPDATE products SET quantity=quantity-#{product.quantity}

>             response = authorize_payment
>             if response.success?

msg Success

>             else

UPDATE products SET quantity=quantity+#{product.quantity}

>               raise FulfillmentError, response.message
>             end

else # what if credit card is not needed ??

>           end
>         else
>           raise FulfillmentError, "not enough in stock"
>         end
>       end
>     rescue FulfillmentError => e
>       errors.add_to_base e.message
>       return false
>     end

Imho no lock should be necessary in this case.
You simply risk to answer that there's no more stock whereas there is
some 5 ms later.
Better than the opposite.

Jean-Christophe M.
--
symetrie.com

Better Nested Set for rails:
http://opensource.symetrie.com/trac/better_nested_set
unknown (Guest)
on 2007-09-26 00:59
(Received via mailing list)
http://api.rubyonrails.org/classes/ActiveRecord/Lo...

Says that the record should be reloaded.

In fact, here's the source code for lock:

71:       def lock!(lock = true)
72:         reload(:lock => lock) unless new_record?
73:         self
74:       end

Am I not understanding this correctly?

Joe
Paolo N. (Guest)
on 2007-09-26 01:03
(Received via mailing list)
On 14/05/07, removed_email_address@domain.invalid 
<removed_email_address@domain.invalid> wrote:
> 73:         self
> 74:       end
>
> Am I not understanding this correctly?
>
> Joe
>

Sorry, you're definitely right, it should reload the object so what
I've said is not valid.

Paolo
This topic is locked and can not be replied to.