3

The magento ORM layer doesn`t seem to handle concurrent saving of the same object gracefully. When saving a model the ORM issues an update sql query that just tests for the primary key.

Lets say you updated the customer_lastname of an order and save it to the database. The ORM layer will issue a query like this:

UPDATE sales_flat_order SET customer_lastname=`Test` WHERE entity_id=3

If another process (e.g. the paypal ipn controller) changed the same order at the same time (e.g. mark the order as payed), its changes might be overwritten.

This seems to me like a major flaw in the design of the ORM. Is there a reason the ORM layer doesn`t test for changes in the database upon saving?

The ORM layer might issue a query like:

UPDATE sales_flat_order SET customer_lastname=`Test` WHERE entity_id=3
       AND state='pending' AND status='pending' …

If the number of altered rows returned by the db is 1 it is certain that the database had not changed since the model was loaded. If the number of altered rows is 0 then the database was changed by another process after the model was loaded and the new changes were not saved and the save method should fail.

Is there a reason Magento doesn`t already do that (perhaps performance)?

Update

When creating a shipment in magento this happens (not only when creating a shipment, but this mechanism can be found throughout the codebase):

  1. Load the order from the database using SELECT into model object
  2. Create shipment model object from order
    • Loads all order items in model objects
    • gets QtyToShip from item model object (qty_ordered - qty_shipped - qty_refunded - qty_canceled)
    • updates qty_shipped in order_item model object
  3. Save in DB
    • START TRANSACTION
    • save shipment model object in db
    • save order model object in db
    • save order_item model objects in db
    • COMMIT

When two processes try to create a shipment for the same order at the same time and process B is able to finish step 3 outlined above while process A is in step 2 then once process A finishes step 3 the changes made to the order_items by process B are overwritten.

Improve this question
4
  • Because concurrent writes to the same model isn't a problem of the ORM, but a feature for its consumer. Only the application layer can determine whether it's crucial and transaction isolation is or rather should be used whenever it applies. Commented Feb 25, 2015 at 8:18
  • So, you are saying that it is the application layers responsibility to use this feature, but I do not think the magento ORM provides this feature. The underlying Zend_Db seems to provide that through SELECT ... FOR UPDATE but the orm doesn`t provide a way to use it. Commented Feb 25, 2015 at 8:40
  • Yes, that's one of many short comings. Using some of the more complex database operations requires one to drill into Zend. Is there a specific issue you ran into that prompted this question? Commented Feb 25, 2015 at 9:07
  • When two admins added a shipment at the same time, the order item was shipped twice. Before adding an item to a shipment magento checks qty_shipped in sales_flat_order_item. If there is still an item to be shipped magento adds it to the shipment and updates qty_shipped in sales_flat_order_item. In this case one update was overwritten by the other and the same item appeared on two shipments. Commented Feb 25, 2015 at 9:25

1 Answer 1

Reset to default
1

If you have a test for this, can you run it with READ COMMITTED isolation level? See the discussion here: https://code.djangoproject.com/ticket/13906

And yes, I'm not convinced either the transaction isolation will fix the issue. But with repeatable reads, you cannot solve the problem period. Both selects within each transaction will not see the updates. So your check for "has this changed and should I rollback" will yield "no" for both transactions.

An undesirable fix is to lock the table for the update as it will stall quote-to-order conversion in the checkout.

Perhaps the real fix is to put UNIQUE(order_item_id) on sales_flat_shipment_item?

Improve this answer
6
  • Tomorrow I could simulate a concurrent change of the database. On adding a shipment to an order the qty_shipped field of the corresponding sales_flat_order_item is updated. I could step through the code using xdebug and pause directly before the update of that table and change the value in the database directly. That should simulate an update done by another process (e.g. another admin creating a shipment). I doubt that the transaction isolation level makes a difference, but I will test it. What do you think of this method? Commented Feb 25, 2015 at 21:29
  • WHen using firebug or chrome dev tools, you can copy a request "as curl". If you fire both up on the command line where the first is put in the background, you should have near concurrent requests firing. With "both" I mean, using two different logins, so two different adminhtml cookies and perhaps /key/ part of the URL, but otherwise identical request. Commented Feb 26, 2015 at 0:02
  • Thanks for the clarifying update. I see your point. I've dug through the mysql manual and according to dev.mysql.com/doc/refman/5.7/en/innodb-consistent-read.html the repeatable read isolation level doesn`t apply to DML statements. Issuing a plain select statement won't help, but an UPDATE statement that tests for the old values should work. Commented Feb 26, 2015 at 7:22
  • I could not reproduce the malicious behaviour with chrome and curl. It is a one in a million shot to get the timing right. I will try to add my findings to my question. Commented Feb 26, 2015 at 7:23
  • 1
    I also think that the second update clause I've written in my question would prevent these problems. I would have expected that magento already prevents errors like this. As an ecommerce solution data integrity should be a top concern. So perhaps there is more to it than meets the eye. Perhaps there is a reason magento doesn't already do that. Commented Feb 26, 2015 at 9:46

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.