Re: Turning recovery.conf into GUCs

Jaime,
> = Building =
>
> In pg_basebackup we have now 2 unused functions:
> escapeConnectionParameter and escape_quotes. the only use of these
> functions was when that tool created the recovery.conf file so they
> aren't needed anymore.

Except that we'll want 9.4's -R to do something, probably create a file
called conf.d/replication.conf. Mind you, it won't need the same wonky
quoting stuff.

> 1) the file to trigger recovery is now called standby.enabled. I know
> this is because we use the file to also make the node a standby.
> Now, reality is that the file doesn't make the node a standby but the
> combination of this file (which starts recovery) + standby_mode=on.
> so, i agree with the suggestion of naming the file: recovery.trigger
>
> 2) This patch removes the dual existence of recovery.conf: as a state
> file and as a configuration file
>
> - the former is accomplished by changing the name of the file that
> triggers recovery (from recovery.conf to standby.enabled)
> - the latter is done by moving all parameters to postgresql.conf and
> *not reading* recovery.conf anymore
>
> so, after applying this patch postgres don't use recovery.conf for
> anything... except for complaining.
> it's completely fair to say we are no longer using that file and
> ignoring its existence, but why we should care if users want to use a
> file with that name for inclusion in postgresql.conf or where they put
> that hypotetic file?

So this is a bit of a catch-22. If we allow the user to use a file
named "recovery.conf" in $PGDATA, then the user may be unaware that the
*meaning* of the file has changed, which can result in unplanned
promotion of replicas after upgrade.

*on the other hand*, if we prevent creation of a configuration file
named "recovery.conf", then we block efforts to write
backwards-compatible management utilities.

AFAIK, there is no good solution for this, which is why it's taken so
long for us to resolve the general issue. Given that, I think it's
better to go for the breakage and all the warnings. If a user wants a
file called recovery.conf, put it in the conf.d directory.

I don't remember, though: how does this patch handle PITR recovery?

> = Code & functionality =

> + {"restore_command", PGC_POSTMASTER, WAL_ARCHIVE_RECOVERY,
> + {"archive_cleanup_command", PGC_POSTMASTER, WAL_ARCHIVE_RECOVERY,
> + {"recovery_end_command", PGC_POSTMASTER, WAL_ARCHIVE_RECOVERY,
> + {"recovery_target_xid", PGC_POSTMASTER, WAL_RECOVERY_TARGET,
> + {"recovery_target_name", PGC_POSTMASTER, WAL_RECOVERY_TARGET,
> + {"recovery_target_time", PGC_POSTMASTER, WAL_RECOVERY_TARGET,
> + {"trigger_file", PGC_POSTMASTER, REPLICATION_STANDBY,
>
> Not sure about these ones
>
> + {"recovery_target_timeline", PGC_POSTMASTER, WAL_RECOVERY_TARGET,
> + {"primary_conninfo", PGC_POSTMASTER, REPLICATION_STANDBY,

It would be really nice to change these on the fly; it would help a lot
of issues with minor changes to replication config. I can understand,
though, that the replication code might not be prepared for that.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com