Re: Proposal for changes to recovery.conf API

On Thu, Sep 1, 2016 at 1:15 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> This is a summary of proposed changes to the recovery.conf API for
> v10. These are based in part on earlier discussions, and represent a
> minimal modification to current usage.
>
> Proposed changes (with reference to patches from Abhijit Menon-Sen and myself)
>
> * pg_ctl start -M (normal|recover|continue)
> pg_ctl can now start the server directly as a standby, similar to the
> way pg_ctl promote works. The internal implementation is also similar,
> with pg_ctl writing a "recovery.trigger" file that initiates recovery
> in the same way recovery.conf used to do. It is still possible to
> manually add a file called "recovery.trigger" and have that work if
> users desire that mechanism.
> Different startup methods can be selected with the <option>-M</option>
> option. <quote>Normal</quote> mode starts the server for read/write,
> overriding any previous use in Recover mode.
> <quote>Recover</quote> mode starts the server as a standby server which
> expects to receive changes from a primary/master server using physical
> streaming replication or is used for
> performing a recovery from backup. <quote>Continue</quote> mode is
> the default and will startup the server in whatever mode it was in at
> last proper shutdown, or as modified by any trigger files present.
> (Patch: recovery_startup_r10_api.v1b.patch)

So you basically just set ArchiveRecoveryRequested based on the
presence of recovery.trigger instead of recovery.conf. And the
interface of pg_ctl:
- recover mode => creates recovery.trigger
- continue mode => does nothing
- normal mode => removes recovery.trigger
That looks like a sound design.

> * Recovery parameters would now be part of the main postgresql.conf
> infrastructure
> Any parameters set in $DATADIR/recovery.conf will be read after the
> main parameter file, similar to the way that postgresql.conf.auto is
> read.
> (Abhijit)
> * pg_basebackup -R will continue to generate a parameter file called
> recovery.conf as it does now, but will also create a file named
> recovery.trigger.
> (This part is WIP; patch doesn't include implementation for tar format yet)

Or we could just throw away this dependency with recovery.conf,
simply. I see no need to keep it if recovery is triggered depending on
recovery.trigger, nor recommend its use. We could instead add
include_if_exists 'recovery.conf' at the bottom of postgresql.conf and
let the infrastructure do the rest to simplify the patch.

> * Parameters
> All of the parameters formerly set in recovery.conf can be set in
> postgresql.conf using RELOAD
> These parameters will have no defaults in postgresql.conf.sample
> Setting them has no effect during normal running, or once recovery ends.
> https://www.postgresql.org/docs/devel/static/archive-recovery-settings.html
> https://www.postgresql.org/docs/devel/static/recovery-target-settings.html
> https://www.postgresql.org/docs/devel/static/standby-settings.html
> (Abhijit)

Hm. I think that what would make sense here is a new GUC category,
meaning that once recovery is launched the new parameters are not
taken into account once again. Even updating primary_conninfo would
need a restart of the WAL receiver, so we could just make them
GUC_POSTMASTER and be done with it.

> Related cleanup
> * Promotion signal file is now called "promote.trigger" rather than
> just "promote"

If that's not strictly necessary this renaming is not mandatory.

> * Remove user configurable "trigger_file" mechanism - use
> "promote.trigger" for all cases

Ugh. I am -1 on that. There are likely backup tools and users that
rely on this option, particularly to be able to trigger promotion
using a file that is on a different partition than PGDATA.

> * Remove Fallback promote mechanism, so all promotion is now "fast" in xlog.c

No problem with that. Now others have surely other opinions. That
could be addressed as a separate patch.
--
Michael