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

Amit Kapila <amit.kapila16@gmail.com>

From: Amit Kapila <amit.kapila16@gmail.com>
To: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
Cc: Andres Freund <andres@anarazel.de>, Tom Lane <tgl@sss.pgh.pa.us>, Robert Haas <robertmhaas@gmail.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-03-09T06:50:45Z
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().

On Mon, Mar 9, 2020 at 11:38 AM Masahiko Sawada <
masahiko.sawada@2ndquadrant.com> wrote:

> On Mon, 9 Mar 2020 at 14:16, Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Sun, Mar 8, 2020 at 7:58 AM Masahiko Sawada <
> masahiko.sawada@2ndquadrant.com> wrote:
> >> >
> >> > 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).
> >>
> >> The current patch
> >> (v02_0001-Added-assert-to-verify-that-we-never-try-to-take-any.patch)
> >> doesn't check that acquiring a heavy-weight lock after page type lock,
> >> is that right?
> >
> >
> > No, it should do that.
> >
> >>
> >> There is the path doing that: ginInsertCleanup() holds
> >> a page lock and insert the pending list items, which might hold a
> >> relation extension lock.
> >
> >
> > Right, I could also see that, but do you see any problem with that?  I
> agree that Assert should cover this case, but I don't see any fundamental
> problem with that.
>
> I think that could be a problem if we change the group locking so that
> it doesn't consider page lock type.
>

I might be missing something, but won't that be a problem only when if
there is a case where we acquire page lock after acquiring a relation
extension lock?  Can you please explain the scenario you have in mind which
can create a problem?

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