Re: [PATCH] Refactoring of LWLock tranches

From: Ildus Kurbangaliev <i(dot)kurbangaliev(at)postgrespro(dot)ru>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, "andres(at)anarazel(dot)de" <andres(at)anarazel(dot)de>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
Subject: Re: [PATCH] Refactoring of LWLock tranches
Date: 2015-11-09 09:17:34
Message-ID: 564064AE.70409@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
slru_tranches_v2.patch text/x-patch 7.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Simon Riggs 2015-11-09 09:21:11 Re: eXtensible Transaction Manager API
Previous Message Amit Kapila 2015-11-09 09:06:51 Re: pgsql: Modify tqueue infrastructure to support transient record types.