Re: Changing recovery.conf parameters into GUCs

Hi,

The main argument on which this proposal is based on is to keep
backward-compatibility.
This has been discussed before many times and the position of each people
is well-known,
so I am not going back to that...

So, based on *only* what I see in this thread...

On Fri, Mar 29, 2013 at 12:48 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:

> What we want to do is make recovery parameters into GUCs, allowing
> them to be reset by SIGHUP and also to allow all users to see the
> parameters in use, from any session.
>
> The existing mechanism for recovery is that
> 1. we put parameters in a file called recovery.conf
> 2. we use the existence of a recovery.conf file to trigger archive
> recovery/replication
>
> I also wish to see backwards compatibility maintained, so am proposing
> the following:
>
> a) recovery parameters are made into GUCs (for which we have a patch
> from Fujii)
>
b) all processes automatically read recovery.conf as the last step in
> reading configuration files, if it exists, even if data_directory
> parameter is in use (attached patch)
> c) we trigger archive recovery by the presence of either
> recovery.conf or recovery.trigger in the data directory. At the end,
> we rename to recovery.done just as we do now. This means that any
> parameters put into recovery.conf will not be re-read when we SIGHUP
> after end of recovery. Note that recovery.trigger will NOT be read for
> parameters and is assumed to be zero-length.
> (minor patch required)
>
> This allows these use cases
>
> 1. Existing users create $PGDATA/recovery.conf and everything works as
> before. No servers crash, because the HA instructions in the wiki
> still work. Users can now see the parameters in pg_settings and we can
> use SIGHUP without restarting server. Same stuff, new benefits.
>
Forcing hardcoding of "include_if_exists recovery.conf" at the bottom of
postgresql.conf
is not a good thing for the user as it makes postgresql.conf processing
more opaque and
complicates parameter reloading. IMO, simplicity and transparency of
operations are
important when processing parameters.

2. New users wish to move their existing recovery.conf file to the
> config directory. Same file contents, same file name (if desired),
> same behaviour, just more convenient location for config management
> tools. Recovery is triggered by recovery.trigger in $PGDATA. Trigger
> and configuration are now separate, if desired.
>
So, people could be able to also define a recovery.trigger file? What about
the case where both recovery.conf and recovery.trigger are found in the
base folder?
Priority needs to be given to one way of processing or the other. Is it
really our goal
to confuse the user with internal priority mechanisms at least for GUC
handling?

3. Split mode. We can put things like trigger_file into the
> postgresql.conf directory and also put other parameters (for example
> PITR settings) into recovery.conf. Where multiple tools are in use, we
> support both APIs.
>
> Specific details...
>
> * primary_conninfo, trigger_file and standby_mode are added to
> postgresql.conf.sample
>
Not adding all the recovery_target_* parameters would confuse the user.
With this proposal it is actually possible to define a recovery target
inside
postgresql.conf even if the parameter is not plainly written in
postgresql.conf.sample.

> * all ex-recovery.conf parameters are SIGHUP, so no errors if
> recovery.conf has changed to .done
>
Drop of recovery.trigger at the same time? And what about the case
where both files are specified, that the server can only remove one of the
trigger files, and is then restarted with only 1 trigger file present?

If desired, this behaviour could be enabled by a parameter called
> recovery_conf_enabled = on (default). When set to off, this would
> throw an error if recovery.conf exists. (I don't see a need for this)
>
Me neither, the less GUCs the better.

> The patch to implement this is very small (attached). This works
> standalone, but obviously barfs at the actual parameter parsing stage.
> Just in case it wasn't clear, this patch is intended to go with the
> parts of Fujji's patch that relate to GUC changes.
>
> If we agree, I will merge and re-post before commit.
>
If an agreement is reached based on this proposal, I highly recommend that
you use one of the latest updated version I sent. Fujii's version had some
bugs, one of them being that as standbyModeRequested can be set to true if
specified in postgresql.conf, a portion of the code using in xlog.c can be
triggered even if ArchiveRecoveryRequested is not set to true. I also added
fixes related to documentation.

Comments from others are welcome.
--
Michael