Date: Mon, 21 Dec 2020 16:13:17 +0900
From: Michael Paquier <michael@paquier.xyz>
To: exclusion@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #16780: Inconsistent recovery_target_xid handling across
 platforms
Message-ID: <X+BLDQOGr4V5oeBz@paquier.xyz>
References: <16780-107fd0c0385b1035@postgresql.org>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="Ml+e4hRXXwlAbGdD"
Content-Disposition: inline
In-Reply-To: <16780-107fd0c0385b1035@postgresql.org>
Precedence: bulk


--Ml+e4hRXXwlAbGdD
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Fri, Dec 18, 2020 at 10:00:00AM +0000, PG Bug reporting form wrote:
> So on Windows the recovery_target_xid parameter doesn't accept 64-bit
> value.
> As I see in check_recovery_target_xid() the strtoul() function is used to
> check/convert the input value, that is later stored into 32-bit variable.
> Thus, on platforms where unsigned long is 32-bit wide, you can't specify a
> result of pg_current_xact_id() as recovery_target_xid, while on platforms
> where unsigned long is 64-bit, you can, but high 32 bits are just ignored.

In order to make the experience consistent across all platforms, we
can use pg_strtouint64() for the GUC parsing.  I agree that ignoring
the high bits can be confusing as recovery would stop once a XID
matches with a modulo of UINT32_MAX, but beginning to issue an error
on platforms where unsigned long is 8 bytes could also break existing
tools.  At the same time, WAL record headers and 2PC state data only
use 32-bit XIDs, so ignoring the high bits changes nothing at
recovery (XIDs don't go across more than one epoch in the WAL stream
AFAIK).  In short, I agree that there is room for improvement here and
that we should just allow the case to work by replacing the use of
strtoul() to pg_strtouint64().  I am ready to bet that a lot of users
setting a target XID rely on the return result of txid_current() or
pg_current_xact_id() similarly to the test failing here.  And once the
epoch increments, the problems begin.

Thoughts?
--
Michael

--Ml+e4hRXXwlAbGdD
Content-Type: application/pgp-signature; name="signature.asc"

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAl/gSwwACgkQnvQgOdby
QH23RxAAktQNz5Pm7rOdt/Q+XZVEyigYAuDrJPye3VUSmVBZbb/6EWeG7qXMnjFm
q8rMGL+CYHkcd4n8VAajUUNOwTfWmXyTV22uQdaIgnxe9Xw8Kv1fB52aAc5o9v4e
ul+SgfbsOD/t/OfjkU21ofhiLTreTA4hinZY8vcQNR0WHBWsH54Gn0pWV7pgpCoT
jAxccnnFXT4F1BAjbSioCjrZsj8p7IpJPnz12aGAVTxz7ynkqWkc1qIBbCGOOyHp
a4GeuIHxsb9QW41phZS9IZqEPWfUO9OeSRRrte10gwic6h6lmTUSUdJ7DzbVIDYJ
ykwBeF4zQVZx1sqg7jZqAEATR/vhThd+BLD+nQ6lrRalkr5e6ToraSIou8bmZXt0
ZTNJPDQ8Yr2mCCKT4syu2ZJxThU1nPbMyb3Vf7TrYQ4/1JjRnAnBpVITnQrzYZmX
gK/tUXEn6uWGKFHHP7ukEq+44XXRgZwy+VL+ByyPdWWts3jxy7wZziSn/tP6v6jY
IeiqD7lqikix4uL2KtfhnkGdCEr1LO5nD8e1V7Q0WSYr2CZl0fdN2yYcUTKOay7l
XKGw8LkpGAU7K1Dbv2475Z1eleIkPktqhHt5nSghUHWh9HzA0LDqzlvAlv78d0en
i3ys+Gq8EShpRtgQlNPWXMe+Ihe+Dm3V9LEUlddt6nAydpmsyAo=
=WZFX
-----END PGP SIGNATURE-----

--Ml+e4hRXXwlAbGdD--