Re: [PATCH] LWLock self-deadlock detection

On 26/11/2020 04:50, Tom Lane wrote:
> Craig Ringer <craig.ringer@enterprisedb.com> writes:
>> On Wed, Nov 25, 2020 at 9:23 PM Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
>> 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 a way that could deadlock, you're
> doing it wrong.  The entire point of that mechanism is to be Light Weight
> and not have all the overhead that the heavyweight lock mechanism has.
> Building in deadlock checks and such is exactly the wrong direction.
> If you think you need that, you should be using a heavyweight lock.
> 
> Maybe there's some case for a cassert-only check of this sort, but running
> it in production is right out.
> 
> I'd also opine that checking for self-deadlock, but not any more general
> deadlock, seems pretty pointless.  Careless coding is far more likely
> to create opportunities for the latter.  (Thus, I have little use for
> the existing assertions of this sort, either.)

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't think I've seen it happen in production.

So yeah, I agree it's not worth spending cycles on this. Maybe it would 
be worth it if it's really simple to check, and you only do it after 
waiting X seconds. (I haven't looked at this patch at all)

[1] 
https://www.postgresql.org/message-id/8e1cda9b-1cb9-a634-d383-f757bf67b820@iki.fi

- Heikki