On Fri, Oct 4, 2019 at 6:09 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Wed, Oct 02, 2019 at 03:30:38AM -0400, Stephen Frost wrote:
> > * David Steele (david@pgmasters.net) wrote:
> >> On 9/28/19 1:26 PM, Fujii Masao wrote:
> >>> There might be some recovery parameters that we can safely use
> >>> even in crash recovery, e.g., maybe recovery_end_command
> >>> (now, you can see that recovery_end_command is executed in
> >>> crash recovery in v12). But at this stage of v12, it's worth thinking to
> >>> just cause crash recovery to exit with an error when any recovery
> >>> parameter is set. Thought?
> >>
> >> I dislike the idea of crash recovery throwing fatal errors because there
> >> are recovery settings in postgresql.auto.conf. Since there is no
> >> defined mechanism for cleaning out old recovery settings we have to
> >> assume that they will persist (and accumulate) more or less forever.
>
> Yeah, I don't think that's a good thing either. That's a recipe to
> make the user experience more confusing.
>
> >>> Or if that change is overkill, alternatively we can make crash recovery
> >>> "ignore" any recovery parameters, e.g., by forcibly disabling
> >>> the parameters.
> >>
> >> I'd rather load recovery settings *only* if recovery.signal or
> >> standby.signal is present and do this only after crash recovery is
> >> complete, i.e. in the absence of backup_label.
> >>
> >> I think blindly loading recovery settings then trying to ignore them
> >> later is pretty much why we are having these issues in the first place.
> >> I'd rather not extend that pattern if possible.
> >
> > Agreed.
Agreed, too. Do you have any idea to implement that? I've not found out
"smart" way to do that yet.
One idea is, as Michael suggested, to use SetConfigOption() for all the
archive recovery parameters at the beginning of the startup process as follows,
to forcibly set the default values if crash recovery is running. But this
seems not smart for me.
SetConfigOption("restore_command", ...);
SetConfigOption("archive_cleanup_command", ...);
SetConfigOption("recovery_end_command", ...);
...
Maybe we should make GUC mechanism notice signal files and ignore
archive recovery-related parameters when none of those files exist?
This change seems overkill at least in v12, though.
Regards,
--
Fujii Masao