Feedback-ID: i0fe9450f:Fastmail
Date: Thu, 26 Oct 2023 16:31:19 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Jeff Davis <pgsql@j-davis.com>
Cc: Alexander Lakhin <exclusion@gmail.com>,
	Robert Haas <robertmhaas@gmail.com>,
	Heikki Linnakangas <hlinnaka@iki.fi>, pgsql-hackers@postgresql.org
Subject: Re: Is this a problem in GenericXLogFinish()?
Message-ID: <ZToVxyLSVUisFPp0@paquier.xyz>
References: <bf396b45-318a-4940-87eb-3d295a93da8b@iki.fi>
 <b1f2d0f230d60fa8df33bb0b2af3236fde9d750d.camel@j-davis.com>
 <CA+TgmoZ54E0w5vCXYRAFKnunZy_SKB-4rbFq+7L=qUgFUYdtfQ@mail.gmail.com>
 <d54c5738052eead54d65e4fd3667f161aa447680.camel@j-davis.com>
 <CA+TgmoY+dagCyrMKau7UQeQU6w4LuVEu+yjsmJBoXKAo6XbUUA@mail.gmail.com>
 <4b2dfe9c67713413a06be761f677bc34222afcb4.camel@j-davis.com>
 <CA+TgmobjyWDQfiMLj1_Lf0GheiY0SaOEPbFEepfa3G-XOLmHOA@mail.gmail.com>
 <23d87b531fab74e69fd753791a7e5047f17da9ab.camel@j-davis.com>
 <f045c8f7-ee24-ead6-3679-c04a43d21351@gmail.com>
 <8a1c93b33b6e0aac92b771339cce520eac0adf96.camel@j-davis.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="yhiQ2EQ1CG8zFss4"
Content-Disposition: inline
In-Reply-To: <8a1c93b33b6e0aac92b771339cce520eac0adf96.camel@j-davis.com>
Archived-At: 
 <https://www.postgresql.org/message-id/ZToVxyLSVUisFPp0%40paquier.xyz>
Precedence: bulk


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

On Wed, Oct 25, 2023 at 10:06:23PM -0700, Jeff Davis wrote:
> Thank you for the report! I was able to reproduce and observe that the
> buffer is not marked dirty.
>=20
> The call (hashovfl.c:671):
>=20
>   XLogRegisterBuffer(1, wbuf, REGBUF_STANDARD)
>=20
> is followed unconditionally by:
>=20
>   PageSetLSN(BufferGetPage(wbuf), recptr)
>=20
> so if the Assert were not there, it would be setting the LSN on a page
> that's not marked dirty. Therefore I think this is a bug, or at least
> an interesting/unexpected case.
>=20
> Perhaps the registration and the PageSetLSN don't need to happen when
> nitups=3D=3D0?

Hmm.  Looking at hash_xlog_squeeze_page(), it looks like wbuf is
expected to always be registered.  So, you're right here: it should be
OK to be less aggressive with setting the page LSN on wbuf if ntups is
0, but there's more to it?  For example, it is possible that
bucketbuf, prevbuf and wbuf refer to the same buffer, causing
is_prim_bucket_same_wrt and is_prev_bucket_same_wrt to be both true.=20
Particularly, if nextbuf and wbuf are the same buffers, we finish by
registering twice the same buffer with two different IDs to perform
the tuple insertion and the opaque area updates in two steps.  And
that's..  Err..  not really necessary?  I mean, as far as I read this
code you could just reuse the buffer registered at ID 1 and update its
opaque area if is_prim_bucket_same_wrt is true?
--
Michael

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmU6FccACgkQnvQgOdby
QH2nFQ/9EwMKon984xCilE6EPgJeYa4OqY+PqcVWkdU4UO9hTOSHi3dQNOQxr0EY
4wffDOzpALaFbd9hxEaAdezRF6FVgQCjc+of28wab/uoeTwdCRkEZHGqa5bKaxVB
Dz1/phMWJgQL2HRiFTcTuopY+HCQo5P6U6GzsWQUYXEU37KW5Vcmqo+H+hjM+0w1
nRnnxjBX03R1demirKLOVJHIuX34SfgIz+ot7421jFjqnM9dBym+7oZ2gPCjl69z
amLIdHu4ycoAmPuY/chEMnGBOm8N9/OtyD+HeZx2ftZeamfvA+exQZZ8KMIOnKk8
ASRdBe6zZ8TAM7y7U+/9d1BESbxA7IJ6VTcv3tVitps3RLubKT1DW5u8Jn1v/bOm
STl8zKMiZXMxiyHePifIrXXpYPm+gHvO+mNN71tutcuL5UImLWdCr07QaNUVb/VS
nyiTv8j/+6fo5T+ClBdPKB+wXmWCtk+4tv9D9zMdgAjWdVX629aVEplhbsqsDEDg
2O8mN1oqyWC4ba8RwWXM9bhdJHPP5kQqEZ1vM1fv46LGDff7m67N11zFulu//zSM
8bri11eOXT9d6SvG4Ug3Xhc7/W8gYw2KTKmMM9nWk22P/CwZ9F8O/N18CUjkQCZd
R9uR9zk+4OQZE7K0+nDgFqaIUJs8MYHwgKX5cb7+YAkyDkNfPNw=
=RQvt
-----END PGP SIGNATURE-----

--yhiQ2EQ1CG8zFss4--