Feedback-ID: i0fe9450f:Fastmail
Date: Fri, 26 Jun 2026 17:12:15 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: sjh910805@gmail.com, alvherre@kurilemu.de, scott@scottray.io,
	masao.fujii@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: [PATCH] Don't call ereport(ERROR) from recovery target GUC
 assign hooks
Message-ID: <aj40X40Y1ON5E29a@paquier.xyz>
References: <ajfWZqpzgziKNzX5@alvherre.pgsql>
 <CACSdjfMt5obQc0QCX8E+v-pxROWFQxmeW6Ki9X16F60D3tSJgg@mail.gmail.com>
 <ajzgIRrl8378bnlt@paquier.xyz>
 <20260626.161553.236415645647775260.horikyota.ntt@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="3DCYKRVakMQELXPE"
Content-Disposition: inline
In-Reply-To: <20260626.161553.236415645647775260.horikyota.ntt@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/aj40X40Y1ON5E29a%40paquier.xyz>
Precedence: bulk


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

On Fri, Jun 26, 2026 at 04:15:53PM +0900, Kyotaro Horiguchi wrote:
> As far as I can tell, the existing recovery target variables already
> seem sufficient to determine whether each target is configured. The
> only exception appears to be recovery_target =3D 'immediate', which
> would probably need an additional boolean flag (or similar state) to
> represent whether it has been specified.
>=20
> Then validateRecoveryParameters() could determine which recovery
> target is configured by inspecting those variables directly, report an
> error if more than one target is set, and reconstruct the same
> recoveryTarget state that the current code expects.

And delay setting recoveryTarget at all until we enter the validation
step in the startup process?  We don't use it in any of the early
steps before entering validateRecoveryParameters() (if we do so, that
would be an incorrect thing to do anyway).

I don't see on top of my mind why that would not work, skipping all
the RECOVERY_TARGET_UNSET manipulations in the assign hooks.  One
source of the confusion, to me, is that we have taken the decision to
link directly recoveryTarget with the result of the GUC, while
recoveryTarget is also used to track if the other target GUCs are set.
It looks to me that we shouldn't use recoveryTarget to track both if a
target is set and if the immediate state is set, and split both
things.  This is what I guess you are getting at with your extra
boolean.  Let's call it a new recoveryTargetImmediate.

At the end recoveryTarget should never be touched in any of the GUC
hooks, just once we enter validateRecoveryParameters().
--
Michael

--3DCYKRVakMQELXPE
Content-Type: application/pgp-signature; name=signature.asc

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

iQIzBAEBCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmo+NF8ACgkQnvQgOdby
QH1LMQ/7BAnV91tuWc2BvMNJP6ubjUcHqVC9Zvv68SggwyAyQhIgRuOzgngC8OXx
NQCBbze9ikX5/f2cfRckIJhuq6MsGRF4DOT84gEMeNv3zUF4d8yjGj8iCGTwG26f
ywURByT1YvGD+cB6Ogjpnt/BHL8vkCgOAYB+NAmXuH23p8/p8DogeEs5/eFIsgop
dBAaWkG1Qr+Hf3Gt2EwqVq1yfWiEioR0f46zqBcIrymnwk4WqZgnoeYInVGLIToF
H7pQZCGlz5E6kVoGm4kYZQaxCd3X/sKofCvyShMo1i+2uzgxFVgYfvlbthpjemnT
Dsamvp7807Id694yLxC/4p/RfrzolT1ghyyR+TeQKnDF4q+8La383ZOwwmhdkboO
WKlMUdDM9mYSZAr83gsVv26lW+7hYcTa7TQfxEnnPSlHZzgQdJkh4RkhanoG8XQ2
5iaB8Vlr0HhgmR64LpIMkBSc3VnFWA5H/hUmfZeROHkUkj7NqlavRLDvXTxjjKQf
uSc9m47Fx+aVvzJtGurFCZ5djsEeeDmrz+BKg5lGlp2/oYcJNx6IS7PpSEYYIj7c
/C+h/Cnx+kJfiidpASmDEbNUR9Fw3AuPSUQsVOtnzDcGkpmGjnPWHXp5jvfKu6at
9EwZAakh3gUEhhhsu7a/TLxry4H/o7k+DmD2c3P4lY9gzLGXO+M=
=cwet
-----END PGP SIGNATURE-----

--3DCYKRVakMQELXPE--