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: Kuntal Ghosh <kuntalghosh.2007@gmail.com>
Cc: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>, Amit Kapila <amit.kapila16@gmail.com>, Robert Haas <robertmhaas@gmail.com>, Mahendra Singh Thalor <mahi6run@gmail.com>, Andres Freund <andres@anarazel.de>, Tom Lane <tgl@sss.pgh.pa.us>, 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-16T09:54:10Z
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().

Attachments

On Mon, Mar 16, 2020 at 11:56 AM Kuntal Ghosh
<kuntalghosh.2007@gmail.com> wrote:
>
> On Mon, Mar 16, 2020 at 9:43 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > On Mon, Mar 16, 2020 at 8:57 AM Masahiko Sawada
> > <masahiko.sawada@2ndquadrant.com> wrote:
> > > IsRelationExtensionLockHeld and IsPageLockHeld are used only when
> > > assertion is enabled. So how about making CheckAndSetLockHeld work
> > > only if USE_ASSERT_CHECKING to avoid overheads?
> >
> > That makes sense to me so updated the patch.
> +1
>
> In v10-0001-Assert-that-we-don-t-acquire-a-heavyweight-lock-.patch,
>
> + * Indicate that the lock is released for a particular type of locks.
> s/lock is/locks are

Done

> + /* Indicate that the lock is acquired for a certain type of locks. */
> s/lock is/locks are

Done

>
> In v10-0002-*.patch,
>
> + * Flag to indicate if the page lock is held by this backend.  We don't
> + * acquire any other heavyweight lock while holding the page lock except for
> + * relation extension.  However, these locks are never taken in reverse order
> + * which implies that page locks will also never participate in the deadlock
> + * cycle.
> s/while holding the page lock except for relation extension/while
> holding the page lock except for relation extension and page lock

Done

> + * We don't acquire any other heavyweight lock while holding the page lock
> + * except for relation extension lock.
> Same as above

Done

>
> Other than that, the patches look good to me. I've also done some
> testing after applying the Test-group-deadlock patch provided by Amit
> earlier in the thread. It works as expected.

Thanks for testing.

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