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

On Mon, Mar 16, 2020 at 11:56 AM Kuntal Ghosh
<kuntalghosh(dot)2007(at)gmail(dot)com> wrote:
>
> On Mon, Mar 16, 2020 at 9:43 AM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > On Mon, Mar 16, 2020 at 8:57 AM Masahiko Sawada
> > <masahiko(dot)sawada(at)2ndquadrant(dot)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