Horton

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

Andres Freund <andres@anarazel.de>

View thread
From: Andres Freund <andres@anarazel.de>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Amit Kapila <amit.kapila16@gmail.com>, Masahiko Sawada <masahiko.sawada@2ndquadrant.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Michael Paquier <michael@paquier.xyz>, Mithun Cy <mithun.cy@enterprisedb.com>, Thomas Munro <thomas.munro@enterprisedb.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2020-02-16T21:05:31Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Allow page lock to conflict among parallel group members.

  2. Allow relation extension lock to conflict among parallel group members.

  3. Add assert to ensure that page locks don't participate in deadlock cycle.

  4. Assert that we don't acquire a heavyweight lock on another object after

  5. Fix unsafe usage of strerror(errno) within ereport().

Hi,

On 2020-02-14 13:34:03 -0500, Robert Haas wrote:
> On Fri, Feb 14, 2020 at 1:07 PM Andres Freund <andres@anarazel.de> wrote:
> > Yea, that seems possible.  I'm not really sure it's needed however? As
> > long as you're not teaching the locking mechanism new tricks that
> > influence the wait graph, why would the deadlock detector care? That's
> > quite different from the group locking case, where you explicitly needed
> > to teach it something fairly fundamental.
> 
> Well, you have to teach it that locks of certain types conflict even
> if they are in the same group, and that bleeds over pretty quickly
> into the whole area of deadlock detection, because lock waits are the
> edges in the graph that the deadlock detector processes.

Shouldn't this *theretically* be doable with changes mostly localized to
lock.c, by not using proc->lockGroupLeader but proc for lock types that
don't support group locking? I do see that deadlock.c largely looks at
->lockGroupLeader, but that kind of doesn't seem right to me.


> > It might still be a good idea independently to add the rule & enforce
> > that acquire heavyweight locks while holding certain classes of locks is
> > not allowed.
> 
> I think that's absolutely essential, if we're going to continue using
> the main lock manager for this. I remain somewhat unconvinced that
> doing so is the best way forward, but it is *a* way forward.

Seems like we should build this part independently of the lock.c/new
infra piece.


> > Right. For me that's *the* fundamental service that lock.c delivers. And
> > it's the fundamental bit this thread so far largely has been focusing
> > on.
> 
> For me, the deadlock detection is the far more complicated and problematic bit.
> 
> > Isn't that mostly true to varying degrees for the majority of lock types
> > in lock.c? Sure, perhaps historically that's a misuse of lock.c, but
> > it's been pretty ingrained by now.  I just don't see where leaving out
> > any of these features is going to give us fundamental advantages
> > justifying a different locking infrastructure.
> 
> I think the group locking + deadlock detection things are more
> fundamental than you might be crediting, but I agree that having
> parallel mechanisms has its own set of pitfalls.

It's possible. But I'm also hesitant to believe that we'll not need
other lock types that conflict between leader/worker, but that still
need deadlock detection.  The more work we want to parallelize, the more
likely that imo will become.

Greetings,

Andres Freund