public inbox for [email protected]
help / color / mirror / Atom feedFrom: Michael Paquier <[email protected]>
To: Kyotaro Horiguchi <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: [PATCH] Don't call ereport(ERROR) from recovery target GUC assign hooks
Date: Fri, 26 Jun 2026 17:12:15 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<CACSdjfMt5obQc0QCX8E+v-pxROWFQxmeW6Ki9X16F60D3tSJgg@mail.gmail.com>
<[email protected]>
<[email protected]>
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 = 'immediate', which
> would probably need an additional boolean flag (or similar state) to
> represent whether it has been specified.
>
> 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
Attachments:
[application/pgp-signature] signature.asc (833B, ../[email protected]/2-signature.asc)
download
view thread (25+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: [PATCH] Don't call ereport(ERROR) from recovery target GUC assign hooks
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox