Re: Transaction-scope advisory locks

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