Feedback-ID: i0fe9450f:Fastmail
Date: Sun, 24 Sep 2023 08:23:43 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Thomas Munro <thomas.munro@gmail.com>
Cc: Alexander Lakhin <exclusion@gmail.com>, Sergei Kornilov <sk@zsrv.org>,
	Noah Misch <noah@leadboat.com>, Tom Lane <tgl@sss.pgh.pa.us>,
	Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
	pgsql-bugs@lists.postgresql.org,
	Fujii Masao <masao.fujii@oss.nttdata.com>
Subject: Re: BUG #17928: Standby fails to decode WAL on termination of primary
Message-ID: <ZQ9zf1QO8CP4TZRO@paquier.xyz>
References: <ee604ee6-f464-1502-0289-f1046c85c182@gmail.com>
 <CA+hUKGKp_Go=EqA65r6naxrmXFfD-uH3DwPbm7tRCu9d2T=VyQ@mail.gmail.com>
 <ZQpCG5QanJQXNVH-@paquier.xyz>
 <CA+hUKGKJx=S77ttAtJ_iA=oBGUf2VTnfUp-gtJEhdXCbmdkoAA@mail.gmail.com>
 <ZQ0R4kz3gN6bqS1N@paquier.xyz>
 <CA+hUKGK1+3rj3c7L9HPCc8HajTVmkTx1CEkU5=rqgV3b4OvWeQ@mail.gmail.com>
 <ZQ4r1vHcryBsSi_V@paquier.xyz>
 <CA+hUKGKs=yj55ejAwuUJQuURU=uKjYgRqEQcCi1GrP2+=jUgNQ@mail.gmail.com>
 <ZQ5tIoarrcwAlbVZ@paquier.xyz>
 <CA+hUKGJf3Hhb2MB88-rW2di2H9XT0xr6-hd6ZjGEwdJs3A=b+Q@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="s78qGRLUwiaTeLCJ"
Content-Disposition: inline
In-Reply-To: 
 <CA+hUKGJf3Hhb2MB88-rW2di2H9XT0xr6-hd6ZjGEwdJs3A=b+Q@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/ZQ9zf1QO8CP4TZRO%40paquier.xyz>
Precedence: bulk


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

On Sun, Sep 24, 2023 at 09:48:42AM +1300, Thomas Munro wrote:
> "grison" has a little more detail --  we see
> pg_comp_crc32c_sb8(len=3D4294636456).  I'm wondering how to reproduce
> this, but among the questions that jump out I have: why was it ever OK
> that we load record->xl_tot_len into total_len, perform header
> validation, determine that total_len < len (=3D this record is all on
> one page, no reassembly loop needed, so now we're in the single-page
> branch), then call ReadPageInternal() again, then call
> ValidXLogRecord() which internally loads record->xl_tot_len *again*?
> ReadPageInternal() might have changed xl_tot_len, no?  That seems to
> be a possible pathway to reading past the end of the buffer in the CRC
> check, no?
>=20
> If that value didn't change underneath us, I think we'd need an
> explanation for how we finished up in the single-page branch at
> xlogreader.c:842 with a large xl_tot_len, which I'm not seeing yet,
> though it might take more coffee.  (Possibly supporting the re-read
> theory is the fact that it's only happening on a few very slow
> computers, though I have no idea why it would only happen on master
> [so far at least].)

Hmm, it looks pretty clear that this is a HEAD-only thing as the
buildfarm shows and as you say, and my primary suspect here would be
71e4cc6b8ec6, I think.  Any race condition underneath it would be
easier to see on slower machines.  So it's likely possible that this
has messed up the page insertion logic.
--
Michael

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmUPc38ACgkQnvQgOdby
QH3DNA//Zy70GBoctvIfoHVIPb+LBTpNfk2w9QQWHEskvQAdU4JHgYGZQi2/ksdp
qX1c1MZnadazw6sw9mUYMYG1wJmUGOgZt7f/QXhBHol8ay0jrwWhJeEG42b6/eFD
NMLaIQIALYB6aurtUX3NDtTWPrhmWKLzcVhjK4CKb7zikD5X+7PhCDQGAsyok0ss
3ntllEnN6Zl5kXaDnM9Bsz5gnRHLDlDCgo7D3/6rXTcwumsmAXBKmKpeT+d8+Fxz
ZsxPkR7swHl1hZWxo5MzbL1LtB3aqFAVC4UANlcEuaFmJSkLLQJYNE8WGN9ynjoi
5t1rScT4nxmG0b4pcKuo/ctXi3LlEDII8H3FGE/xPbzOOVyhcsnO6tjjy5VTaw6D
2VK4a3ikHFdxnsEp2EwSOPspUb/GeHrBhazCnN2jLG4pi+JzMdOllkPnc7FZga56
qPGHe+64ETDY6xOP9wXRuFh7LFr44L6D9VSqnDbALYMHch4dePvQqNq6acH7X45x
1ZktUkVBSBQ5xlP2aCjIFCEc88v+WO/gslSCZLwaURCQxtcnhmbFF7GQgDtuYVPI
qC2JW7hNFF5eZ9ycdeYxU2vUhBPv4kTXs+EJC/iLx7sLGwi0Wex5gKMWuSbBJTne
qYqoiztT+HzSae7DQKRENTdkggn0x+cRKmsuaXKuASNOWSf5QKM=
=Jqfy
-----END PGP SIGNATURE-----

--s78qGRLUwiaTeLCJ--