Re: Turning recovery.conf into GUCs

On Fri, Nov 21, 2014 at 12:55 PM, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
> 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.
>

well, my approach was that postgres just ignore the file completely. I
mean, recovery.conf will no longer mean anything special.
Then, every tool that create recovery.conf in $PGDATA only has to add
an ALTER SYSTEM to include 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).
>

no. currently we enter in recovery mode when postgres see a
recovery.conf and stays in recovery mode when standby_mode is on or an
appropiate restore_command is provided.

which means recovery.conf has two uses:
1) start in recovery mode (not continuous)
2) provide parameters for recovery mode and for streaming

we still need a "recovery trigger" file that forces postgres to start
in recovery mode and acts accordingly to recovery GUCs

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

we need to delete or rename the "recovery trigger" file, all standby
GUCs are ignored (and recovery GUCs should be ignored too) unless
you're in recovery mode

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

haven't read that thread, will do

>
>>>> /* 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.)
>

yes, we need it. but other names were suggested "standby.enabled"
transmit the wrong idea

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

this is not for promotion, this is to force postgres to start in
recovery mode and read recovery configuration parameters.

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

only that you need to start in recovery mode to start replication

--
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157