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: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Robert Haas <robertmhaas@gmail.com>, Andres Freund <andres@anarazel.de>, 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-13T03:03:42Z
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 Wed, Feb 12, 2020 at 10:23 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Amit Kapila <amit.kapila16@gmail.com> writes:
> > On Wed, Feb 12, 2020 at 7:36 AM Masahiko Sawada
> > <masahiko.sawada@2ndquadrant.com> wrote:
> >> On Wed, 12 Feb 2020 at 00:43, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >>> I would like to suggest that we do something similar to Robert Haas'
> >>> excellent hack (daa7527af) for the !HAVE_SPINLOCK case in lmgr/spin.c,
>
> >> My original proposal used LWLocks and hash tables for relation
> >> extension but there was a discussion that using LWLocks is not good
> >> because it's not interruptible[1].
>
> > Hmm, but we use LWLocks for (a) WALWrite/Flush (see the usage of
> > WALWriteLock), (b) writing the shared buffer contents (see
> > io_in_progress lock and its usage in FlushBuffer) and might be for few
> > other similar stuff.  Many times those take more time than extending a
> > block in relation especially when we combine the WAL write for
> > multiple commits.  So, if this is a problem for relation extension
> > lock, then the same thing holds true there also.
>
> Yeah.  I would say a couple more things:
>
> * I see no reason to think that a relation extension lock would ever
> be held long enough for noninterruptibility to be a real issue.  Our
> expectations for query cancel response time are in the tens to
> hundreds of msec anyway.
>
> * There are other places where an LWLock can be held for a *long* time,
> notably the CheckpointLock.  If we do think this is an issue, we could
> devise a way to not insist on noninterruptibility.  The easiest fix
> is just to do a matching RESUME_INTERRUPTS after getting the lock and
> HOLD_INTERRUPTS again before releasing it; though maybe it'd be worth
> offering some slightly cleaner way.
>

Yeah, this sounds like the better answer for noninterruptibility
aspect of this design.  One idea that occurred to me was to pass a
parameter to LWLOCK acquire/release APIs to indicate whether to
hold/resume interrupts, but I don't know if that is any better than
doing it at the required place.  I am not sure if all places are
careful whether they really want to hold interrupts, so if we provide
a new parameter at least new users of API will think about it.

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