0

I have following interaction (pseudocode):

start transaction
  select delivery for update                     # semaphore
  select delivery_items where processed = false  # get unprocessed data and process it
  update delivery_items SET processed = true
commit                                           # release semaphore

I was expecting that this will be enough to lock the delivery so that no two people will be able to perform actions on its items at the same time, but for some reason I am getting race conditions where two people do manage to modify delivery_items (I am getting the same items processed twice).

Am I not understanding ACID correctly?
Should the first select not block until the other user finishes transaction with a commit preventing race condition between second select and update?

All MySQL settings are set to default (except some memory pool sizes).
I am using Doctrine if that makes any difference.
I am also using nested transactions (doctrine implements them using SAVEPOINTs).

2 Answers 2

2

You understand the locking logic correctly, however I suspect the nested transactions are causing your problem. (Without seeing your actual code it is hard to be sure, but the following is a likely explanation.)

If your first SELECT statement in a transaction is not the locking SELECT ... FOR UPDATE then you are going to have issues, given that the default transaction isolation level is READ-REPEATABLE.

Assuming that you do have an earlier SELECT in your transaction, then the whole transaction is taking a snapshot of the database state at the moment, and negating the benefit of the locking.

In detail, here is what I think is causing your race conditions:

Step 1. Request1: SELECT statement without FOR UPDATE (snapshot is taken here)

Step 2. Request1: SELECT ... FOR UPDATE starts a lock

Step 3. Request1: SELECT processed=false

Step 4. Request2: SELECT statement without FOR UPDATE (snapshot is taken here)

Step 5. Request2: SELECT ... FOR UPDATE waits for Request1 to finish

Step 6. Request1: UPDATE processed=true

Step 7. Request1: COMMIT

Step 8. Request2: SELECT processed=false (will see the database at it was at step 4, BEFORE the processed rows were changed, so the query will return the same rows as Request1 did)

Creating a SAVEPOINT will not prevent this issue.

I wrote about this issue in the past on stackoverflow.

You psuedocode says you aren't doing that, but the comment about nested transactions suggests that you may already have an open transaction at the time your psuedocode starts, which is causing an "incorrect" snapshot to be taken for your needs.

My advice is the same as in my linked post: Make sure your transaction starts with the SELECT ... FOR UPDATE statement. That means committing the earlier transaction, rather than using the nesting logic, as simply making a SAVEPOINT as Doctrine does won't suffice to make sure you are working from the correct snapshot of data.

If you are using nested transactions in an attempt to wrap an entire HTTP request in a transaction, and then using nested transactions deeper down in the request, then this is an expected problem, and I suggest not trying to wrap an entire request like that. (I only mention that because I used to naively think it was a good idea to wrap an entire request like that... and then discovered the hard way issues like I outlined in this answer.)

3
  • I do have earlier selects, yes. I cannot remove nested transactions since I have a command based code where a command triggers events that can trigger another command. Each command is its own transaction, but if an exception is thrown a transaction is reverted instead of committing. If an exception is not handled it can bubble all the way to the first command and revert everything.
    – HubertNNN
    Apr 12 at 6:49
  • Is there any way to force the snapshot to be updated? I found out that doing select delivery_items for update gives the correct result while without for update indeed returns old data. But changing all selects into select for update feels like a deadlock waiting to happen.
    – HubertNNN
    Apr 12 at 6:55
  • 1
    @HubertNNN If you can't change the earlier code, then yes, making select delivery_items for update will pull the true data from the database, instead of reading from the snapshot. It's a trade off due to your design, it looks like, but as long as this is the only way those rows are locked, then you have a consistent pattern that should (hopefully) avoid deadlocks. Apr 12 at 13:02
0

Syntax problems. Perhaps you meant something like this:

start transaction;
  select * FROM delivery_items
           where processed = false FOR UPDATE; # get unprocessed data and process it
  ... ((process the rows found by the above SELECT))
  update delivery_items SET processed = true
           WHERE processed = false;
commit;                                           # release semaphore

Presumably, the "processing" would act on just the 'false' rows. And those are the only rows that really need 'locking' for ACID.

Without a suitable index, possibly all the rows in the table will be locked. You might consider having an index starting with processed. With that, only the "false" rows would be "locked".

Hmmm... That raises the question of what happens if a separate connection inserts/updates another row to have 'false'. It might be safer for the UDPATE to modify, not the 'processed=false' rows but rather the rows returned by the SELECT.

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.