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

Josh Berkus <josh@agliodbs.com> writes:
>> 
>> 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.

It doesn't look like we can provide for 100% backwards compatibility
with existing tools, here's why:

a) The only sensible way to use recovery.conf as GUC source is via  include/include_dir from postgresql.conf.

b) The server used to rename $PGDATA/recovery.conf to  $PGDATA/recovery.done so when it is re-started it doesn't
accidentallystart in recovery mode.
 

We can't keep doing (b) because that will make postgres fail to start on
a missing include.  Well, it won't error out if we place the file under
include_dir, as only files with the ".conf" suffix are included, but
then again the server wouldn't know which file to rename unless we tell
it or hard-code the the filename.  Either way this is not 100% backwards
compatible.

Now the question is if we are going to break compatibility anyway:
should we try to minimize the difference or will it disserve the user by
providing a false sense of compatibility?

For one, if we're breaking things, the trigger_file GUC should be
renamed to end_recovery_trigger_file or something like that, IMO.
That's if we decide to keep it at all.

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

Well, standby_mode is only consulted at startup after crash recovery.
But suddenly, a tool that *writes* recovery.conf needs to also
consult/edit postgresql.auto.conf...  Maybe that's inevitable, but still
a point to consider.

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

Makes sense.  Should we incorporate the actual tech and doc fix in this
patch?

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

Me neither.  It is tempting to make everything spin around GUCs without
the need for any external trigger files.

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

Well, I don't readily see how conf.d is special with regard to base
backup, wouldn't you need to exclude it explicitly still?

--
Alex