Feedback-ID: i0fe9450f:Fastmail
Date: Thu, 25 May 2023 07:41:21 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Andres Freund <andres@anarazel.de>
Cc: Robert Haas <robertmhaas@gmail.com>,
	John Morris <john.morris@crunchydata.com>,
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>,
	Stephen Frost <sfrost@snowman.net>
Subject: Re: Atomic ops for unlogged LSN
Message-ID: <ZG6SkQwO2obI95js@paquier.xyz>
References: <BYAPR13MB26772534335255E50318C574A0409@BYAPR13MB2677.namprd13.prod.outlook.com>
 <ZG09kSRxOdvz5TVv@paquier.xyz>
 <CA+TgmoaVokb-_VkSL=0k0XCj5ow0ew8M1+e9=7JKvgtDqibzpA@mail.gmail.com>
 <20230524214958.mt6f5xokpumvnrio@awork3.anarazel.de>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="Yi95FZybArnsUoXx"
Content-Disposition: inline
In-Reply-To: <20230524214958.mt6f5xokpumvnrio@awork3.anarazel.de>
Archived-At: <https://www.postgresql.org/message-id/ZG6SkQwO2obI95js%40paquier.xyz>
Precedence: bulk


--Yi95FZybArnsUoXx
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

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

--Yi95FZybArnsUoXx
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmRukpEACgkQnvQgOdby
QH1wNA//fp5PdWNvmXQjBt1hYpz6dAZ9cABrPo0rODOCA6339sYUUJYKgePYCf/w
sy/baIduTEazXwkwQBI18OUIlO2hbf1405gTpvcThqTdFgiL1KKZYAwF3/zNJcy5
l6cafHvlqGe+YEg8BnrtwbeDfpCYEnLm29U5ehqczSsB3qT1/4cGbvGDtv0qUmr2
7eT1mbAkQ4kzzH8E+GmCpOU6PmAD1gKbnIcoxpVMPjaawEDPJsAoKn1pqExKZuWe
Ag6a4MHkpiFnY28Riyc5I8wUJRinKkKZySyZOElHqaLXhTBp/tUUx6FvwSN/ObZz
OCuCm3QHeWD5zG/M2MVePmNQp3eX5LHqUceidsK4891e6wNx7a50N0KeuysGd04m
yq8O9cprz9vfGIhxi7rfN04ZdPKpLoABXAktg/DSMyDSbrVZCNyEAj2N57bzch8D
zsoJIdyt8maHiwC6IV03K1fHWay4/m9fEeq2/MV+EotYijEGk+JEOn+jOGjDUP+7
6L/TUbSHEB/UJdWGr1dCx4fjVx0J3t0F1SBhxikwQaQ34xP+V+w4NuLFvrRX6dfS
8diDDRMqYq48XKlNR2dEtEMLaVnA12wctRukn9SQufIc3gXO04QfyiXYyP4ee2ug
nyWs59oL5ue9rrzyZlderFtyB7KfY7metLK/lL2TraBjROIXFV4=
=2Sxe
-----END PGP SIGNATURE-----

--Yi95FZybArnsUoXx--