Re: Standby accepts recovery_target_timeline setting?

On Mon, Oct 7, 2019 at 9:14 AM Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> 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.

I think this approach is going in the wrong direction. In every other
part of the system, it's the job of the code around the GUC system to
use parameters when they're relevant and ignore them when they should
be ignored. Deciding that the parameters that were formerly part of
recovery.conf are an exception to that rule and that the GUC system is
responsible for making sure they're set only when we pay attention to
them seems like it's bringing back or exacerbating a code-level split
between recovery.conf parameters and postgresql.conf parameters when,
meanwhile, we've been wanting to eradicate that split so that the
things we allow for postgresql.conf parameters -- e.g. changing them
while they are running -- can be applied to these parameters also.

I don't particularly like the use of SetConfigOption() either,
although it does have some precedent in autovacuum, for example.
Generally, it's right and proper that the GUC system sets the
variables to which the parameters it controls are tied -- and then the
rest of the code has to do the right thing around that. It sounds like
the patch that got rid of recovery.conf wasn't considered carefully
enough, and missed the fact that it was introducing some inadvertent
behavior changes. That's too bad, but let's not overreact. It seems
totally fine to me to just add ad-hoc checks that rule out
inappropriately relying on these parameters while performing crash
recovery - and be done with it.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company