public inbox for [email protected]
help / color / mirror / Atom feedFrom: Nathan Bossart <[email protected]>
To: John Morris <[email protected]>
Cc: Bharath Rupireddy <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Stephen Frost <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: Atomic ops for unlogged LSN
Date: Mon, 6 Nov 2023 21:35:58 -0600
Message-ID: <20231107033558.GC729644@nathanxps13> (raw)
In-Reply-To: <BYAPR13MB26777BA0E688ECE294BDE4A6A0AAA@BYAPR13MB2677.namprd13.prod.outlook.com>
References: <MN2PR13MB2688FD8B757316CB5C54C8A2A0DDA@MN2PR13MB2688.namprd13.prod.outlook.com>
<20231026203433.GA1088329@nathanxps13>
<BYAPR13MB26779988F4C73DD395401C59A0DCA@BYAPR13MB2677.namprd13.prod.outlook.com>
<BYAPR13MB267762560386F4FB5BFDA235A0A7A@BYAPR13MB2677.namprd13.prod.outlook.com>
<20231102034006.GA85609@nathanxps13>
<20231106203350.GA598180@nathanxps13>
<BYAPR13MB26777BA0E688ECE294BDE4A6A0AAA@BYAPR13MB2677.namprd13.prod.outlook.com>
On Tue, Nov 07, 2023 at 12:57:32AM +0000, John Morris wrote:
> I incorporated your suggestions and added a few more. The changes are
> mainly related to catching potential errors if some basic assumptions
> aren’t met.
Hm. Could we move that to a separate patch? We've lived without these
extra checks for a very long time, and I'm not aware of any related issues,
so I'm not sure it's worth the added complexity. And IMO it'd be better to
keep it separate from the initial atomics conversion, anyway.
> I found the comment about cache coherency a bit confusing. We are dealing
> with a single address, so there should be no memory ordering or coherency
> issues. (Did I misunderstand?) I see it more as a race condition. Rather
> than merely explaining why it shouldn’t happen, the new version verifies
> the assumptions and throws an Assert() if something goes wrong.
I was thinking of the comment for pg_atomic_read_u32() that I cited earlier
[0]. This comment also notes that pg_atomic_read_u32/64() has no barrier
semantics. My interpretation of that comment is that these functions
provide no guarantee that the value returned is the most up-to-date value.
But my interpretation could be wrong, and maybe this is meant to highlight
that the value might change before we can use the return value in a
compare/exchange or something.
I spent a little time earlier today reviewing the various underlying
implementations, but apparently I need to spend some more time looking at
those...
[0] https://postgr.es/m/20231102034006.GA85609%40nathanxps13
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
view thread (9+ 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], [email protected]
Subject: Re: Atomic ops for unlogged LSN
In-Reply-To: <20231107033558.GC729644@nathanxps13>
* 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