Re: Proposal for changes to recovery.conf API

On Fri, Dec 2, 2016 at 2:28 AM, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
wrote:

> On Fri, Dec 2, 2016 at 10:10 AM, Robert Haas <robertmhaas(at)gmail(dot)com>
> wrote:
> > On Thu, Dec 1, 2016 at 7:31 PM, Simon Riggs <simon(at)2ndquadrant(dot)com>
> wrote:
> >> * pg_basebackup -R
> >> will write recovery.trigger to data directory
> >> insert parameters postgresql.conf.auto, if possible
> >
> > Don't understand that last line; otherwise, +1.
>
> pg_basebackup -R creates a recovery.conf now, by appending the
> parameters to postgresql.conf.auto we are sure that:
> 1) there is no need to check for the existence of recovery.conf as it
> could be included by postgresql.conf with something like an
> include_if_exists
> 2) postgresql.conf.auto is loaded automatically without any additional
> tweaks needed in the GUC parsing code paths.
>

I'd also add the point that once there, you can deal with it the same way
as other parameters with say ALTER SYSTEM if you wish, so it integrates
with existing processes better than creating a separate config file and
including it. And since postgresql.conf.auto has a very well defined
format, editing it by machine (pg_basebackup that is) is fairly simple and
safe.

> >> * Add docs: "Guide to changes in recovery.conf in 10.0"
> >
> > Hmm, we don't usually write the docs in terms of how things are
> > different from a previous version. Might seem strange in 5 years.
> > Not sure what's best, here.
>
> A good chunk in the release notes would make sense as well?
>

It would.

And in fairness, having such a "guide to changes" chapter in each release
probably *would* be a good idea. But it would take resources to make that.
The release notes are good, but having a more hand-holding version
explaining incompatible changes in "regular sentences" would probably be
quite useful to users.

> >> * recovery_target as a single parameter, using proposed "xid 666"
> >> "target value" format
> >
> > +1.
> >
> >> * remove hot_standby parameter altogether, in line with earlier changes
> >
> > That seems a little surprising. We don't think anyone ever wants to
> > refuse connections during archive recovery?

Sure. But pg_hba.conf does that, and listen_addresses does that. We don't
really need yet-another-parameter for it.

> I suggested that yesterday. We have talked as well about merging
> standby_mode with hot_standby, but at the end most use cases I have
> seen involve looking at pg_is_in_recovery() these days to determine if
> a node is out of recovery of not, and this makes particularly more
> sense since 9.6 where wal_level = archive <=> hot_standby. The thought
> behind that is also partially that people complain that replication is
> too hard to understand for people.
>

In particular on the topic of confusion it would help. And just in general
not have mostly unnecessary parameters is a win.

//Magnus