1

I have a lot of console applications that perform different tasks. I getting unique task from php script:

$mysqli->autocommit(FALSE);
$result = $mysqli->query("SELECT id, task FROM queue WHERE locked = 0 LIMIT 1 FOR UPDATE;");

while($row = $result->fetch_assoc()){ 
    $mysqli->query('UPDATE queue SET locked = 1 WHERE id="'.$row['id'].'";');
    $mysqli->commit();
    $response["response"]["task"] = $row["task"];
}

$mysqli->close();   
echo json_encode($response);

Sometimes I have duplicate task and, "Deadlock found when trying to get lock; try restarting transaction". What am I doing wrong?

UPD: set index on "locked" column solve problem

Noe D.
  • 13
  • 4
  • 1
    Don't think it's related to the error, but why are you using a `while` loop when you specify `LIMIT 1`? It can never return more than one row, so you don't need to loop. – Barmar Jan 04 '17 at 15:58
  • 1
    Do you have an index on the `locked` column? Try adding it. – Barmar Jan 04 '17 at 16:00
  • Your application should handle such errors according to http://dev.mysql.com/doc/refman/5.7/en/innodb-deadlocks-handling.html and then resubmit, in case of an error. Setting an index, and keeping the transactions small can minimize the deadlock risk. – user3606329 Jan 04 '17 at 16:02
  • This code makes no sense! You set up to run a transaction and then commit after each update?? You Select using `LIMIT 1` and then while loop over a single result?? – RiggsFolly Jan 04 '17 at 16:09
  • Is this code also in an outer while loop? – RiggsFolly Jan 04 '17 at 16:10
  • @Barmar seems index solved this problem, thx :) – Noe D. Jan 04 '17 at 17:48
  • @RiggsFolly Since it's at most a single row, the `while` is effectively an `if`. – Barmar Jan 04 '17 at 18:09
  • 1
    I've seen hundreds of questions here where people use `while` to process the result of a query, even though it can only return 1 row, simply because that's the pattern they're used to. Newbie programmers don't design based on logic, they just copy what they've seen or written before without thinking about it. – Barmar Jan 04 '17 at 18:10
  • @Barmar I agree. Thats no reason not to point out that it is unnecessary/illogical though in my opinion – RiggsFolly Jan 04 '17 at 18:12
  • @RiggsFolly I know, did you read my first comment where I pointed out the same folly? – Barmar Jan 04 '17 at 18:14
  • Maybe this question is also relevant: http://stackoverflow.com/questions/21851119/deadlock-using-select-for-update-in-mysql/23244630#23244630 – Barmar Jan 04 '17 at 18:16

1 Answers1

0

From the MySQL documentation How to Minimize and Handle Deadlocks:

Add well-chosen indexes to your tables. Then your queries need to scan fewer index records and consequently set fewer locks.

Adding an index to the locked column should solve this. Without it, the SELECT query has to scan through the table, looking for a row with locked = 0, and all the rows it steps through must be locked.

If you add an index to the column in the WHERE clause, it can go directly to that record and lock it. No other records are locked, so the possibility of deadlock is reduced.

Barmar
  • 741,623
  • 53
  • 500
  • 612