On Tue, Nov 19, 2013 at 3:32 PM, Andres Freund <and...@2ndquadrant.com> wrote: > On 2013-11-19 22:09:48 +0900, Michael Paquier wrote: >> On Tue, Nov 19, 2013 at 2:27 AM, Andres Freund <and...@2ndquadrant.com> >> wrote: > >> > * I am not sure I like "recovery.trigger" as a name. It seems to close >> > to what I've seen people use to trigger failover and too close to >> > trigger_file. > >> This name was chosen and kept in accordance to the spec of this >> feature. Looks fine for me... > > I still think "start_as_standby.trigger" or such would be much clearer > and far less likely to be confused with the promotion trigger file. >
the function of the file is to inform the server it's in recovery and it needs to consider recovery parameters, not to make the server a standby. yes, i admit that is half the way to make the server a standby. for example, if you are doing PITR and stopping the server before some specific point (recovery_target_*) then "start_as_standby.trigger" will has no meaning and could confuse people >> > * You made recovery_target_inclusive/pause_at_recovery_target PGC_SIGHUP >> > - did you review that actually works? Imo that should be changed in a >> > separate commit. >> >> Yep, I'd rather see those parameters as PGC_POSTMASTER... As of now, >> once recovery is started those parameter values do not change once >> readRecoveryCommandFile is kicked. Having them SIGHUP would mean that >> you could change them during recovery. Sounds kind of dangerous, no? > > I think it's desirable to make them changeable during recovery, but it's > a separate patch. > uh! i read the patch and miss that. will change > >> > * Why did you change some of the recovery gucs to lowercase names, but >> > left out XLogRestoreCommand? >> >> This was part of the former patch, perhaps you are right and keeping >> the names as close as possible to the old ones would make sense to >> facilitate maintenance across versions. > > I think lowercase is slightly more consistent with the majority of the > other GUCs, but if you change it you should change all the new GUC variables. > This one was my change, in the original patch is called "restore_command" and i changed it because archive_command's parameter is called XLogArchiveCommand so i wanted the name to follow the same format. i can return it to the original name if no one likes XLogRestoreCommand -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación Phone: +593 4 5107566 Cell: +593 987171157 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
