Re: [HACKERS] Moving relation extension locks out of heavyweight lock manager

On Thu, Feb 20, 2020 at 8:06 AM Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
> On 2020-02-19 11:12:18 +0530, Amit Kapila wrote:
> > I think till we know the real need for changing group locking, going
> > in the direction of what Tom suggested to use an array of LWLocks [1]
> > to address the problems in hand is a good idea.
>
> -many
>
> I think that building yet another locking subsystem is the entirely
> wrong idea - especially when there's imo no convincing architectural
> reasons to do so.
>

Hmm, AFAIU, it will be done by having an array of LWLocks which we do
at other places as well (like BufferIO locks).  I am not sure if we
can call it as new locking subsystem, but if we decide to continue
using lock.c and change group locking then I think we can do that as
well, see my comments below regarding that.

>
> > It is not very clear to me that are we thinking to give up on Tom's
> > idea [1] and change group locking even though it is not clear or at
> > least nobody has proposed an idea/patch which requires that?  Or are
> > we thinking that we can do what Tom suggested for relation extension
> > lock and also plan to change group locking for future parallel
> > operations that might require it?
>
> What I'm advocating is that extension locks should continue to go
> through lock.c. And yes, that requires some changes to group locking,
> but I still don't see why they'd be complicated.
>

Fair position, as per initial analysis, I think if we do below three
things, it should work out without changing to a new way of locking
for relation extension or page type locks.
a. As per the discussion above, ensure in code we will never try to
acquire another heavy-weight lock after acquiring relation extension
or page type locks (probably by having Asserts in code or maybe some
other way).
b. Change lock.c so that group locking is not considered for these two
lock types. For ex. in LockCheckConflicts, along with the check (if
(proclock->groupLeader == MyProc && MyProc->lockGroupLeader == NULL)),
we also check lock->tag and call it a conflict for these two locks.
c. The deadlock detector can ignore checking these two types of locks
because point (a) ensures that those won't lead to deadlock.  One idea
could be that FindLockCycleRecurseMember just ignores these two types
of locks by checking the lock tag.

It is possible that I might be missing something or we could achieve
this some other way as well.

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com