On 11/06/2015 08:53 PM, Robert Haas wrote:
> On Fri, Nov 6, 2015 at 6:27 AM, Ildus Kurbangaliev
> <i(dot)kurbangaliev(at)postgrespro(dot)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