Feedback-ID: i0fe9450f:Fastmail
Date: Wed, 18 Oct 2023 09:35:17 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Andres Freund <andres@anarazel.de>, Dilip Kumar <dilipbalaut@gmail.com>,
	Heikki Linnakangas <hlinnaka@iki.fi>,
	pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: New WAL record to detect the checkpoint redo location
Message-ID: <ZS8oRWqglxVqJqdw@paquier.xyz>
References: <20230714151626.rhgae7taigk2xrq7@awork3.anarazel.de>
 <CA+TgmoYy-Vc6G9QKcAKNksCa29cv__czr+N9X_QCxEfQVpp_8w@mail.gmail.com>
 <CA+TgmoZ_C1DKuNT0w13_b_6J8JgSupHFSLBCKgQdhJf3FannbA@mail.gmail.com>
 <CA+TgmoZAM24Ub=uxP0aWuWstNYTUJQ64j976FYJeVaMJ+qD0uw@mail.gmail.com>
 <20231005183400.n5myso7vu6crd656@alap3.anarazel.de>
 <CA+Tgmob90XtBJ+WdSHA+y8ZikHPSxQvZwYDefEBLC7Bkwx=3-g@mail.gmail.com>
 <20231009204702.cy3ilp6wgh7a5qms@awork3.anarazel.de>
 <CA+TgmoaaMge7NNBE45kSWz9HkgbSoe9mwicOuOOxLAkb6i2JdQ@mail.gmail.com>
 <ZSjxyCP9JaE4dJUA@paquier.xyz>
 <CA+Tgmoar8fD7tocgauu6xtFF5HEwK-dBqoTKEGeDBA8Prv=fkQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="5IP3FdTzgaNtzLjg"
Content-Disposition: inline
In-Reply-To: <CA+Tgmoar8fD7tocgauu6xtFF5HEwK-dBqoTKEGeDBA8Prv=fkQ@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/ZS8oRWqglxVqJqdw%40paquier.xyz>
Precedence: bulk


--5IP3FdTzgaNtzLjg
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, Oct 17, 2023 at 12:45:52PM -0400, Robert Haas wrote:
> On Fri, Oct 13, 2023 at 3:29=E2=80=AFAM Michael Paquier <michael@paquier.=
xyz> wrote:
>> I've mentioned as well a test in pg_walinspect after one of the
>> checkpoints generated there, but what you do here is enough for the
>> online case.
>=20
> I don't quite understand what you're saying here. If you're suggesting
> a potential improvement, can you be a bit more clear and explicit
> about what the suggestion is?

Suggestion is from here, with a test for pg_walinspect after it runs
its online checkpoint (see the full-page case):
https://www.postgresql.org/message-id/ZOvf1tu6rfL/B2PW@paquier.xyz

+-- Check presence of REDO record.
+SELECT redo_lsn FROM pg_control_checkpoint() \gset
+SELECT start_lsn =3D :'redo_lsn'::pg_lsn AS same_lsn, record_type
+  FROM pg_get_wal_record_info(:'redo_lsn');

>> Then repeat this pattern for non-shutdown checkpoints a few lines down
>> without touching the copy of the redo LSN in XLogCtl->Insert, because
>> of course we don't hold the WAL insert locks in an exclusive fashion
>> here:
>> checkPoint.redo =3D RedoRecPtr;
>>
>> My point is that this is not only about RedoRecPtr, but also about
>> XLogCtl->Insert.RedoRecPtr here.  The comment in ReserveXLogSwitch()
>> says that.
>=20
> I have adjusted the comment in CreateCheckPoint to hopefully address
> this concern.

-        * XLogInsertRecord will have updated RedoRecPtr, but we need to co=
py
-        * that into the record that will be inserted when the checkpoint is
-        * complete.
+        * XLogInsertRecord will have updated XLogCtl->Insert.RedoRecPtr in
+        * shared memory and RedoRecPtr in backend-local memory, but we need
+        * to copy that into the record that will be inserted when the
+        * checkpoint is complete.=20

This comment diff between v8 and v9 looks OK to me.  Thanks.

> I don't understand what you mean about
> ReserveXLogSwitch(), though.

I am not sure either, looking back at that :p
--
Michael

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmUvKEUACgkQnvQgOdby
QH2geg//U71YHOn2V9uizYEE69SqqKBnivSrgNswAenN/XrcMfpADtDEfE7U+uZH
lmJo1Gg+qIUPt4Qpl7/2dObCrWa4pS/O/S791UlUoY4vUsrqPRQgqa6m8XYP5TQR
jOP+F04T4Wf4o+ACz244sCvXId93/S5z5S2xQF46w7PuZzChNCmgK9hwud171+Kb
NR6hpwxuDy6Fu+YG7yAUqiIKbRkTvop9HUT/VWJXH8EhHnh5r/0NjSsQMinvzfXB
SvKkmkasGhhcSufo0Os367K40kcrONI0+YiipGTZSRtWONJTlr+x0fQzeCgSO50f
+GPpLjBMZ/rcuKGR5J//eVPNoAihYKPVpnX8lCrzjQ75lo1hCr7h7XJ/xkgWDR6Q
zH+8Pf9DoJSuAcAiC+Ftoj4am64DZi8uyYcYpTbrMSHts4XqjDV9RC4LelZnJ3xc
Fb4q+nHZrfRbbVioh4HOd+a9+aAWUMgodueXh8Ew2rkPZSd20yOauzV9SYS73Zcl
4+UbXDF8e9/p6TwMc+iPkcDkqe6olbKM8zqAerH/MB1svIe0M3nsuQEsv8ykygIo
6mdgStpoY74zAy/wVgupUbzappOsrv3hYhsYVZLsqnefr0x7QzEfPwBAIHiA6RLn
l0GSnah8Q34cvmiNck3JVBEBWKYIpemue6B0FSe6PV6Ouz3W5bs=
=G7D4
-----END PGP SIGNATURE-----

--5IP3FdTzgaNtzLjg--