From: Anastasia Lubennikova <a.lubennikova@postgrespro.ru>
To: Andres Freund <andres@anarazel.de>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: pgsql-hackers@lists.postgresql.org, Jaime Casanova <jaime.casanova@2ndquadrant.com>
Subject: Re: LogwrtResult contended spinlock
Date: Tue, 24 Nov 2020 13:36:58 +0300
Message-ID: <45bc9909-8f6c-74d8-83ac-d86d35e147cc@postgrespro.ru> (raw)
In-Reply-To: <20200904171355.bldopurt65m2aqn4@alap3.anarazel.de>
References: <E494450B-9C6B-4716-9902-976DC7D69490@anarazel.de>
<20200903183452.GA10634@alvherre.pgsql>
<20200904170545.ko4bwixl3yuu5jxz@alap3.anarazel.de>
<20200904171355.bldopurt65m2aqn4@alap3.anarazel.de>
On 04.09.2020 20:13, Andres Freund wrote:
> Hi,
>
> On 2020-09-04 10:05:45 -0700, Andres Freund wrote:
>> On 2020-09-03 14:34:52 -0400, Alvaro Herrera wrote:
>>> Looking at patterns like this
>>>
>>> if (XLogCtl->LogwrtRqst.Write < EndPos)
>>> XLogCtl->LogwrtRqst.Write = EndPos;
>>>
>>> It seems possible to implement with
>>>
>>> do {
>>> XLogRecPtr currwrite;
>>>
>>> currwrite = pg_atomic_read_u64(LogwrtRqst.Write);
>>> if (currwrite > EndPos)
>>> break; // already done by somebody else
>>> if (pg_atomic_compare_exchange_u64(LogwrtRqst.Write,
>>> currwrite, EndPos))
>>> break; // successfully updated
>>> } while (true);
>>>
>>> This assumes that LogwrtRqst.Write never goes backwards, so it doesn't
>>> seem good material for a general routine.
>>>
>>> This *seems* correct to me, though this is muddy territory to me. Also,
>>> are there better ways to go about this?
>> Hm, I was thinking that we'd first go for reading it without a spinlock,
>> but continuing to write it as we currently do.
>>
>> But yea, I can't see an issue with what you propose here. I personally
>> find do {} while () weird and avoid it when not explicitly useful, but
>> that's extremely minor, obviously.
> Re general routine: On second thought, it might actually be worth having
> it. Even just for LSNs - there's plenty places where it's useful to
> ensure a variable is at least a certain size. I think I would be in
> favor of a general helper function.
Do you mean by general helper function something like this?
void
swap_lsn(XLogRecPtr old_value, XLogRecPtr new_value, bool to_largest)
{
while (true) {
XLogRecPtr currwrite;
currwrite = pg_atomic_read_u64(old_value);
if (to_largest)
if (currwrite > new_value)
break; /* already done by somebody else */
else
if (currwrite < new_value)
break; /* already done by somebody else */
if (pg_atomic_compare_exchange_u64(old_value,
currwrite, new_value))
break; /* already done by somebody else */
}
}
which will be called like
swap_lsn(XLogCtl->LogwrtRqst.Write, EndPos, true);
>
> Greetings,
>
> Andres Freund
This CF entry was inactive for a while. Alvaro, are you going to
continue working on it?
--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
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: pgsql-hackers@postgresql.org
Cc: a.lubennikova@postgrespro.ru, andres@anarazel.de, alvherre@2ndquadrant.com, jaime.casanova@2ndquadrant.com
Subject: Re: LogwrtResult contended spinlock
In-Reply-To: <45bc9909-8f6c-74d8-83ac-d86d35e147cc@postgrespro.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox