public inbox for [email protected]
help / color / mirror / Atom feedFrom: Tomas Vondra <[email protected]>
To: Alexander Korotkov <[email protected]>
Cc: Andrey Borodin <[email protected]>
Cc: Anastasia Lubennikova <[email protected]>
Cc: Daniel Gustafsson <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: MultiXact\SLRU buffers configuration
Date: Thu, 29 Oct 2020 00:21:01 +0100
Message-ID: <20201028232101.4awgmc2cu2cemu7p@development> (raw)
In-Reply-To: <CAPpHfdsC7-ZkaKD9xYoVejdD5OeGc3jr7du3-84nkHuNGuKnKQ@mail.gmail.com>
References: <[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<CAPpHfduekT=BrM3rLqvo4ajzKYmDk+6aCoYJrNHhK0Je+v2z=Q@mail.gmail.com>
<[email protected]>
<CAPpHfduT0Kb+8fMt4t0EF5G_3hig+wYgeRadQnNq20F4VxP7LA@mail.gmail.com>
<CAPpHfdsUtSGxRrKn65F_wrE66sNztNY4TpDGDHh2kvAZZDOMqA@mail.gmail.com>
<20201028013651.de5cj2xadgmba5nf@development>
<CAPpHfdsC7-ZkaKD9xYoVejdD5OeGc3jr7du3-84nkHuNGuKnKQ@mail.gmail.com>
On Wed, Oct 28, 2020 at 10:36:39PM +0300, Alexander Korotkov wrote:
>Hi, Tomas!
>
>Thank you for your review.
>
>On Wed, Oct 28, 2020 at 4:36 AM Tomas Vondra
><[email protected]> wrote:
>> I did a quick review on this patch series. A couple comments:
>>
>>
>> 0001
>> ----
>>
>> This looks quite suspicious to me - SimpleLruReadPage_ReadOnly is
>> changed to return information about what lock was used, merely to allow
>> the callers to do an Assert() that the value is not LW_NONE.
>
>Yes, but this is not merely to allow callers to do an Assert().
>Sometimes in multixacts it could save us some relocks. So, we can
>skip relocking lock to exclusive mode if it's in exclusive already.
>Adding Assert() to every caller is probably overkill.
>
Hmm, OK. That can only happen in GetMultiXactIdMembers, which is the
only place where we do retry, right? Do we actually know this makes any
measurable difference? It seems we're mostly imagining that it might
help, but we don't have any actual proof of that (e.g. a workload which
we might benchmark). Or am I missing something?
For me, the extra conditions make it way harder to reason about the
behavior of the code, and I can't convince myself it's worth it.
>> IMO we could achieve exactly the same thing by passing a simple flag
>> that would say 'make sure we got a lock' or something like that. In
>> fact, aren't all callers doing the assert? That'd mean we can just do
>> the check always, without the flag. (I see GetMultiXactIdMembers does
>> two calls and only checks the second result, but I wonder if that's
>> intended or omission.)
>
>Having just the flag is exactly what the original version by Andrey
>did. But if we have to read two multixact offsets pages or multiple
>members page in one GetMultiXactIdMembers()), then it does relocks
>from exclusive mode to exclusive mode. I decide that once we decide
>to optimize this locks, this situation is nice to evade.
>
>> In any case, it'd make the lwlock.c changes unnecessary, I think.
>
>I agree that it would be better to not touch lwlock.c. But I didn't
>find a way to evade relocking exclusive mode to exclusive mode without
>touching lwlock.c or making code cumbersome in other places.
>
Hmm. OK.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
view thread (81+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: MultiXact\SLRU buffers configuration
In-Reply-To: <20201028232101.4awgmc2cu2cemu7p@development>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox