On Tue, Feb 15, 2011 at 2:10 AM, Stephen Frost <sfrost@snowman.net> wrote:
> Fujii,
>
> * Fujii Masao (masao.fujii@gmail.com) wrote:
>> Yeah, I rebased the patch to the current git master and attached it.
>
> Reviewing this, I just had a couple of comments and questions. Overall,
> I think it looks good and hence will be marking it 'Ready for
> Committer'.
Thanks for the review!
> * You removed trigger_file from the list in
> doc/src/sgml/high-availability.sgml and I'm not sure I agree with
> that. It's still perfectly valid and could be used by someone
> instead of pg_ctl promote. I'd recommend two things:
> - Adding comments into this recovery.conf snippet
Adding the following is enough?
+# NOTE that if you plan to use "pg_ctl promote" command to promote
+# the standby, no trigger file needs to be specified.
> - Adding a comment indicationg that trigger_file is only needed if
> you're not using pg_ctl promote.
Where should I add such a comment?
> * I'm not happy that pg_ctl.c doesn't #include something which defines
> all the file names which are used, couldn't we use a header which
> makes sense and is pulled in by pg_ctl.c and xlog.c to #define all of
> these? Still, that's not really the fault of this patch.
That would make sense. But I'm not sure that's possible. As a trial,
I added '#include "access/xlog.h"' into pg_ctl.c and compiled the source,
but I got many compilation errors. So probably hacking Makefiles is
required to do that. Do you know where should be changed?
> * I'm a bit worried that there's just only so many USR signals that we
> can send and it looks like we're burning another one here. Should we
> be considering a better way to do this?
You're worried about the case where users wrongly send the SIGUSR2
to the startup process, and then the standby is brought up to the master
unexpectedly?
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center