public inbox for [email protected]  

help / color / mirror / Atom feed

From: Michael Paquier <[email protected]>
To: Andres Freund <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: John Morris <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Stephen Frost <[email protected]>
Subject: Re: Atomic ops for unlogged LSN
Date: Thu, 25 May 2023 07:41:21 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <BYAPR13MB26772534335255E50318C574A0409@BYAPR13MB2677.namprd13.prod.outlook.com>
	<[email protected]>
	<CA+TgmoaVokb-_VkSL=0k0XCj5ow0ew8M1+e9=7JKvgtDqibzpA@mail.gmail.com>
	<[email protected]>

On Wed, May 24, 2023 at 02:49:58PM -0700, Andres Freund wrote:
> I was a bit confused by Michael's comment as well, due to the section of code
> quoted. But he does have a point: pg_atomic_read_u32() does indeed *not* have
> barrier semantics (it'd be way more expensive), and the patch does contain
> this hunk:

Thanks for the correction.  The part about _add was incorrect.

> So we indeed loose some "barrier strength" - but I think that's fine. For one,
> it's a debugging-only value. But more importantly, I don't see what reordering
> the barrier could prevent - a barrier is useful for things like sequencing two
> memory accesses to happen in the intended order - but unloggedLSN doesn't
> interact with another variable that's accessed within the ControlFileLock
> afaict.

This stuff is usually tricky enough that I am never completely sure
whether it is fine without reading again README.barrier, which is
where unloggedLSN is saved in the control file under its LWLock.
Something that I find confusing in the patch is that it does not
document the reason why this is OK.
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../[email protected]/2-signature.asc)
  download

view thread (5+ 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]
  Subject: Re: Atomic ops for unlogged LSN
  In-Reply-To: <[email protected]>

* 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