Thread: 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@2ndquadrant.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@2ndquadrant.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