On 2011-01-17 9:28 AM +0200, Itagaki Takahiro wrote:
Here is a short review for Transaction scoped advisory locks:
https://commitfest.postgresql.org/action/patch_view?id=518

Thanks for reviewing!

== Features ==
The patch adds pg_[try_]advisory_xact_lock[_shared] functions.
The function names follows the past discussion -- it's better than
"bool isXact" argument or changing the existing behavior.

== Coding ==
I expect documentation will come soon.

I'm sorry about this, I have been occupied with other stuff. I'm going to work on this tonight.

There is no regression test, but we have no regression test for
advisory locks even now. Tests for lock conflict might be difficult,
but we could have single-threaded test for lock/unlock and pg_locks view.

Seems useful.

== Questions ==
I have a question about unlocking transaction-scope advisory locks.
We cannot unlock them with pg_advisory_unlock(), but can unlock with
pg_advisory_unlock_all(). It's inconsistent behavior.
Furthermore, I wonder we can allow unlocking transaction-scope locks
-- we have LOCK TABLE but don't have UNLOCK TABLE.

I guess we could add new pg_advisory_txn_unlock() functions to unlock transaction-scope locks, but I do share your doubt on whether or not we want to allow this at all. On the other hand, the reasons why we don't allow non-advisory locks to be unreleased is a lot more clear than the issue at hand. I have no strong opinion on this.

Another thing I now see is this:

BEGIN;
SELECT pg_advisory_xact_lock(1);

-- do something here

-- upgrade to session lock
SELECT pg_advisory_lock(1);
COMMIT;


This seems useful, since the xact lock would be automatically released if an error happens during "-- do something here" so you wouldn't need to worry about releasing the lock elsewhere. But I'm not sure this is safe. Can anyone see a problem with it?


Regards,
Marko Tiikkaja

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to