Re: Changing recovery.conf parameters into GUCs

On 10.07.2013 02:54, Josh Berkus wrote:
> On 07/08/2013 11:43 PM, Simon Riggs wrote:
>> 1. MOVE SETTINGS
>> All settings move into the postgresql.conf.
>>
>> Comment: AFAIK, all agree this.
>
> Good to go then.

+1.

>> 2. RELOCATE RECOVERY PARAMETER FILE(s)
>> As of 9.2, relocating the postgresql.conf means there are no user
>> writable files needed in the data directory.
>>
>> Comment: AFAIK, all except Heikki wanted this. He has very strong
>> objections to my commit that would have allowed relocating
>> recovery.conf outside of the data directory. By which he means both
>> the concepts of triggerring replication and of specifying parameters.
>> Changes in 9.3 specifically write files to the data directory that
>> expect this.
>
> Yeah, I didn't understand why this was shot down either.
>
> Heikki?

Does this refer to this:

http://www.postgresql.org/message-id/5152F778.2070205@vmware.com

? I listed some objections and suggestions there. Probably the biggest 
issue back then, however, was that it was committed so late in the 
release cycle. In any case, relocating the config/trigger file has 
nothing to do with turning recovery.conf parameters into GUCs, so let's 
not confuse this patch with that goal.

>> 3. SEPARATE TRIGGER FILE
>> Creating a standby.enabled file in the directory that houses the
>> postgresql.conf (same logic as "include" uses to locate things) 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.  I think this is
>> enough that people who just want to use replication features need never
>> hear about this file at all, at least during the important to simplify
>> first run through.
>>
>> Comment: AFAIK, all except Heikki are OK with this.

Sorry, I don't quite understand what this is about. Can you point me to 
the previous discussion on this?

"pg_ctl standby" sounds handy. It's not very useful without something 
like pg_rewind or some other mechanism to do a clean failover, though. 
Have to make sure that we have enough safeguards in place that you can't 
shoot yourself in the foot with that, though; if you turn a master 
server into a standby with that, must make sure that you can't corrupt 
the database if you point that standby to another standby.

But I don't see how that's related to changing recovery.conf parameters 
into gucs.

> One bit of complexity I'd like to see go away is that we have two
> trigger files: one to put a server into replication, and one to promote
> it.  The promotion trigger file is a legacy of warm standby, I believe.
>   Maybe, now that we have pg_ctl promote available, we can eliminate the
> promotion trigger?

No, see http://www.postgresql.org/message-id/5112A54B.8090500@vmware.com.

> Also, previously, deleting the recovery.conf file did not cause the
> server to be promoted AFAIK.  Is that something we should change if
> we're going to keep a trigger file to start replication?

Deleting recovery.conf file (and restarting) takes the server out of 
standby mode, but in an unsafe way. Yeah, would be nice to do something 
about that.

>> 4. DISALLOW PREVIOUS API
>> 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
>> it 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 conf.d
>> directory where the rename will make it then skipped.
>>
>> Comment: I am against this. Tool vendors are not the problem here.
>> There is no good reason to just break everybody's scripts with no
>> warning of future deprecataion and an alternate API, especially since
>> we now allow multiple parameter files.
>
> Well, the issue is not so much the presence of a recovery.conf file full
> of config variables ... which as you point out is now effectively
> supported ... but the use of that as a trigger file.   So I think the
> two points you make here are flipped.
>
> Personally, I don't care if we support the old recovery.conf-trigger
> behavior as long as I'm not forced to use it.  The main objection to
> supporting both was code complexity, I believe.

I'm also undecided on this one. If we can figure out a good way forward 
that keeps backwards-compatibility, good. But it's not worth very much 
to me - if we can get a better interface overall by dropping 
backwards-compatibility, then let's drop it.

- Heikki