Should PostgresMain() do a LWLockReleaseAll()?

Hi,

Currently the error handling of normal backends only does a
LWLockReleaseAll() once CurrentTransactionState->state != TRANS_DEFAULT
because it's called in AbortTransaction(). There's pretty damn few
places that fiddle with lwlocks outside of a transaction command, but I
still do wonder whether it'd wouldn't be a tad more robust to
unconditionally do a LWLockReleaseAll(), just like other error handlers
are doing?
In comparison to the cost of a longjmp and the rest of error handling
that ought to be nearly free.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> Currently the error handling of normal backends only does a
> LWLockReleaseAll() once CurrentTransactionState->state != TRANS_DEFAULT
> because it's called in AbortTransaction(). There's pretty damn few
> places that fiddle with lwlocks outside of a transaction command, but I
> still do wonder whether it'd wouldn't be a tad more robust to
> unconditionally do a LWLockReleaseAll(), just like other error handlers
> are doing?

Why do that thing in particular, and not all the other things that
AbortTransaction() does?

The reason that other process main loops don't use AbortTransaction is
that they don't run transactions. I don't think arguing from what they
do is particularly relevant to PostgresMain.

regards, tom lane

On 2014-02-23 14:48:12 -0500, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > Currently the error handling of normal backends only does a
> > LWLockReleaseAll() once CurrentTransactionState->state != TRANS_DEFAULT
> > because it's called in AbortTransaction(). There's pretty damn few
> > places that fiddle with lwlocks outside of a transaction command, but I
> > still do wonder whether it'd wouldn't be a tad more robust to
> > unconditionally do a LWLockReleaseAll(), just like other error handlers
> > are doing?
>
> Why do that thing in particular, and not all the other things that
> AbortTransaction() does?

Because the other things in AbortTransaction() should really only be
relevant inside a transaction, but there's valid reasons to use lwlocks
outside one.

E.g. I think that before Robert and I added a LWLockReleaseAll() to
WalSndErrorCleanup() the whole walsender code wasn't protected. I am not
entirely sure there's a real problem there in the backbranches, but it's
a fair amount of code, espcially around base backups...

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services