Horton

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

Dilip Kumar <dilipbalaut@gmail.com>

View thread
From: Dilip Kumar <dilipbalaut@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, 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>, Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
Date: 2020-02-13T04:52:33Z
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 Thu, Feb 13, 2020 at 9:46 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Tue, Feb 11, 2020 at 9:13 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >
> > I took a brief look through this patch.  I agree with the fundamental
> > idea that we shouldn't need to use the heavyweight lock manager for
> > relation extension, since deadlock is not a concern and no backend
> > should ever need to hold more than one such lock at once.  But it feels
> > to me like this particular solution is rather seriously overengineered.
> > 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,
> > that is,
> >
> > * Create some predetermined number N of LWLocks for relation extension.
> > * When we want to extend some relation R, choose one of those locks
> >   (say, R's relfilenode number mod N) and lock it.
> >
>
> I am imagining something on the lines of BufferIOLWLockArray (here it
> will be RelExtLWLockArray).  The size (N) could MaxBackends or some
> percentage of it (depending on testing) and indexing into an array
> could be as suggested (R's relfilenode number mod N).  We need to
> initialize this during shared memory initialization.  Then, to extend
> the relation with multiple blocks at-a-time (as we do in
> RelationAddExtraBlocks), we can either use the already proven
> technique of group clear xid mechanism (see ProcArrayGroupClearXid) or
> have an additional state in the RelExtLWLockArray which will keep the
> count of waiters (as done in latest patch of Sawada-san [1]).  We
> might want to experiment with both approaches and see which yields
> better results.

IMHO, in this case, there is no point in using the "group clear" type
of mechanism mainly for two reasons 1) It will unnecessarily make
PGPROC structure heavy.
2) For our case, we don't need any specific pieces of information from
other waiters, we just need the count.


Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com