<div dir="ltr"><div dir="ltr">Hi Ishii-San<div><br></div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jun 16, 2021 at 3:15 PM 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">Hi Usama,<br>
<br>
Unfortunately the patch did not apply cleanly on current master<br>
branch:<br></blockquote><div><br></div><div>Sorry, Appearently I had'nt created a patch from current master head.</div><div>Attached is the rebaed version</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
$ git apply ~/wd_coordinating_follow_and_detach__primary.patch <br>
error: patch failed: src/include/pool.h:426<br>
error: src/include/pool.h: patch does not apply<br>
<br>
So I have not actually tested the patch but it seems the idea of<br>
locking watchdog level is more robust than my idea (executing false<br>
primary check only on the coordinator node).<br></blockquote><div><br></div><div>Thanks for the confirmation. I have confirmed the regression is fine with the patch</div><div>but I think I need some more testing before I can commit it. </div><div><br></div><div>Best regards</div><div>Muhammad Usama</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<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>
> Hi Ishii-San<br>
> <br>
> As discussed over the slack. I have cooked up a POC patch for implementing<br>
> the<br>
> follow_primary locking over the watchdog channel.<br>
> <br>
> The idea is just before executing the follow_primary during the failover<br>
> process<br>
> we just direct all standby watchdog nodes to acquire the same lock on their<br>
> respective<br>
> nodes, so that they stop the false primary detection during the period when<br>
> the<br>
> follow_primary is being executed on the watchdog coordinator node.<br>
> <br>
> Moreover to keep the watchdog process blocked on waiting for the lock I<br>
> have introduced<br>
> the pending remote lock mechanism, so that remote locks can be acquired in<br>
> the background<br>
> after the completion of the inflight replication checks.<br>
> <br>
> Finally I have removed the REQ_DETAIL_CONFIRMED flag from<br>
> degenerate_backend_set()<br>
> request that gets issued to detach the false primary, That means all quorum<br>
> and consensus rules<br>
> will needed to be satisfied for the detach to happen.<br>
> <br>
> I haven't done a rigorous testing or regression with the patch and<br>
> sharing the initial version with you<br>
> to get your consensus on the basic idea and design.<br>
> <br>
> Can you kindly take a look if you agree with the approach.<br>
> <br>
> Thanks<br>
> Best regards<br>
> Muhammad Usama<br>
> <br>
> <br>
> On Fri, May 7, 2021 at 9:47 AM Tatsuo Ishii <<a href="mailto:ishii@sraoss.co.jp" target="_blank">ishii@sraoss.co.jp</a>> wrote:<br>
> <br>
>> 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<br>
>> 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<br>
>> 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<br>
>> | pg_role | select_cnt | load_balance_node | replication_delay |<br>
>> replication_state | replication_sync_state | last_status_change<br>
>> >><br>
>> ---------+----------+-------+--------+-----------+-----------+---------+---------+------------+-------------------+-------------------+-------------------+------------------------+---------------------<br>
>> >> 0 | /tmp | 11002 | up | up | 0.333333 | primary<br>
>> | primary | 0 | true | 0 |<br>
>> | | 2021-05-04 11:12:01<br>
>> >> 1 | /tmp | 11003 | up | up | 0.333333 | standby<br>
>> | standby | 0 | false | 0 | streaming<br>
>> | async | 2021-05-04 11:12:01<br>
>> >> 2 | /tmp | 11004 | up | up | 0.333333 | standby<br>
>> | standby | 0 | false | 0 | streaming<br>
>> | 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<br>
>> 1.<br>
>> >><br>
>> >> 2021-05-04 12:12:14: pcp_child pid 31449: LOG: received degenerate<br>
>> 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<br>
>> received failover request<br>
>> >> 2021-05-04 12:12:14: main pid 31221: LOG: starting degeneration.<br>
>> shutdown host /tmp(11002)<br>
>> >> 2021-05-04 12:12:14: pcp_main pid 31260: LOG: PCP process with pid:<br>
>> 31449 exit with SUCCESS.<br>
>> >> 2021-05-04 12:12:14: pcp_main pid 31260: LOG: PCP process with pid:<br>
>> 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:<br>
>> /home/t-ishii/work/Pgpool-II/current/x/etc/failover.sh 0 /tmp 11002<br>
>> /home/t-ishii/work/Pgpool-II/current/x/data0 1 0 /tmp 0 11003<br>
>> /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:<br>
>> 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:<br>
>> 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:<br>
>> 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<br>
>> 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:<br>
>> 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:<br>
>> 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:<br>
>> pgpool_worker_child: invalid node found 1<br>
>> >> 2021-05-04 12:12:15: sr_check_worker pid 31261: LOG: received<br>
>> 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<br>
>> | pg_role | select_cnt | load_balance_node | replication_delay |<br>
>> replication_state | replication_sync_state | last_status_change<br>
>> >><br>
>> ---------+----------+-------+--------+-----------+-----------+---------+---------+------------+-------------------+-------------------+-------------------+------------------------+---------------------<br>
>> >> 0 | /tmp | 11002 | down | down | 0.333333 | standby<br>
>> | unknown | 0 | false | 0 |<br>
>> | | 2021-05-04 12:12:16<br>
>> >> 1 | /tmp | 11003 | up | up | 0.333333 | standby<br>
>> | standby | 0 | false | 0 |<br>
>> | | 2021-05-04 12:22:28<br>
>> >> 2 | /tmp | 11004 | up | up | 0.333333 | standby<br>
>> | standby | 0 | true | 0 |<br>
>> | | 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>
>><br>
</blockquote></div></div>