On Tue, Nov 20, 2018 at 12:07:12PM +0100, Peter Eisentraut wrote: > I went over the patch and did a bunch of small refinements. I have also > updated the documentation more extensively. The attached patch is > committable to me.
Thanks for putting energy into that. > Recovery is now initiated by a file recovery.signal. Standby mode is > initiated by a file standby.signal. The standby_mode setting is > gone. If a recovery.conf file is found, an error is issued. I am having second thoughts about this part of the patch actually. What's bad in keeping standby_mode and just rely on recovery.signal to enforce recovery to happen? When the startup process starts all the parameters should be loaded. That would also need less work from users to switch to the new APIs. I think that there could be a point to *merge* hot_standby and standby_mode actually into an enum, so keeping standby_mode would help with that (not this patch work of course). The barrier between recovery.trigger standby.trigger is also rather thin. > The trigger_file setting has been renamed to promote_trigger_file as > part of the move. This rename looks fine. > pg_basebackup -R now appends settings to postgresql.auto.conf and > creates a standby.signal file. > > Author: Simon Riggs <si...@2ndquadrant.com> > Author: Abhijit Menon-Sen <a...@2ndquadrant.com> > Author: Sergei Kornilov <s...@zsrv.org> I think that Fujii Masao should also be listed as an author, or at least get a mention. He was one of the first to work on this stuff. Using separate GUCs for each target is fine by me. I would have thought that 003_recovery_targets.pl needed some tweaks, so I am happy to see that it is not the case. So overall this stuff looks fine per its shape, just the part for standby.signal may not justify breaking compatibility more than necessary... The first mention of this part comes from https://postgr.es/m/canp8+jloysdjb5ip7wynvcckoe4oynhabunb8pqmgbjgfkq...@mail.gmail.com as far as I know. -- Michael
signature.asc
Description: PGP signature
