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: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>, 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-13T03:07:22Z
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, Mar 12, 2020 at 5:28 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Thu, Mar 12, 2020 at 11:15 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > I have fixed this in the attached patch set.
> >
>
> I have modified your
> v4-0003-Conflict-Extension-Page-lock-in-group-member patch.  The
> modifications are (a) Change src/backend/storage/lmgr/README to
> reflect new behaviour, (b) Introduce a new macro LOCK_LOCKTAG which
> slightly simplifies the code, (c) moved the deadlock.c check a few
> lines up and (d) changed a few comments.

Changes look fine to me.

> It might be better if we can move the checks related to extension and
> page lock in a separate API or macro.  What do you think?

I feel it looks cleaner this way as well.  But, If we plan to move it
to common function/macro then we should use some common name such that
it can be used in FindLockCycleRecurseMember as well as in
LockCheckConflicts.

> I have also used an extension to test this patch.  This is the same
> extension that I have used to test the group locking patch.  It will
> allow backends to form a group as we do for parallel workers.  The
> extension is attached to this email.
>
> Test without patch:
> Session-1
> Create table t1(c1 int, c2 char(500));
> Select become_lock_group_leader();
>
> Insert into t1 values(generate_series(1,100),'aaa'); -- stop this
> after acquiring relation extension lock via GDB.
>
> Session-2
> Select  become_lock_group_member();
> Insert into t1 values(generate_series(101,200),'aaa');
> - Debug LockAcquire and found that it doesn't generate conflict for
> Relation Extension lock.
>
> The above experiment has shown that without patch group members can
> acquire relation extension lock if the group leader has that lock.
> After patch the second session waits for the first session to release
> the relation extension lock. I know this is not a perfect way to test,
> but it is better than nothing.  I think we need to do some more
> testing either using this extension or some other way for extension
> and page locks.

I have also tested the same and verified it.

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