Re: Request for vote to move forward with recovery.conf overhaul

On Sun, Mar 3, 2013 at 9:25 PM, Greg Smith <greg(at)2ndquadrant(dot)com> wrote:
> -Allow a "pg_ctl standby" and "pg_ctl recover" command that work similarly
> to "promote". This should slim down the work needed for the first
> replication setup people do.
> -Make it obvious when people try to use recovery.conf that it's not
> supported anymore.
> -Provide a migration path for tool authors strictly in the form of some
> documentation and error message hints. That was it as far as concessions to
> backward compatibility.
>
> The wrap-up I did started at
> http://www.postgresql.org/message-id/4EE91248.8010505@2ndQuadrant.com and
> only had a few responses/controversy from there. Robert wrote a good
> summary:
>
> 1. Get rid of recovery.conf - error out if it is seen
> 2. For each parameter that was previously a recovery.conf parameter, make it
> a GUC
> 3. For the parameter that was "does recovery.conf exist?", replace it with
> "does standby.enabled exist?".

All that works for me.

> I thought this stopped from there because no one went back to clean up
> Fujii's submission, which Simon and Michael have now put more time into.
> There is not much distance between it and the last update Michael sent.
> Here's the detailed notes from my original proposal, with updates to
> incorporate the main feedback I got then; note that much of this is
> documentation rather than code:
>
> -Creating a standby.enabled file puts the system into recovery mode. That
> feature needs to save some state, and making those decisions based on
> existence of a file is already a thing we do. Rather than emulating the
> rename to recovery.done that happens now, the server can just delete it, to
> keep from incorrectly returning to a state it's exited. A UI along the
> lines of the promote one, allowing "pg_ctl standby", should fall out of
> here.
>
> This file can be relocated to the config directory, similarly to how the
> include directory looks for things. There was a concern that this would
> require write permissions that don't exist on systems that relocate configs,
> like Debian/Ubuntu. That doesn't look to be a real issue though. Here's a
> random Debian server showing the postgres user can write to all of those:
>
> $ ls -ld /etc/postgresql
> drwxr-xr-x 4 root root 4096 Jun 29 2012 /etc/postgresql
> $ ls -ld /etc/postgresql/9.1
> drwxr-xr-x 3 postgres postgres 4096 Jul 1 2012 /etc/postgresql/9.1
> $ ls -ld /etc/postgresql/9.1/main
> drwxr-xr-x 2 postgres postgres 4096 Mar 3 11:00 /etc/postgresql/9.1/main
>
> -A similar recovery.enabled file turns on PITR recovery.
>
> -It should be possible to copy a postgresql.conf file from master to standby
> and just use it. For example:
> --"standby_mode = on": Ignored unless you've started the server with
> standby.enabled, won't bother the master if you include it.
> --"primary_conninfo": This will look funny on the master showing it
> connecting to itself, but it will get ignored there too.
>
> -If startup finds a recovery.conf file where it used to live at,
> abort--someone is expecting the old behavior. Hint to RTFM or include a
> short migration guide right on the spot. That can have a nice section about
> how you might use the various postgresql.conf include* features if they want
> to continue managing those files separately. Example: rename what you used
> to make recovery.conf as replication.conf and use include_if_exists if you
> want to be able to rename it to recovery.done like before. Or drop it into
> a config/ directory (similarly to the proposal for SET PERSISTENT) where the
> rename to recovery.done will make it then skipped. (Only files ending with
> .conf are processed by include_dir)
>
> -Tools such as pgpool that want to write a simple configuration file,
> only touching the things that used to go into recovery.conf, can tell
> people to do the same trick. End their postgresql.conf with a call to
> \include_if_exists replication.conf as part of setup. While I don't
> like pushing problems toward tool vendors, as one I think validating if
> this has been done doesn't require the sort of fully GUC compatible
> parser people (rightly) want to avoid. A simple scan of the
> postgresql.conf looking for the recommended text at the front of a line
> could confirm whether that bit is there. And adding a single
> "include_if_exists" line to the end of the postgresql.conf is not a
> terrible edit job to consider pushing toward tools. None of this is any
> more complicated than the little search and replace job that initdb does
> right now.
>
> -If you want to do something special yourself to clean up when recovery
> finishes, perhaps to better emulate the old "those settings go away"
> implementation, there's already recovery_end_command available for that.
> Let's say you wanted to force the old name and did "include_if_exists
> config/recovery.conf". Now you could do:
>
> recovery_end_command = 'rm -f /tmp/pgsql.trigger.5432 && mv
> conf.d/recovery.conf conf.d/recovery.done'

No argument with any of that, either.

If that's what we're implementing, I'm on board.

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