<div dir="ltr">Hi Ishii-San<div><br></div><div>As discussed over the slack. I have cooked up a POC patch for implementing the</div><div>follow_primary locking over the watchdog channel.</div><div><br></div><div>The idea is just before executing the follow_primary during the failover process</div><div>we just direct all standby watchdog nodes to acquire the same lock on their respective</div><div>nodes, so that they stop the false primary detection during the period when the</div><div>follow_primary <span style="background-color:transparent">is being executed on the watchdog coordinator node.</span></div><div><br></div><div>Moreover to keep the watchdog process blocked on waiting for the lock I have introduced</div><div>the pending remote lock mechanism, so that remote locks can be acquired in the background</div><div>after the completion of <span style="background-color:transparent">the inflight replication checks.</span></div><div><br></div><div>Finally I have removed the REQ_DETAIL_CONFIRMED flag from degenerate_backend_set()</div><div>request that gets issued to detach the false primary, That means all quorum and consensus rules</div><div>will needed to be satisfied for the detach to happen.</div><div><br></div><div>I haven't done a rigorous testing or regression with the patch and sharing the initial version with you</div><div>to <span style="background-color:transparent">get your consensus on the basic idea and design.</span></div><div><span style="background-color:transparent"><br></span></div><div><span style="background-color:transparent">Can you kindly take a look if you agree </span><span style="background-color:transparent">with the approach.</span></div><div><br></div><div>Thanks</div><div>Best regards</div><div>Muhammad Usama</div><div><br></div><div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, May 7, 2021 at 9:47 AM Tatsuo Ishii <<a href="mailto:ishii@sraoss.co.jp">ishii@sraoss.co.jp</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">I am going to commit/push the patches to master down to 4.0 stable<br>
(detach_false_primary was introduced in 4.0) branches if there's no<br>
objection.<br>
<br>
Best regards,<br>
--<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_en.php</a><br>
Japanese:<a href="http://www.sraoss.co.jp" rel="noreferrer" target="_blank">http://www.sraoss.co.jp</a><br>
<br>
From: Tatsuo Ishii <<a href="mailto:ishii@sraoss.co.jp" target="_blank">ishii@sraoss.co.jp</a>><br>
Subject: [pgpool-hackers: 3893] Re: Problem with detach_false_primary/follow_primary_command<br>
Date: Tue, 04 May 2021 13:09:23 +0900 (JST)<br>
Message-ID: <<a href="mailto:20210504.130923.644768896074013686.t-ishii@gmail.com" target="_blank">20210504.130923.644768896074013686.t-ishii@gmail.com</a>><br>
<br>
> In the previous mail I have explained the problem and proposed a patch<br>
> for the issue.<br>
> <br>
> However the original reporter also said the problem will occur in more<br>
> complex way if watchdog is enabled.<br>
> <br>
> <a href="https://www.pgpool.net/pipermail/pgpool-general/2021-April/007590.html" rel="noreferrer" target="_blank">https://www.pgpool.net/pipermail/pgpool-general/2021-April/007590.html</a><br>
> <br>
> In summary it seems multiple pgpool nodes perform detach_false_primary<br>
> concurrently and this is the cause of the problem. I think there's no<br>
> reason to perform detach_false_primary in multiple pgpool nodes<br>
> concurrently. Rather we should perform detach_false_primary only on<br>
> the leader node. If this is correct, we also should not perform<br>
> detach_false_primary if the quorum is absent because there's no leader<br>
> if the quorum is absent. Attached is the patch to introduce the check<br>
> in addition to the v2 patch.<br>
> <br>
> I would like to hear opinion from other pgpool developers on that<br>
> whether we should apply the v3 patch to existing branches. I am asking<br>
> because currently we perform detach_false_primary even if the quorum<br>
> is absent and the change may be "change of user visible behavior"<br>
> which we usually avoid on stable branches. However the current<br>
> detach_false_primary apparently does not work on the environment where<br>
> watchdog is enabled, I think patching to back branches are exceptionally<br>
> reasonable choice.<br>
> <br>
> Also I have added the regression test patch.<br>
> <br>
>> In the posting:<br>
>> <br>
>> [pgpool-general: 7525] Strange behavior on switchover with detach_false_primary enabled<br>
>> <br>
>> it is reported that detach_false_primary and follow_primary_command<br>
>> could conflict each other and pgpool goes into unwanted state. We can<br>
>> reproduce the issue by using pgpool_setup to create 3 node<br>
>> configuration.<br>
>> <br>
>> $ pgpool_setup -n 3<br>
>> <br>
>> echo "detach_false_primary" >> etc/pgpool.conf<br>
>> echo "sr_check_period = 1" >> etc/pgpool.conf<br>
>> <br>
>> The latter may not be mandatory but making the streaming replication<br>
>> check frequently will reliably reproduce the problem because<br>
>> detach_false_primary is executed in the streaming replication check<br>
>> process.<br>
>> <br>
>> The initial state is as follows:<br>
>> <br>
>> psql -p 11000 -c "show pool_nodes" test<br>
>> node_id | hostname | port | status | pg_status | lb_weight | role | pg_role | select_cnt | load_balance_node | replication_delay | replication_state | replication_sync_state | last_status_change <br>
>> ---------+----------+-------+--------+-----------+-----------+---------+---------+------------+-------------------+-------------------+-------------------+------------------------+---------------------<br>
>> 0 | /tmp | 11002 | up | up | 0.333333 | primary | primary | 0 | true | 0 | | | 2021-05-04 11:12:01<br>
>> 1 | /tmp | 11003 | up | up | 0.333333 | standby | standby | 0 | false | 0 | streaming | async | 2021-05-04 11:12:01<br>
>> 2 | /tmp | 11004 | up | up | 0.333333 | standby | standby | 0 | false | 0 | streaming | async | 2021-05-04 11:12:01<br>
>> (3 rows)<br>
>> <br>
>> Execute pcp_detatch_node against node 0.<br>
>> <br>
>> $ pcp_detach_node -w -p 11001 0<br>
>> <br>
>> This will let the primary be in down status and this will promote node 1.<br>
>> <br>
>> 2021-05-04 12:12:14: pcp_child pid 31449: LOG: received degenerate backend request for node_id: 0 from pid [31449]<br>
>> 2021-05-04 12:12:14: main pid 31221: LOG: Pgpool-II parent process has received failover request<br>
>> 2021-05-04 12:12:14: main pid 31221: LOG: starting degeneration. shutdown host /tmp(11002)<br>
>> 2021-05-04 12:12:14: pcp_main pid 31260: LOG: PCP process with pid: 31449 exit with SUCCESS.<br>
>> 2021-05-04 12:12:14: pcp_main pid 31260: LOG: PCP process with pid: 31449 exits with status 0<br>
>> 2021-05-04 12:12:14: main pid 31221: LOG: Restart all children<br>
>> 2021-05-04 12:12:14: main pid 31221: LOG: execute command: /home/t-ishii/work/Pgpool-II/current/x/etc/failover.sh 0 /tmp 11002 /home/t-ishii/work/Pgpool-II/current/x/data0 1 0 /tmp 0 11003 /home/t-ishii/work/Pgpool-II/current/x/data1<br>
>> <br>
>> However detach_false_primary found that the just promoted node 1 is<br>
>> not good because it does not have any follower standby node because<br>
>> follow_primary_command did not completed yet.<br>
>> <br>
>> 2021-05-04 12:12:14: sr_check_worker pid 31261: LOG: verify_backend_node_status: primary 1 does not connect to standby 2<br>
>> 2021-05-04 12:12:14: sr_check_worker pid 31261: LOG: verify_backend_node_status: primary 1 owns only 0 standbys out of 1<br>
>> 2021-05-04 12:12:14: sr_check_worker pid 31261: LOG: pgpool_worker_child: invalid node found 1<br>
>> <br>
>> And detach_false_primary sent failover request for node 1.<br>
>> <br>
>> 2021-05-04 12:12:14: sr_check_worker pid 31261: LOG: received degenerate backend request for node_id: 1 from pid [31261]<br>
>> <br>
>> Moreover every 1 second detach_false_primary tries to detach node 1.<br>
>> <br>
>> 2021-05-04 12:12:15: sr_check_worker pid 31261: LOG: verify_backend_node_status: primary 1 does not connect to standby 2<br>
>> 2021-05-04 12:12:15: sr_check_worker pid 31261: LOG: verify_backend_node_status: primary 1 owns only 0 standbys out of 1<br>
>> 2021-05-04 12:12:15: sr_check_worker pid 31261: LOG: pgpool_worker_child: invalid node found 1<br>
>> 2021-05-04 12:12:15: sr_check_worker pid 31261: LOG: received degenerate backend request for node_id: 1 from pid [31261]<br>
>> <br>
>> The confuses the whole follow_primary_command and in the end we have:<br>
>> <br>
>> psql -p 11000 -c "show pool_nodes" test<br>
>> node_id | hostname | port | status | pg_status | lb_weight | role | pg_role | select_cnt | load_balance_node | replication_delay | replication_state | replication_sync_state | last_status_change <br>
>> ---------+----------+-------+--------+-----------+-----------+---------+---------+------------+-------------------+-------------------+-------------------+------------------------+---------------------<br>
>> 0 | /tmp | 11002 | down | down | 0.333333 | standby | unknown | 0 | false | 0 | | | 2021-05-04 12:12:16<br>
>> 1 | /tmp | 11003 | up | up | 0.333333 | standby | standby | 0 | false | 0 | | | 2021-05-04 12:22:28<br>
>> 2 | /tmp | 11004 | up | up | 0.333333 | standby | standby | 0 | true | 0 | | | 2021-05-04 12:22:28<br>
>> (3 rows)<br>
>> <br>
>> Of course this is totally unwanted result.<br>
>> <br>
>> I think the root cause of the problem is, detach_false_primary and<br>
>> follow_primary_command are allowed to run concurrently. To solve the<br>
>> problem we need to have a lock so that if detach_false_primary already<br>
>> runs, follow_primary_command should wait for it's completion or vice<br>
>> versa.<br>
>> <br>
>> For this purpose I propose attached patch<br>
>> detach_false_primary_v2.diff. In the patch new function<br>
>> pool_acquire_follow_primary_lock(bool block) and<br>
>> pool_release_follow_primary_lock(void) are introduced. They are<br>
>> responsible for acquiring or releasing the lock. There are 3 places<br>
>> where those functions are used:<br>
>> <br>
>> 1) find_primary_node<br>
>> <br>
>> This function is called upon startup and failover in the main pgpool<br>
>> process to find new primary node.<br>
>> <br>
>> 2) failover<br>
>> <br>
>> This function is called in the follow_primary_command subprocess<br>
>> forked off by pgpool main process to execute follow_primary_command<br>
>> script. The lock should be help until all follow_primary_command are<br>
>> completed.<br>
>> <br>
>> 3) streaming replication check<br>
>> <br>
>> Before starting verify_backend_node, which is the work horse of<br>
>> detach_false_primary, the lock must be acquired. If it fails, just<br>
>> skip the streaming replication check cycle.<br>
>> <br>
>> <br>
>> I and the user who made the initial report confirmed that tha patch<br>
>> works well.<br>
>> <br>
>> Unfortunately the story is not the all. However the mail is already<br>
>> too long. I will continue to the next mail.<br>
>> <br>
>> Best regards,<br>
>> --<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_en.php</a><br>
>> Japanese:<a href="http://www.sraoss.co.jp" rel="noreferrer" target="_blank">http://www.sraoss.co.jp</a><br>
_______________________________________________<br>
pgpool-hackers mailing list<br>
<a href="mailto:pgpool-hackers@pgpool.net" target="_blank">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/listinfo/pgpool-hackers</a><br>
</blockquote></div>