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

On 1/23/13 6:36 AM, Michael Paquier wrote:
> The only problem I see is if the same parameter is defined in recovery.conf and postgresql.conf, is the priority given to recovery.conf?

This sort of thing is what dragged down the past work on this. I don't
really agree with the idea this thread is based on, that it was backward
compatibility that kept this from being finished last year. I put a
good bit of time into a proposal that a few people seemed happy with;
all its ideas seem to have washed away. That balanced a couple of goals:

-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?".

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'

--
Greg Smith 2ndQuadrant US greg(at)2ndQuadrant(dot)com Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.com