Re: LogwrtResult contended spinlock

From: Anastasia Lubennikova <a(dot)lubennikova(at)postgrespro(dot)ru>
To: Andres Freund <andres(at)anarazel(dot)de>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org, Jaime Casanova <jaime(dot)casanova(at)2ndquadrant(dot)com>
Subject: Re: LogwrtResult contended spinlock
Date: 2020-11-24 10:36:58
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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?

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 */
      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:
The Russian Postgres Company

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2020-11-24 10:52:08 Re: scram-sha-256 broken with FIPS and OpenSSL 1.0.2
Previous Message Yugo NAGATA 2020-11-24 10:11:38 Re: Implementing Incremental View Maintenance