Feedback-ID: i0fe9450f:Fastmail
Date: Mon, 20 Jun 2022 16:13:43 +0900
From: Michael Paquier <michael@paquier.xyz>
To: "Imseih (AWS), Sami" <simseih@amazon.com>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
	"alvherre@alvh.no-ip.org" <alvherre@alvh.no-ip.org>,
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Subject: Re: [BUG] Panic due to incorrect missingContrecPtr after promotion
Message-ID: <YrAeJw+DjSFkrgG1@paquier.xyz>
References: <C7EF4CE5-0ED5-46D8-B1C4-4B805006B9B9@amazon.com>
 <B27FBFC8-F9D9-47E1-A305-E93938FC23FF@amazon.com>
 <FB0DEA0B-E14E-43A0-811F-C1AE93D00FF3@amazon.com>
 <20220527.153735.871115454014517938.horikyota.ntt@gmail.com>
 <E37D2989-CFB6-4302-A871-A007DC71DA25@amazon.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="Sonn9DC8t5Nd7Ywd"
Content-Disposition: inline
In-Reply-To: <E37D2989-CFB6-4302-A871-A007DC71DA25@amazon.com>
Archived-At: 
 <https://www.postgresql.org/message-id/YrAeJw%2BDjSFkrgG1%40paquier.xyz>
Precedence: bulk


--Sonn9DC8t5Nd7Ywd
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Fri, May 27, 2022 at 07:01:37PM +0000, Imseih (AWS), Sami wrote:
> What we found:
>=20
> 1. missingContrecPtr is set when=20
>    StandbyMode is false, therefore
>    only a writer should set this value
>    and a record is then sent downstream.
>=20
>    But a standby going through crash=20
>    recovery will always have StandbyMode =3D false,
>    causing the missingContrecPtr to be incorrectly
>    set.

That stands as true as far as I know, StandbyMode would be switched
only once we get out of crash recovery, and only if archive recovery
completes when there is a restore_command.

> 2. If StandbyModeRequested is checked instead,
>      we ensure that a standby will not set a=20
>      missingContrecPtr.
>=20
> 3. After applying the patch below, the tap test succeeded

Hmm.  I have not looked at that in depth, but if the intention is to
check that the database is able to write WAL, looking at
XLogCtl->SharedRecoveryState would be the way to go because that's the
flip switching between crash recovery, archive recovery and the end of
recovery (when WAL can be safely written).

The check in xlogrecovery_redo() still looks like a good thing to have
anyway, because we know that we can safely skip the contrecord.  Now,
for any patch produced, could the existing TAP test be extended so as
we are able to get a PANIC even if we keep around the sanity check in
xlogrecovery_redo().  That would likely involve an immediate shutdown
of a standby followed by a start sequence?
--
Michael

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmKwHicACgkQnvQgOdby
QH2wMw/+I7jy4CSGi8gC8QAjSk0ri6U88vZSXlYW0FJIzv+KhwDrqxDvlFp0U1pt
m2XY5cWqKvielHuVwRkPsdW1oysJKmmoPXpE1Ftx9H9sxKwq+EtjIn0RXh55YQzz
VHFMDx8bhzenSjhXO9jrkIwUdijvGPl0XhNfBYMBj2S7YjP75DMSmL7+IWacdLft
mSXda3zhmcWclaPRNFHvdacoByhzbANTxH3qampySkuv0RV5r2yINSd2iGjOlxG0
WDcWwCF/LmmyRX5jca4N+hC6qKDnfKVibTgHed4MmRQjUqRNJRwFEMTANoqPafzf
XOjf1VRiAFyV0yV2ykbrW5wmgQW2VUj47FUtnWrC5KsdPOFd8F2xNEN3kXpgVTBD
JxPR960HNq10mx7sA9hyZOJtgJ5QVIFpcZwQCNIqgFMI9E8e2h9p2/j+8bren2bo
tzcWhj3yTxF2W9b6v/bn93o4FelWVDkfmvbip/1d3DTI0ySfRtu40lzU5194/Yuq
dC8Du3ub6IrMzYOkNa5afLOseQWJOUjSNOvpYO7kzvOY1cbyM6qCsmNiB/nXMFeQ
6UjhcYuzxCt8oGOpkoNmBl+susmog4LmFDkAGyxKk8ZCXCpZJv0ELypj2OelvmR6
DY3WphEnMCYFMFFSDaEXNZJRr+g4poXtFqXB4IOltvINCSX0MpY=
=gqaz
-----END PGP SIGNATURE-----

--Sonn9DC8t5Nd7Ywd--