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

On Wed, Feb 12, 2020 at 10:23 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> writes:
> > On Wed, Feb 12, 2020 at 7:36 AM Masahiko Sawada
> > <masahiko(dot)sawada(at)2ndquadrant(dot)com> wrote:
> >> On Wed, 12 Feb 2020 at 00:43, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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