Re: [PATCH] LWLock self-deadlock detection

2021-01-04 Thread Craig Ringer
On Wed, 30 Dec 2020 at 10:11, Andres Freund wrote: > Hi, > > On 2020-11-27 20:22:41 +0200, Heikki Linnakangas wrote: > > On 26/11/2020 04:50, Tom Lane wrote: > > > Craig Ringer writes: > > > > On Wed, Nov 25, 2020 at 9:23 PM Ashutosh Bapat < > ashutosh.bapat@gmail.com> > > > > wrote: > > > >

Re: [PATCH] LWLock self-deadlock detection

2020-12-29 Thread Andres Freund
Hi, On 2020-11-27 20:22:41 +0200, Heikki Linnakangas wrote: > On 26/11/2020 04:50, Tom Lane wrote: > > Craig Ringer writes: > > > On Wed, Nov 25, 2020 at 9:23 PM Ashutosh Bapat > > > > > > wrote: > > > > I'd prefer to make the lock self deadlock check run for production > > > > builds, not just

Re: [PATCH] LWLock self-deadlock detection

2020-11-28 Thread Michael Paquier
On Fri, Nov 27, 2020 at 11:08:49AM -0800, Peter Geoghegan wrote: > On Fri, Nov 27, 2020 at 10:22 AM Heikki Linnakangas wrote: >> I've made the mistake of forgetting to release an LWLock many times, >> leading to self-deadlock. And I just reviewed a patch that did that this >> week [1]. You usually

Re: [PATCH] LWLock self-deadlock detection

2020-11-27 Thread Peter Geoghegan
On Fri, Nov 27, 2020 at 10:22 AM Heikki Linnakangas wrote: > I've made the mistake of forgetting to release an LWLock many times, > leading to self-deadlock. And I just reviewed a patch that did that this > week [1]. You usually find that mistake very quickly when you start > testing though, I don

Re: [PATCH] LWLock self-deadlock detection

2020-11-27 Thread Heikki Linnakangas
On 26/11/2020 04:50, Tom Lane wrote: Craig Ringer writes: On Wed, Nov 25, 2020 at 9:23 PM Ashutosh Bapat wrote: I'd prefer to make the lock self deadlock check run for production builds, not just cassert builds. I'd like to register a strong objection to spending any cycles whatsoever on th

Re: [PATCH] LWLock self-deadlock detection

2020-11-25 Thread Tom Lane
Craig Ringer writes: > On Wed, Nov 25, 2020 at 9:23 PM Ashutosh Bapat > wrote: >> I'd prefer to make the lock self deadlock check run for production >> builds, not just cassert builds. I'd like to register a strong objection to spending any cycles whatsoever on this. If you're using LWLocks in

Re: [PATCH] LWLock self-deadlock detection

2020-11-25 Thread Craig Ringer
On Wed, Nov 25, 2020 at 9:23 PM Ashutosh Bapat wrote: > On Wed, Nov 25, 2020 at 11:47 AM Craig Ringer > wrote: > >> I am also seeing a pattern > >> Assert(!LWLockHeldByMe()); > >> LWLockAcquire() > >> > >> at some places. Should we change LWLockAcquire to do > >> Assert(!LWLockHeldByMe()) always

Re: [PATCH] LWLock self-deadlock detection

2020-11-25 Thread Ashutosh Bapat
On Wed, Nov 25, 2020 at 11:47 AM Craig Ringer wrote: >> I am also seeing a pattern >> Assert(!LWLockHeldByMe()); >> LWLockAcquire() >> >> at some places. Should we change LWLockAcquire to do >> Assert(!LWLockHeldByMe()) always to detect such occurrences? > > > I'm inclined not to, at least not wit

Re: [PATCH] LWLock self-deadlock detection

2020-11-24 Thread Craig Ringer
On Tue, Nov 24, 2020 at 10:11 PM Ashutosh Bapat < ashutosh.bapat@gmail.com> wrote: > This looks useful. LWLockCheckSelfDeadlock() could use LWLockHeldByMe > variant instead of copying that code with possibly a change in that > function to return the required information. > Yes, possibly so. I

Re: [PATCH] LWLock self-deadlock detection

2020-11-24 Thread Ashutosh Bapat
This looks useful. LWLockCheckSelfDeadlock() could use LWLockHeldByMe variant instead of copying that code with possibly a change in that function to return the required information. I am also seeing a pattern Assert(LWLockHeldByMe*()) LWLockAcquire() at some places. Should we change LWLockAcquir