Re: Turning recovery.conf into GUCs - Mailing list pgsql-hackers

On 11/21/2014 09:35 AM, Alex Shulgin wrote:
> Hello,
> 
> Here's an attempt to revive this patch.

Yayy!  Thank you.

>> People might not like me for the suggestion, but I think we should
>> simply always include a 'recovery.conf' in $PGDATA
>> unconditionally. That'd make this easier.
>> Alternatively we could pass a filename to --write-recovery-conf.
> 
> Well, the latest version of this patch fails to start when it sees
> 'recovery.conf' in PGDATA:
> 
>   FATAL:  "recovery.conf" is not supported anymore as a recovery method
>   DETAIL:  Refer to appropriate documentation about migration methods
> 
> I've missed all the discussion behind this decision and after reading
> the ALTER SYSTEM/conf.d thread I'm even more confused so I'd like
> someone more knowledgeable to speak up on the status of this.

The argument was that people wanted to be able to have an empty
recovery.conf file as a "we're in standby" trigger so that they could
preserve backwards compatibility with external tools.  I don't agree
with this argument, but several people championed it.

> The docs use the term "continuous recovery".
> 
> Either way, from the code it is clear that we only stay in recovery if
> standby_mode is directly turned on.  This makes the whole check for a
> specially named file unnecessary, IMO: we should just check the value of
> standby_mode (which is off by default).

So, what happens when someone does "pg_ctl promote"?  Somehow
standby_mode needs to get set to "off".  Maybe we write "standby_mode =
off" to postgresql.auto.conf?

> By the way, is there any use in setting standby_mode=on and any of the
> recovery_target* GUCs at the same time?

See my thread on this topic from last week.  Short answer: No.

> I think it can only play together if you set the target farther than the
> latest point you've got in the archive locally.  So that's sort of
> "Point-in-Future-Recovery".  Does that make any sense at all?

Again, see the thread.  This doesn't work in a useful way, so there's no
reason to write code to enable it.

>>>  /* File path names (all relative to $PGDATA) */
>>> -#define RECOVERY_COMMAND_FILE    "recovery.conf"
>>> -#define RECOVERY_COMMAND_DONE    "recovery.done"
>>> +#define RECOVERY_ENABLE_FILE    "standby.enabled"
>>
>> Imo the file and variable names should stay coherent.
> 
> Yes, once we settle on the name (and if we really need that extra
> trigger file.)

Personally, if we have three methods of promotion:

1) pg_ctl promote
2) edit postgresql.conf and reload
3) ALTER SYSTEM SET and reload

... I don't honestly think we need a 4th method for promotion.

The main reason to want a "we're in recovery file" is for PITR rather
than for replication, where it has a number of advantages as a method,
the main one being that recovery.conf is unlikely to be overwritten by
the contents of the backup.

HOWEVER, there's a clear out for this with conf.d.  If we enable conf.d
by default, then we can simply put recovery settings in conf.d, and
since that specific file (which can have whatever name the user chooses)
will not be part of backups, it would have the same advantage as
recovery.conf, without the drawbacks.

Discuss?

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