Re: Exposing the lock manager's WaitForLockers() to SQL

Hi,

On 2023-01-11 23:03:30 -0800, Will Mortensen wrote:
> On Wed, Jan 11, 2023 at 12:33 PM Andres Freund <andres(at)anarazel(dot)de> wrote:
> > I think such a function would still have to integrate enough with the lock
> > manager infrastructure to participate in the deadlock detector. Otherwise I
> > think you'd trivially end up with loads of deadlocks.
>
> Could you elaborate on which unusual deadlock concerns arise? To be
> clear, WaitForLockers() is an existing function in lmgr.c
> (https://github.com/postgres/postgres/blob/216a784829c2c5f03ab0c43e009126cbb819e9b2/src/backend/storage/lmgr/lmgr.c#L986),
> and naively it seems like we mostly just need to call it.

I know that WaitForLockers() is an existing function :). I'm not sure it's
entirely suitable for your use case. So I mainly wanted to point out that if
you end up writing a separate version of it, you still need to integrate with
the deadlock detection. WaitForLockers() does that by actually acquiring a
lock on the "transaction" its waiting for.

> To my very limited understanding, from looking at the existing callers and
> the implementation of LOCK, that would look something like this (assuming
> we're in a SQL command like LOCK and calling unmodified WaitForLockers()
> with a single table):
>
> 1. Call something like RangeVarGetRelidExtended() with AccessShareLock
> to ensure the table is not dropped and obtain the table oid
>
> 2. Use SET_LOCKTAG_RELATION() to construct the lock tag from the oid
>
> 3. Call WaitForLockers(), which internally calls GetLockConflicts() and
> VirtualXactLock(). These certainly take plenty of locks of various types,
> and will likely sleep in LockAcquire() waiting for transactions to finish,
> but there don't seem to be any unusual pre/postconditions, nor do we
> hold any unusual locks already.

I suspect that keeping the AccessShareLock while doing the WaitForLockers() is
likely to increase the deadlock risk noticeably. I think for the use case you
might get away with resolving the relation names, building the locktags, and
then release the lock, before calling WaitForLockers. If somebody drops the
table or such, you'd presumably still get desired behaviour that way, without
the increased deaadlock risk.

Greetings,

Andres Freund