Re: [HACKERS] pg_ctl failover Re: Latches, signals, and waiting
* Fujii Masao (masao.fu...@gmail.com) wrote: On Tue, Feb 15, 2011 at 2:10 AM, Stephen Frost sfr...@snowman.net wrote: * 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. No.. I was thinking of having comments throughout the whole file, similar to what we have for postgresql.conf.sample. I realize it's an example in the docs, but people are likely to copy paste it into their own files. - 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? This was still in the example recovery.conf in high-availability.sgml. * 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? No, I wouldn't expect that to work.. I would have thought something that was already included in the necessary places, eg, miscadmin.h. * 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? No.. My concern was that it looked like we were adding a meaning to SIGUSR2 which might make it unavailable for other uses later, and at least according to man 7 signal on my system, there's only 2 User-defined signals available (SIGUSR1 and SIGUSR2).. I just wouldn't want to use up something which is a finite resource without good cause, in case we have a need for it later. Now, perhaps that's not happening and my concern is unfounded. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] pg_ctl failover Re: Latches, signals, and waiting
Fujii, * Fujii Masao (masao.fu...@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'. * 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 a comment indicationg that trigger_file is only needed if you're not using pg_ctl promote. * 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. * 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? Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] pg_ctl failover Re: Latches, signals, and waiting
On Tue, Feb 15, 2011 at 2:10 AM, Stephen Frost sfr...@snowman.net wrote: Fujii, * Fujii Masao (masao.fu...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
