Feedback-ID: i0fe9450f:Fastmail
Date: Fri, 14 Jun 2024 13:51:17 +0900
From: Michael Paquier <michael@paquier.xyz>
To: m.litsarev@postgrespro.ru
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: SQL function which allows to distinguish a server being in point
 in time recovery mode and an ordinary replica
Message-ID: <ZmvMRZs6bSDseMOQ@paquier.xyz>
References: <06c02ef916d85e77778e5cdfdf67a3df@postgrespro.ru>
 <D0L038YNGLYO.D2Q6GHBR7U40@neon.tech>
 <Zh26u-ifdXMPTrdq@paquier.xyz>
 <fac0db6695a7fefe33621a18f0484c49@postgrespro.ru>
 <Zjr-d9jSVz0Khw88@paquier.xyz>
 <4ba66566b84df983c881b996eb8831f1@postgrespro.ru>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="RpzOvGa20+1A3Wug"
Content-Disposition: inline
In-Reply-To: <4ba66566b84df983c881b996eb8831f1@postgrespro.ru>
Archived-At: <https://www.postgresql.org/message-id/ZmvMRZs6bSDseMOQ%40paquier.xyz>
Precedence: bulk


--RpzOvGa20+1A3Wug
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Jun 13, 2024 at 09:07:42PM +0300, m.litsarev@postgrespro.ru wrote:
> Hi!
>=20
> Michael,
> I have fixed the patches according to your comments.
>=20
> > merge both local variables into a single bits32 store?
> This is done in v3-0001-Standby-mode-requested.patch
>=20
> > Then this could be used with a function that returns a
> > text[] array with all the states retrieved?
> Placed this in the v3-0002-Text-array-sql-wrapper.patch
>=20
> > The refactoring pieces and the function pieces should be split, for
> > clarity.
> Sure. I also added the third patch with some tests. Perhaps it would be
> usefull.

+     *    --    XLR_PROMOTE_IS_TRIGGERED indicates if a standby promotion
+     *        has been triggered. Protected by info_lck.
+     *
+     *     --    XLR_STANDBY_MODE_REQUESTED indicates if we're in a standb=
y mode
+     *        at start, while recovery mode is on. No info_lck protection.
+     *
+     *    and can be extended in future.

This comment is incorrect for XLR_STANDBY_MODE_REQUESTED?  A startup
we would unlikely be in standby mode, most likely in crash recovery,
then switch to standby mode.

-    LocalPromoteIsTriggered =3D XLogRecoveryCtl->SharedPromoteIsTriggered;
+    LocalRecoveryDataFlags &=3D ~XLR_PROMOTE_IS_TRIGGERED;
+    LocalRecoveryDataFlags |=3D
+        (XLogRecoveryCtl->SharedRecoveryDataFlags & XLR_PROMOTE_IS_TRIGGER=
ED)

Are these complications really needed?  All these flags are false,
then switched to true.  true -> false is not possible.

         StandbyModeRequested =3D true;
         ArchiveRecoveryRequested =3D true;
+        XLogRecoveryCtl->SharedRecoveryDataFlags |=3D XLR_STANDBY_MODE_REQ=
UESTED;

Shouldn't STANDBY_MODE be only used in the local flag, as well as an
ARCHIVE_RECOVERY_REQUESTED?  It looks like this could push a bit more
forward the removal of more of these booleans, with a bit more work..

     return (LocalRecoveryDataFlags & XLR_HOT_STANDBY_ACTIVE);
 }
=20
+
 /*
Some noise lying around.

+	/* Returns bit array as Datum */
+	txt_arr =3D construct_array_builtin(flags, cnt, TEXTOID);

Yep, that's the correct way to do it.

+is($ret_mode_primary, '{}', "master is not a replica");=20

The test additions are welcome.  Note that we avoid the word "master",
see 229f8c219f8f.
--
Michael

--RpzOvGa20+1A3Wug
Content-Type: application/pgp-signature; name="signature.asc"

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmZrzEUACgkQnvQgOdby
QH3Ipg/8DARdfDLhz/N3i2X1/5per6wC5bmnvGytmyB9PPENY78w5yMilTfdQ/pn
eNfvZ2/rQQHF5739LoHZiVZTkMTBUqmtEE/yx+aHTWuytsA2ow2D4k6BLfFOKaT4
yI8zlZGuL0M26eftZKuovvoVCg1izzPWSJxf/3aiA6yf4xjMfuTy8kM42wCMVUO0
abgWSXiO8FEW1f7UVQlwn1nA7gTeHItZeLakn1VXNbhyuXme3/po2IeWY45dHsxr
dFVLJQLuK+TaevF5GihhNCiWjHG9k7YRsv/2q1MkkNR5VJ+HPew4JAMjviVnoC2W
3YpbHDyPufcD+47CplSKGDB3tQS4odN8X8PQHp0maTHPFxUp7aNt0xVxI/LYqYDV
EVBOXbtUGE/+vTXT+p+ijrv6xedwhXmVr+VSYEBf8Rnp2gxINF081/uSMIfWjchC
nnZNwoNIUQJMP4Fh9yaQAZ/OqDXB40H0MY5I2acQYf2SVUwALuvlq9iMkxcELIQn
THekSnGEz8TqbQMXQSS5X/nrYYpXN3X4nJY/5IPhL1fUxEuP7CEgQ70+DaHQIrOj
VGUUrJhlEczH/1HL5BgXOWz9HhVmrcsfxedeP4dUQRe2mmrYL0Jy75QmB5o/NpZB
xswNu02r3Te4gpDPBAqqNSRcBZXS5piH28ZxFgnFKzkeRo+Xmao=
=6Oti
-----END PGP SIGNATURE-----

--RpzOvGa20+1A3Wug--