Reputation: 14406
I am building a locking system based on PostgreSQL, I have two methods, acquire
and release
.
For acquire
, it works like this
BEGIN
while True:
SELECT id FROM my_locks WHERE locked = false AND id = '<NAME>' FOR UPDATE
if no rows return:
continue
UPDATE my_locks SET locked = true WHERE id = '<NAME>'
COMMIT
break
And for release
BEGIN
UPDATE my_locks SET locked = false WHERE id = '<NAME>'
COMMIT
This looks pretty straightforward, but it doesn't work. The strange part of it is, I thought
SELECT id FROM my_locks WHERE locked = false AND id = '<NAME>' FOR UPDATE
should only acquire the lock on target row only if the target row's locked
is false
. But in reality, it's not like that. Somehow, even no locked = false
row exists, it acquire lock anyway. As a result, I have a deadlock issue. It looks like this
Release is waiting for SELECT FOR UPDATE
, and SELECT FOR UPDATE
is doing infinite loop while it's holding a lock for no reason.
To reproduce the issue, I wrote a simple test here
https://gist.github.com/victorlin/d9119dd9dfdd5ac3836b
You can run it with psycopg2
and pytest
, remember to change the database setting, and run
pip install pytest psycopg2
py.test -sv test_lock.py
Upvotes: 6
Views: 9316
Reputation: 21336
The test case plays out like this:
SELECT
and acquires the record lock.SELECT
and enters the lock's wait queue.UPDATE
/ COMMIT
and releases the lock.SELECT
, it rechecks the data against its WHERE
condition. The check fails, and the row is filtered out of the result set, but the lock is still held.This behaviour is mentioned in the FOR UPDATE
documentation:
...rows that satisfied the query conditions as of the query snapshot will be locked, although they will not be returned if they were updated after the snapshot and no longer satisfy the query conditions.
This can have some unpleasant consequences, so a superfluous lock isn't that bad, all things considered.
Probably the simplest workaround is to limit the lock duration by committing after every iteration of acquire
. There are various other ways to prevent it from holding this lock (e.g. SELECT ... NOWAIT
, running in a REPEATABLE READ
or SERIALIZABLE
isolation level, SELECT ... SKIP LOCKED
in Postgres 9.5).
I think the cleanest implementation using this retry-loop approach would be to skip the SELECT
altogether, and just run an UPDATE ... WHERE locked = false
, committing each time. You can tell if you acquired the lock by checking cur.rowcount
after calling cur.execute()
. If there is additional information you need to pull from the lock record, you can use an UPDATE ... RETURNING
statement.
But I would have to agree with @Kevin, and say that you'd probably be better off leveraging Postgres' built-in locking support than trying to reinvent it. It would solve a lot of problems for you, e.g.:
The easiest way might be to implement acquire
as SELECT FROM my_locks FOR UPDATE
, release
simply as COMMIT
, and let the processes contend for the row lock. If you need more flexibility (e.g. blocking/non-blocking calls, transaction/session/custom scope), advisory locks should prove useful.
Upvotes: 9
Reputation: 7320
Also, you should add locked = true
in the release code:
BEGIN
UPDATE my_locks SET locked = false WHERE id = '<NAME>' AND locked = true
COMMIT
If not, you are updating the record whatever locked state it is (in your case, even when locked = false), and adding the odds of causing a deadlock.
Upvotes: 1
Reputation: 30151
PostgreSQL normally aborts transactions which deadlock:
The use of explicit locking can increase the likelihood of deadlocks, wherein two (or more) transactions each hold locks that the other wants. For example, if transaction 1 acquires an exclusive lock on table A and then tries to acquire an exclusive lock on table B, while transaction 2 has already exclusive-locked table B and now wants an exclusive lock on table A, then neither one can proceed. PostgreSQL automatically detects deadlock situations and resolves them by aborting one of the transactions involved, allowing the other(s) to complete. (Exactly which transaction will be aborted is difficult to predict and should not be relied upon.)
Looking at your Python code, and at the screenshot you showed, it appears to me that:
locked=true
lock, and is waiting to acquire a row lock.locked=true
lock.locked=true
lock (note the short time on that query; it is looping, not blocking).Since Postgres is not aware of the locked=true
lock, it is unable to abort transactions to prevent deadlock in this case.
It's not immediately clear to me how T2 acquired the row lock, since all the information I've looked at says it can't do that:
FOR UPDATE causes the rows retrieved by the SELECT statement to be locked as though for update. This prevents them from being locked, modified or deleted by other transactions until the current transaction ends. That is, other transactions that attempt UPDATE, DELETE, SELECT FOR UPDATE, SELECT FOR NO KEY UPDATE, SELECT FOR SHARE or SELECT FOR KEY SHARE of these rows will be blocked until the current transaction ends; conversely, SELECT FOR UPDATE will wait for a concurrent transaction that has run any of those commands on the same row, and will then lock and return the updated row (or no row, if the row was deleted). Within a REPEATABLE READ or SERIALIZABLE transaction, however, an error will be thrown if a row to be locked has changed since the transaction started. For further discussion see Section 13.4.
I was not able to find any evidence of PostgreSQL "magically" upgrading row locks to table locks or anything similar.
But what you're doing is not obviously safe, either. You're acquiring lock A (the row lock), then acquiring lock B (the explicit locked=true
lock), then releasing and re-acquiring A, before finally releasing B and A in that order. This does not properly observe a lock hierarchy since we try both to acquire A while holding B and vice-versa. But OTOH, acquiring B while holding A should not fail (I think), so I'm still not sure this is outright wrong.
Quite frankly, it's my opinion that you'd be better off just using the LOCK TABLE
statement on an empty table. Postgres is aware of these locks and will detect deadlocks for you. It also saves you the trouble of the SELECT FOR UPDATE
finagling.
Upvotes: 2