Josh Berkus <j...@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 accidentally start 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
