Re: [PATCH] Refactoring of LWLock tranches

On 11/06/2015 08:53 PM, Robert Haas wrote:
> On Fri, Nov 6, 2015 at 6:27 AM, Ildus Kurbangaliev
> <i.kurbangaliev@postgrespro.ru> wrote:
>> There is a patch that splits SLRU LWLocks to separate tranches and
>> moves them to SLRU Ctl. It does some work from the main patch from
>> this thread, but can be commited separately. It also simplifies
>> lwlock.c.
> Thanks.  I like the direction this is going.
>
> -               char       *ptr;
> -               Size            offset;
> -               int                     slotno;
> +               char            *ptr;
> +               Size             offset;
> +               int              slotno;
> +               int              tranche_id;
> +               LWLockPadded    *locks;
>
> Please don't introduce this kind of churn.  pgindent will undo it.
>
> This isn't going to work for EXEC_BACKEND builds, I think.  It seems
> to rely on the LWLockRegisterTranche() performed !IsUnderPostmaster
> being inherited by subsequent children, which won't work under
> EXEC_BACKEND.  Instead, store the tranche ID in SlruSharedData.  Move
> the LWLockRegisterTranche call out from the (!IsUnderPostmaster) case
> and call it based on the tranche ID from SlruSharedData.
>
> I would just drop the add_postfix stuff.  I think it's fine if the
> names of the shared memory checks are just "CLOG" etc. rather than
> "CLOG Slru Ctl", and similarly I think the locks can be registered
> without the "Locks" suffix.  It'll be clear from context that they are
> locks.  I suggest also that we just change all of these names to be
> lower case, though I realize that's a debatable and cosmetic point.
>
Thanks for the review. I've attached a new version of SLRU patch. I've
removed
add_postfix and fixed EXEC_BACKEND case.

--
Ildus Kurbangaliev
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company