Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Use of "FOR UPDATE" during Magento stock quantity update

High level summary of the issue: Getting issues around locking of the inventory table when placing orders resulting in order failures due to timeouts.

Tracing through the checkout process, I see the following queries being executed: (comments added by me)

-- Lock stock and product tables
SELECT `si`.*, `p`.`type_id` FROM `cataloginventory_stock_item` AS `si`
INNER JOIN `catalog_product_entity` AS `p` ON p.entity_id=si.product_id
WHERE (stock_id=1) AND (product_id IN(28775, 28777)) FOR UPDATE

-- Perform the actual stock update
UPDATE `cataloginventory_stock_item`
SET `qty` = 
    CASE product_id
        WHEN 28775 THEN qty-2
        WHEN 28777 THEN qty-1
    ELSE
        qty
    END
WHERE (product_id IN (28775, 28777)) AND (stock_id = 1)

My understanding of the FOR UPDATE modifier of a SELECT statement is that all rows in tables that were returned in the SELECT will be locked (read and write?) until the transaction is committed.

From my understanding of MySQL, the fact that the cataloginventory_stock_item query has a calculated value for the qty column (i.e. the value wasn't calculated in PHP and passed into the query, the new column value is based on the existing column value when the query is performed) means it will not be susceptible to race conditions.

My questions are:

  • Are my assumptions correct?
  • Why does Magento need a lock of catalog_product_entity in order to update the stock?
  • Why does Magento need a lock of cataloginventory_stock_item if the cataloginventory_stock_item UPDATE is atomic?
like image 281
Nick Avatar asked Jan 13 '14 13:01

Nick


1 Answers

1) Yes, your assumptions regarding FOR UPDATE are correct, the rows selected in both cataloginventory_stock_item and catalog_product_entity will be locked for reading and writing. That is, other queries for these rows will block.

2) I don't know, and in fact it seems it doesn't.. Perhaps this to prevent race conditions when a user is manually updating stock status or similar, but I still don't see why it couldn't be removed. Another possibility is the original author intended to support multiple stock items per product and thought the "parent" should be locked.

3) Because the PHP code checks if the item is "salable" using the loaded values before issuing the update. Without locking, two processes could load the same value and then race to update the value. So even though it is atomic, the query doesn't fail properly if there was a race condition when loading the data.

like image 101
ColinM Avatar answered Oct 24 '22 09:10

ColinM