Feedback-ID: i0fe9450f:Fastmail
Date: Mon, 4 Nov 2024 15:04:09 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>,
	Pavel Borisov <pashkin.elfe@gmail.com>,
	Peter Eisentraut <peter@eisentraut.org>,
	pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: pgsql: Implement pg_wal_replay_wait() stored procedure
Message-ID: <Zyhj2anOPRKtb0xW@paquier.xyz>
References: 
 <CAPpHfdu7eAN6_Xfs+xSnRhu1zoN6WiOvj58i6qv27FgYcd8DSg@mail.gmail.com>
 <CALT9ZEERNvqJ1iDk==G_B-ZZXUBZROaEptR-zUC4mHC+iUmHUQ@mail.gmail.com>
 <CAPpHfdtc5AG0bRDaDPX+3UfwnONauGs6=vXzSKSnNs_rY8dfqg@mail.gmail.com>
 <ab0eddce-06d4-4db2-87ce-46fa2427806c@iki.fi>
 <CAPpHfdu+9NnT8Mx8gpYVXy7-BHANDVdrD6qRwt=-1=m1_pWMxw@mail.gmail.com>
 <14de8671-e328-4c3e-b136-664f6f13a39f@iki.fi>
 <CAPpHfduOkVL-GOC6V8w8gPv10f5mZ+nbwbuVn5xsLBS60jXA6A@mail.gmail.com>
 <CAPpHfdvRmTzGJw5rQdSMkTxUPZkjwtbQ=LJE2u9Jqh9gFXHpmg@mail.gmail.com>
 <CAPpHfdsywQyOcnFONiQocFu2c_bqotbU=7pG4g8RE8j3nqhhNg@mail.gmail.com>
 <CAPpHfdvzjEWA4a7eBhwjKJadZM4sTg0nfOfrbF8PDDK_TEqxoA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="29jdUWkjKlCSYaWU"
Content-Disposition: inline
In-Reply-To: 
 <CAPpHfdvzjEWA4a7eBhwjKJadZM4sTg0nfOfrbF8PDDK_TEqxoA@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/Zyhj2anOPRKtb0xW%40paquier.xyz>
Precedence: bulk


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

On Mon, Nov 04, 2024 at 06:29:42AM +0200, Alexander Korotkov wrote:
> The attached patchset contains patch 0001, which improves handling of
> not in recovery state by usage of PromoteIsTriggered().  When
> (PromoteIsTriggered() =3D=3D false), last replay LSN is not accepted and
> not reported in errdetail().
>=20
> 0002 contains patch finishing implicit transaction in default
> isolation level REPEATABLE READ or higher with revised commit message.

I was just catching up with this thread, and I'm still confused about
the state of things.  There are two things that are out of order for
me, at least, after skimming through the code on HEAD (I suspect there
is more):
- WaitForLSNReplay() uses an initial set of checks that are duplicated
in the main loop.  This is still overcomplicated, no?  Wouldn't it be
simpler to eliminate the first of checks or just have a goto block
with addLSNWaiter() called after the first round of checks?
- pg_wal_replay_wait_status() returns a status based on a static
variable that can only be accessed with the same backend as the one
that has called the wait function.  That's.. Err.. Unfriendly.  Why
being sticky with one backend for the job?

Using output parameters in a procedure is something I did not recall.
Based on your point about not using a function due your argument based
on the snapshots, let's just use that and forget about the status
function entirely (please?).

Based on the latest set of issues reported, it does not feel like this
is really baked and ready for prime day, either.  Perhaps it would be
less confusing to just revert the whole and repost a more complete and
structured implementation with an extra round of reviews?  There's
still time in this release cycle.
--
Michael

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmcoY9kACgkQnvQgOdby
QH1cTw//Y9VSp2g0m+7L8AvC4caOb4S/ruDV4/qkuEo1Ga4VaDUt/N2xGknbln7p
YQumioebdBce4rSbXbL33vwD1/melC/cMkWzlWOCaIP4dWy0eheIDkZcVeA33Ww9
zQQTNpwWiSICA5crp+Nzz+Y6hwJVkz7w2FPgZ1kDgw0h4BxXvUx58xqNhNfMp4NL
j3D5XFG7wtrfq6xFBwV6YNhtyH8UGJVKeLcNjPy6JZ97+jL9eiHXZvqoZGeKxZYx
vS7kON+Jqg3wrIi4A663oMjtn7q+gMKDDg/h5mY457ATkoB/FriqrECWLofu2SCS
tne+iBQzlF202kv+fURkicwOh34TDkS+eUtwyGzGNc2mdB25/k0fyQgMCEHe3QJp
20OTuhwDz9LeY1DhdW6/u8dfZ88AotaSC9CxnqrrF3KRzUCYPECs46DM7n1gqsxs
vwIXOqW77ixH+3gTjZMAFGhy2TUQgZorGiTUB4cupip+R5iH/gpF9GRpRLA0IB7s
qZ9ojagLSAlsqI1UEhT1ULu4uv7pgyiIqsgBtvTH91P9I75VKrC+eTozpxmxW+i3
ObjFwtRLwvB7thg3tClnKBO6Cwndo0QmHUDB4hLIuofKYhOmFQ8SBq6XPlhvjI5E
C/gYdjQBlebuckVSaVPi6i530hHBXWqzuRpvn+YxoqJ6JbNBNvM=
=eDN4
-----END PGP SIGNATURE-----

--29jdUWkjKlCSYaWU--