<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, May 9, 2017 at 6:16 AM, Tatsuo Ishii <span dir="ltr"><<a href="mailto:ishii@sraoss.co.jp" target="_blank">ishii@sraoss.co.jp</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">Usama,<br>
<br>
While adding a new test case to 003.failover regression test, I found<br>
a corner case bug in primary failover.<br>
<br>
Suppose Pgpool-II starts but is yet finding primary node. If primary<br>
failover happens, it skips finding primary node and let the initial<br>
value of it (Req_info->primary_node_id == -1) to be used as the new<br>
primary node id. As a result, no primary node id exists until next<br>
failover happens.<br>
<br>
Initialy I thought The problem is in the code of<br>
pgpool_main.c:failover() which tries to optimize finding primary node<br>
process.<br>
<br>
/*<br>
* If the down node was a standby node in streaming replication<br>
* mode, we can avoid calling find_primary_node_repeatedly() and<br>
* recognize the former primary as the new primary node, which<br>
* will reduce the time to process standby down.<br>
*/<br>
else if (MASTER_SLAVE && pool_config->master_slave_sub_<wbr>mode == STREAM_MODE &&<br>
reqkind == NODE_DOWN_REQUEST)<br>
{<br>
if (Req_info->primary_node_id != node_id)<br>
new_primary = Req_info->primary_node_id;<br>
else<br>
new_primary = find_primary_node_repeatedly()<wbr>;<br>
<br>
I was attempting to fix it by checking Req_info->primary_node_id to<br>
see if it's initial value (-1) or not. If it's -1,<br>
find_primary_node_repeatedly() need to be called.<br>
<br>
But looking into pgpool_main() closely, I suspect there's a<br>
fundamental problem:<br>
<br>
1) It processes failover in CHECK_REQUEST *before* setting<br>
Req_info->primary_node_id.<br>
<br>
/*<br>
* check for child signals to ensure child startup before reporting successfull start<br>
*/<br>
CHECK_REQUEST;<br>
<br>
ereport(LOG,<br>
(errmsg("%s successfully started. version %s (%s)", PACKAGE, VERSION, PGPOOLVERSION)));<br>
<br>
/*<br>
* if the primary node id is not loaded by watchdog, search for it<br>
*/<br>
if (Req_info->primary_node_id < 0)<br>
{<br>
/* Save primary node id */<br>
Req_info->primary_node_id = find_primary_node();<br>
}<br>
<br>
2) It uses find_primary_node(), rather than<br>
find_primary_node_repeatedly()<wbr>. So if by some reasons (for example<br>
the backend does not come up yet), find_primary_node() will fail<br>
and Req_info->primary_node_id is set to -1.<br>
<br>
I think proper fix will be moving the CHECK_REQUEST call above inside<br>
main loop, and change the find_primary_node() call to<br>
find_primary_node_repeatedly()<wbr>.<br>
<br>
Attached is the patch to do that (plus change the<br>
search_primary_node_timeout to smaller value in 055.backend_all_down<br>
test. Otherwise, regression timeout is triggered) against master<br>
branch.<br>
<br>
What do you think?<br></blockquote><div><br></div><div><br></div><div>Waoo thanks for catching this, it is a really annoying issue, I think your patch does solve the problem and is the right approach,</div><div>But I was thinking what if we move search for the primary node before starting the child processes. So that we spawn the child processes after finishing all the</div><div>startup rituals? Do you think it will cause some issues?</div><div><br></div><div>Thanks</div><div>Best Regards</div><div>Muhammad Usama</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
--<br>
Tatsuo Ishii<br>
SRA OSS, Inc. Japan<br>
English: <a href="http://www.sraoss.co.jp/index_en.php" rel="noreferrer" target="_blank">http://www.sraoss.co.jp/index_<wbr>en.php</a><br>
Japanese:<a href="http://www.sraoss.co.jp" rel="noreferrer" target="_blank">http://www.sraoss.co.<wbr>jp</a><br>
<br>______________________________<wbr>_________________<br>
pgpool-hackers mailing list<br>
<a href="mailto:pgpool-hackers@pgpool.net">pgpool-hackers@pgpool.net</a><br>
<a href="http://www.pgpool.net/mailman/listinfo/pgpool-hackers" rel="noreferrer" target="_blank">http://www.pgpool.net/mailman/<wbr>listinfo/pgpool-hackers</a><br>
<br></blockquote></div><br></div></div>