Re: .ready and .done files considered harmful

From: "Bossart, Nathan" <bossartn(at)amazon(dot)com>
To: Dipesh Pandit <dipesh(dot)pandit(at)gmail(dot)com>
Cc: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Jeevan Ladhe <jeevan(dot)ladhe(at)enterprisedb(dot)com>, Stephen Frost <sfrost(at)snowman(dot)net>, Andres Freund <andres(at)anarazel(dot)de>, Hannu Krosing <hannuk(at)google(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: .ready and .done files considered harmful
Date: 2021-08-17 16:33:54
Message-ID: 273A6E6D-DCC1-4719-96D2-609D5ADA3EA7@amazon.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 8/17/21, 5:53 AM, "Dipesh Pandit" <dipesh(dot)pandit(at)gmail(dot)com> wrote:
>> I personally don't think it's necessary to use an atomic here. A
>> spinlock or LWLock would probably work just fine, as contention seems
>> unlikely. If we use a lock, we also don't have to worry about memory
>> barriers.
>
> History file should be archived as soon as it gets created. The atomic flag
> here will make sure that there is no reordering of read/write instructions while
> accessing the flag in shared memory. Archiver needs to read this flag at the
> beginning of each cycle. Write to atomic flag is synchronized and it provides
> a lockless read. I think an atomic flag here is an efficient choice unless I am
> missing something.

Sorry, I think my note was not very clear. I agree that a flag should
be used for this purpose, but I think we should just use a regular
bool protected by a spinlock or LWLock instead of an atomic. The file
atomics.h has the following note:

* Use higher level functionality (lwlocks, spinlocks, heavyweight locks)
* whenever possible. Writing correct code using these facilities is hard.

IOW I don't think the extra complexity is necessary. From a
performance standpoint, contention seems unlikely. We only need to
read the flag roughly once per WAL segment, and we only ever set it in
uncommon scenarios such as a timeline switch or the creation of an
out-of-order .ready file.

Nathan

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Julien Rouhaud 2021-08-17 16:34:45 Re: PG14: Avoid checking output-buffer-length for every encoded byte during pg_hex_encode
Previous Message Stephen Frost 2021-08-17 16:33:17 Re: preserving db/ts/relfilenode OIDs across pg_upgrade (was Re: storing an explicit nonce)