<div dir="ltr">Hi Ishii-San<div><br></div><div>Thanks for looking into the patch and valuable feedback.</div><div><br></div><div>Please find the V2 of the patch that fixes the mentioned issues</div><div>except the extended-query-test produces, that is failing because the expected</div><div>output was generated by some different version of PostgreSQL and now the</div><div>line number in the expected notice messages are coming out from different</div><div>line numbers of the PG code.</div><div><br></div><div>So we may need to fix the test cases and I don&#39;t find any functionality issues</div><div>or any issue that is caused by the patch</div><div><br></div><div>See the differences in the result and expected outputs</div><div>





<p class="gmail-p1" style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:14px;line-height:normal;font-family:&quot;Meslo LG L DZ for Powerline&quot;;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><br></span></p><p class="gmail-p1" style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:14px;line-height:normal;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><font face="monospace">[highgo@6132f8b15f3f extended-query-test]$ diff results/disable-load-balance-off-black-function.data expected/disable-load-balance-off-black-function.data<span class="gmail-Apple-converted-space" style=""> </span></font></span></p>
<p class="gmail-p1" style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:14px;line-height:normal;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><font face="monospace">2c2</font></span></p>
<p class="gmail-p1" style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:14px;line-height:normal;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><font face="monospace">&lt; &lt;= BE NoticeResponse(S NOTICE V NOTICE C 00000 M function f1() does not exist, skipping F dropcmds.c L <b>491</b> R does_not_exist_skipping )</font></span></p>
<p class="gmail-p1" style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:14px;line-height:normal;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><font face="monospace">---</font></span></p>
<p class="gmail-p1" style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:14px;line-height:normal;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><font face="monospace">&gt; &lt;= BE NoticeResponse(S NOTICE V NOTICE C 00000 M function f1() does not exist, skipping F dropcmds.c L <b>483</b> R does_not_exist_skipping )</font></span></p>
<p class="gmail-p1" style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:14px;line-height:normal;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><font face="monospace"><br></font></span></p><p class="gmail-p1" style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:14px;line-height:normal;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><font face="monospace">[highgo@6132f8b15f3f extended-query-test]$ diff results/disable-load-balance-off-black-function.data expected/disable-load-balance-trans-black-function.data</font></span></p>
<p class="gmail-p1" style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:14px;line-height:normal;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><font face="monospace">2c2</font></span></p>
<p class="gmail-p1" style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:14px;line-height:normal;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><font face="monospace">&lt; &lt;= BE NoticeResponse(S NOTICE V NOTICE C 00000 M function f1() does not exist, skipping F dropcmds.c L <b>491</b> R does_not_exist_skipping )</font></span></p>
<p class="gmail-p1" style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:14px;line-height:normal;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><font face="monospace">---</font></span></p>
<p class="gmail-p1" style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:14px;line-height:normal;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><font face="monospace">&gt; &lt;= BE NoticeResponse(S NOTICE V NOTICE C 00000 M function f1() does not exist, skipping F dropcmds.c L <b>483</b> R does_not_exist_skipping )</font></span></p><p class="gmail-p1" style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:14px;line-height:normal;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><font face="monospace"><br></font></span></p><p class="gmail-p1" style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:14px;line-height:normal;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><font face="monospace"><br></font></span></p><p class="gmail-p1" style="margin:0px;font-style:normal;font-variant:normal;font-weight:normal;font-stretch:normal;font-size:14px;line-height:normal;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><font face="monospace">[highgo@6132f8b15f3f extended-query-test]$ diff results/disable-load-balance-default.data expected/disable-load-balance-default.data</font></span></p><p class="gmail-p1" style="margin:0px;font-style:normal;font-variant:normal;font-weight:normal;font-stretch:normal;font-size:14px;line-height:normal;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><font face="monospace">2c2</font></span></p><p class="gmail-p1" style="margin:0px;font-style:normal;font-variant:normal;font-stretch:normal;font-size:14px;line-height:normal;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><font face="monospace" style="">&lt; &lt;= BE NoticeResponse(S NOTICE V NOTICE C 00000 M table &quot;pgproto_test1&quot; does not exist, skipping F tablecmds.c L <b>1186</b> R DropErrorMsgNonExistent )</font></span></p><p class="gmail-p1" style="margin:0px;font-style:normal;font-variant:normal;font-weight:normal;font-stretch:normal;font-size:14px;line-height:normal;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><font face="monospace">---</font></span></p><p class="gmail-p1" style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:14px;line-height:normal;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><font face="monospace">









</font></span></p><p class="gmail-p1" style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:14px;line-height:normal;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><font face="monospace">&gt; &lt;= BE NoticeResponse(S NOTICE V NOTICE C 00000 M table &quot;pgproto_test1&quot; does not exist, skipping F tablecmds.c L <b>914</b> R DropErrorMsgNonExistent )</font></span></p><p class="gmail-p1" style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:14px;line-height:normal;font-family:&quot;Meslo LG L DZ for Powerline&quot;;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><br></span></p><p class="gmail-p1" style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:14px;line-height:normal;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><font face="arial, sans-serif">Thanks</font></span></p><p class="gmail-p1" style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:14px;line-height:normal;color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><font face="arial, sans-serif">Best regards</font></span></p><div>...</div><p class="gmail-p1" style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:14px;line-height:normal;color:rgb(0,0,0)"><font color="#888888" style="font-size:small"></font></p><div><div style="color:rgb(0,0,0)"><div id="gmail-m_134367682178226378gmail-Zm-_Id_-Sgn"><div><span style="color:rgb(68,68,68)">Muhammad Usama<br></span></div><div><span><span style="font-family:Lato"><span style="font-size:14px"><span style="color:rgb(68,68,68)">Highgo Software (Canada/China/Pakistan)</span><span><span style="color:rgb(68,68,68)"> </span></span></span></span></span><span style="color:rgb(68,68,68)"><br></span></div><div><span><span style="font-family:Lato"><span style="font-size:14px"><span style="color:rgb(68,68,68)">URL :</span><span><span style="color:rgb(68,68,68)"> </span></span></span></span></span><a href="http://www.highgo.ca/" target="_blank" style="color:rgb(89,143,222);font-family:Lato;font-size:14px"><span style="color:rgb(68,68,68)">http://www.highgo.ca</span></a><span><span style="font-family:Lato"><span style="font-size:14px"><span><span style="color:rgb(68,68,68)"> </span></span></span></span></span><span style="color:rgb(68,68,68)"><br></span></div><div><span><span style="font-family:Lato"><span style="font-size:14px"><span style="color:rgb(68,68,68)">ADDR: 10318 WHALLEY BLVD, Surrey, BC</span><span><span style="color:rgb(68,68,68)"> </span></span></span></span></span></div></div></div></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, May 2, 2020 at 6:28 PM Tatsuo Ishii &lt;<a href="mailto:ishii@sraoss.co.jp">ishii@sraoss.co.jp</a>&gt; 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>
Here is a report on quick tests with your patch.<br>
<br>
1) git apply produced following some warnings.<br>
<br>
/home/t-ishii/pcp_stop_cluster.diff:1984: trailing whitespace.<br>
<br>
/home/t-ishii/pcp_stop_cluster.diff:3919: trailing whitespace.<br>
<br>
/home/t-ishii/pcp_stop_cluster.diff:2882: new blank line at EOF.<br>
+<br>
/home/t-ishii/pcp_stop_cluster.diff:5068: new blank line at EOF.<br>
+<br>
warning: 4 lines add whitespace errors.<br>
<br>
2) compiling produced following warnings.<br>
<br>
wd_escalation.c: In function ‘fork_escalation_process’:<br>
wd_escalation.c:79:2: warning: ‘sigsetmask’ is deprecated [-Wdeprecated-declarations]<br>
  POOL_SETMASK(&amp;UnBlockSig);<br>
  ^~~~~~~~~~~~<br>
In file included from ../../src/include/utils/pool_signal.h:33:0,<br>
                 from wd_escalation.c:34:<br>
/usr/include/signal.h:173:12: note: declared here<br>
 extern int sigsetmask (int __mask) __THROW __attribute_deprecated__;<br>
            ^~~~~~~~~~<br>
wd_escalation.c: In function ‘fork_plunging_process’:<br>
wd_escalation.c:168:2: warning: ‘sigsetmask’ is deprecated [-Wdeprecated-declarations]<br>
  POOL_SETMASK(&amp;UnBlockSig);<br>
  ^~~~~~~~~~~~<br>
In file included from ../../src/include/utils/pool_signal.h:33:0,<br>
                 from wd_escalation.c:34:<br>
/usr/include/signal.h:173:12: note: declared here<br>
 extern int sigsetmask (int __mask) __THROW __attribute_deprecated__;<br>
            ^~~~~~~~~~<br>
main/main.c: In function ‘main’:<br>
main/main.c:312:3: warning: implicit declaration of function ‘SSL_ServerSide_init’; did you mean ‘SSL_in_init’? [-Wimplicit-function-declaration]<br>
   SSL_ServerSide_init();<br>
   ^~~~~~~~~~~~~~~~~~~<br>
   SSL_in_init<br>
<br>
3) There is a one failure in the regression test.<br>
<br>
testing 010.rewrite_timestamp...failed.<br>
<br>
4) extedend-query-test produces 10 errors.<br>
<br>
t-ishii$ ./test.sh <br>
creating pgpool-II temporary installation ...<br>
moving pgpool_setup to temporary installation path ...<br>
using pgpool-II at /home/t-ishii/work/Pgpool-II/current/pgpool2/src/test/extended-query-test/temp/installed<br>
*************************<br>
REGRESSION MODE : install<br>
PGPOOL-II       : /home/t-ishii/work/Pgpool-II/current/pgpool2/src/test/extended-query-test/temp/installed<br>
PostgreSQL bin  : /usr/local/pgsql/bin<br>
pgbench         : /usr/local/pgsql/bin/pgbench<br>
PostgreSQL jdbc : /usr/local/pgsql/share/postgresql-9.2-1003.jdbc4.jar<br>
*************************<br>
*** creating test database with 2 nodes...done.<br>
statement_level_load_balance = off<br>
testing aborted-transaction.data ... ok.<br>
testing bug370-sql-error-followed-by-select.data ... ok.<br>
testing disable-load-balance-always-black-function.data ... ok.<br>
testing disable-load-balance-always.data ... ok.<br>
testing disable-load-balance-default-black-function.data ... extra test failed.<br>
testing disable-load-balance-default-simple.data ... ok.<br>
testing disable-load-balance-default.data ... extra test failed.<br>
testing disable-load-balance-off-black-function.data ... extra test failed.<br>
testing disable-load-balance-off.data ... ok.<br>
testing disable-load-balance-simple-black-function.data ... ok.<br>
testing disable-load-balance-simple.data ... ok.<br>
testing disable-load-balance-trans-black-function.data ... extra test failed.<br>
testing disable-load-balance-white-function.data ... ok.<br>
testing disable-load-balance.data ... extra test failed.<br>
testing node_js.data ... ok.<br>
testing parse-before-bind-2.data ... ok.<br>
testing parse-before-bind.data ... ok.<br>
testing query-cache-notrans.data ... ok.<br>
testing query-cache.data ... ok.<br>
testing select-multi-rows.data ... ok.<br>
testing sql-error.data ... ok.<br>
testing unable_to_bind.data ... ok.<br>
*** creating test database with 2 nodes...done.<br>
statement_level_load_balance = on<br>
testing aborted-transaction.data ... ok.<br>
testing bug370-sql-error-followed-by-select.data ... ok.<br>
testing disable-load-balance-always-black-function.data ... ok.<br>
testing disable-load-balance-always.data ... ok.<br>
testing disable-load-balance-default-black-function.data ... extra test failed.<br>
testing disable-load-balance-default-simple.data ... ok.<br>
testing disable-load-balance-default.data ... extra test failed.<br>
testing disable-load-balance-off-black-function.data ... extra test failed.<br>
testing disable-load-balance-off.data ... ok.<br>
testing disable-load-balance-simple-black-function.data ... ok.<br>
testing disable-load-balance-simple.data ... ok.<br>
testing disable-load-balance-trans-black-function.data ... extra test failed.<br>
testing disable-load-balance-white-function.data ... ok.<br>
testing disable-load-balance.data ... extra test failed.<br>
testing node_js.data ... ok.<br>
testing parse-before-bind-2.data ... ok.<br>
testing parse-before-bind.data ... ok.<br>
testing query-cache-notrans.data ... ok.<br>
testing query-cache.data ... ok.<br>
testing select-multi-rows.data ... ok.<br>
testing sql-error.data ... ok.<br>
testing unable_to_bind.data ... ok.<br>
creating test database with 3 nodes...done.<br>
testing statement_level_load_balance.data ... ok.<br>
out of 45 ok: 35 failed: 10 timeout: 0.<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: Muhammad Usama &lt;<a href="mailto:m.usama@gmail.com" target="_blank">m.usama@gmail.com</a>&gt;<br>
Subject: Re: PCP command to interact with Pgpool-II cluster<br>
Date: Fri, 1 May 2020 15:16:50 +0500<br>
Message-ID: &lt;<a href="mailto:CAEJvTzXWkQhWgJkM%2BcW1cCNeZYbaF_kcE7vVuHvWB5SmNnUeZw@mail.gmail.com" target="_blank">CAEJvTzXWkQhWgJkM+cW1cCNeZYbaF_kcE7vVuHvWB5SmNnUeZw@mail.gmail.com</a>&gt;<br>
<br>
&gt; Hi,<br>
&gt; <br>
&gt; While working on the pcp_stop_pgpool command to implement cluster mode,<br>
&gt; I realized that we have a lot of clutter in few source<br>
&gt; files. Especially pool.h include<br>
&gt; file. So I also did some code reorganization and added a bunch of new<br>
&gt; include<br>
&gt; files in the hope to reduce the size of pool.h and also get rid of too many<br>
&gt; global functions.<br>
&gt; <br>
&gt; Along with that, I think there are few other files that need a little<br>
&gt; shredding,<br>
&gt; src/main/pgpool_main.c and src/protocol/child.c are at the top of this<br>
&gt; wanted to be trimmed list.<br>
&gt; So as part of this patch, I have taken a few things related to internal<br>
&gt; commands<br>
&gt; (interface between the child processes can and Pgpool-II main process)  from<br>
&gt; pgpool_main.c file and moved them into new &quot;<br>
&gt; src/main/pgpool_internal_commands.c&quot;<br>
&gt; <br>
&gt; Similarly, src/protocol/child.c had and still has so many functions that do<br>
&gt; not fit<br>
&gt; with the personality of child.c file. So I have moved some of the stuff<br>
&gt; related to<br>
&gt; DB functions info src/protocol/pool_pg_utils.c file.<br>
&gt; <br>
&gt; But I think this is still not enough and we may require another round ( if<br>
&gt; someone<br>
&gt; is willing to work on it) of a source file shredding.<br>
&gt; <br>
&gt; Also, I would like to suggest that we just try to refrain from putting<br>
&gt; everything in pool.h<br>
&gt; and only do that if it is absolutely necessary.<br>
&gt; As pool.h is like a global include for Pgpool-II source and in my opinion<br>
&gt; other than few<br>
&gt; struct definitions, Macros, preprocessor Defines, global variable externs<br>
&gt; like pool_config<br>
&gt; there are very few things (especially functions) that really needed to be<br>
&gt; globally<br>
&gt; scoped in Pgpool-II.<br>
&gt; So I would like to suggest that we should at least think twice before<br>
&gt; adding anything to<br>
&gt; pool.h file and consider adding a new include file or even new source file<br>
&gt; if we<br>
&gt; can&#39;t find an appropriate existing file for putting in the function we just<br>
&gt; worked up.<br>
&gt; <br>
&gt; Other than lots and lots of core reorganization changes the patch implements<br>
&gt; &quot;--scope=cluster/local&quot; option in the pcp_stop_pgpool command and adds a<br>
&gt; proper<br>
&gt; infrastructure in watchdog for broadcasting the custom commands over the<br>
&gt; watchdog<br>
&gt; cluster to make it easier to implement the --scope=cluster for other PCP<br>
&gt; utilities.<br>
&gt; <br>
&gt; Note on implementing the new command to execute over the watchdog network.<br>
&gt; <br>
&gt; Step 1: Define the command name string<br>
&gt; ============<br>
&gt; Define a new string to identify the command name in<br>
&gt; src/include/watchdog/wd_ipc_defines.h<br>
&gt; <br>
&gt; For example<br>
&gt; <br>
&gt; +#define WD_COMMAND_RESTART_CLUSTE       &quot;RESTART_CLUSTER&quot;<br>
&gt; +#define WD_COMMAND_REELECT_MASTER       &quot;REELECT_MASTER&quot;<br>
&gt; +#define WD_COMMAND_SHUTDOWN_CLUSTER     &quot;SHUTDOWN_CLUSTER&quot;<br>
&gt; <br>
&gt; Step 2: send the command to watchdog<br>
&gt; =========<br>
&gt; Construct the argument list using WDExecCommandArg struct<br>
&gt; and just fire   wd_execute_cluster_command() function with your new command<br>
&gt; name<br>
&gt; and argument list.<br>
&gt; See implementation of process_shutown_request() for details<br>
&gt; <br>
&gt; ....<br>
&gt; <br>
&gt; WDExecCommandArg wdExecCommandArg;<br>
&gt; <br>
&gt; <br>
&gt; strncpy(wdExecCommandArg.arg_name, &quot;mode&quot;,<br>
&gt; sizeof(wdExecCommandArg.arg_name) - 1);<br>
&gt; <br>
&gt; snprintf(wdExecCommandArg.arg_value, sizeof(wdExecCommandArg.arg_name) - 1,<br>
&gt; &quot;%c&quot;,mode);<br>
&gt; <br>
&gt; <br>
&gt; if (wd_execute_cluster_command(WD_COMMAND_SHUTDOWN_CLUSTER,1,<br>
&gt; &amp;wdExecCommandArg) != COMMAND_OK)<br>
&gt; <br>
&gt;    ereport(ERROR,<br>
&gt; <br>
&gt;        (errmsg(&quot;PCP: error while processing shutdown cluster request&quot;),<br>
&gt; <br>
&gt;         errdetail(&quot;failed to propogate shutdown command through<br>
&gt; watchdog&quot;)));<br>
&gt; ...<br>
&gt; <br>
&gt; <br>
&gt; Step 3: Implement the executor<br>
&gt; ===========<br>
&gt; *wd_execute_cluster_command_processor*(..) in watchdog.c is a place where a<br>
&gt; remote<br>
&gt; Pgpool-II would receive a command. So Just verify if its the command you<br>
&gt; are implementing<br>
&gt; by comparing the clusterCommand string as it is done for the shutdown<br>
&gt; command<br>
&gt; <br>
&gt; if (strcasecmp(WD_COMMAND_SHUTDOWN_CLUSTER, clusterCommand) == 0)<br>
&gt; {<br>
&gt; ...<br>
&gt; }<br>
&gt; <br>
&gt; <br>
&gt; @Tatsuo Ishii &lt;<a href="mailto:ishii@sraoss.co.jp" target="_blank">ishii@sraoss.co.jp</a>&gt; what are your thoughts on using the<br>
&gt; &quot;--scope=[local | cluster ]&quot; argument<br>
&gt; and also on the code reorganization.<br>
&gt; <br>
&gt; <br>
&gt; Thanks<br>
&gt; Best regards<br>
&gt; ...<br>
&gt; Muhammad Usama<br>
&gt; Highgo Software (Canada/China/Pakistan)<br>
&gt; URL : <a href="http://www.highgo.ca" rel="noreferrer" target="_blank">http://www.highgo.ca</a><br>
&gt; ADDR: 10318 WHALLEY BLVD, Surrey, BC<br>
&gt; <br>
&gt; <br>
&gt; <br>
&gt; <br>
&gt; On Tue, Apr 28, 2020 at 5:30 PM Muhammad Usama &lt;<a href="mailto:m.usama@gmail.com" target="_blank">m.usama@gmail.com</a>&gt; wrote:<br>
&gt; <br>
&gt;&gt;<br>
&gt;&gt;<br>
&gt;&gt; On Thu, Apr 23, 2020 at 2:06 PM Muhammad Usama &lt;<a href="mailto:m.usama@gmail.com" target="_blank">m.usama@gmail.com</a>&gt; wrote:<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt; On Tue, Apr 21, 2020 at 12:33 PM Tatsuo Ishii &lt;<a href="mailto:ishii@sraoss.co.jp" target="_blank">ishii@sraoss.co.jp</a>&gt;<br>
&gt;&gt; wrote:<br>
&gt;&gt; &gt; &gt;<br>
&gt;&gt; &gt; &gt; Usama,<br>
&gt;&gt; &gt; &gt;<br>
&gt;&gt; &gt; &gt; Thank you for the propsal.<br>
&gt;&gt; &gt; &gt;<br>
&gt;&gt; &gt; &gt; &gt; Hi Hackers,<br>
&gt;&gt; &gt; &gt; &gt;<br>
&gt;&gt; &gt; &gt; &gt; When PCP was first added to Pgpool-II it was meant to manage/control<br>
&gt;&gt; &gt; &gt; &gt; the single Pgpool-II instance.<br>
&gt;&gt; &gt; &gt; &gt; And after the addition of watchdog more often than not multiple<br>
&gt;&gt; &gt; &gt; &gt; Pgpool-II nodes are installed the system<br>
&gt;&gt; &gt; &gt; &gt; and there is not much work/thoughts have gone into the enhancements<br>
&gt;&gt; of<br>
&gt;&gt; &gt; &gt; &gt; the PCP utilities to make<br>
&gt;&gt; &gt; &gt; &gt; them work on the Pgpool-II cluster.  So lately I was thinking to<br>
&gt;&gt; &gt; &gt; &gt; enhance/add the PCP utilities to make them<br>
&gt;&gt; &gt; &gt; &gt; more cluster-mode friendly.<br>
&gt;&gt; &gt; &gt; &gt; And for that purpose, I think we would require to add/enhance the<br>
&gt;&gt; &gt; &gt; &gt; following PCP commands.<br>
&gt;&gt; &gt; &gt; &gt;<br>
&gt;&gt; &gt; &gt; &gt; 1- pcp_reload_config<br>
&gt;&gt; &gt; &gt; &gt; ==================<br>
&gt;&gt; &gt; &gt; &gt; The utility to make the whole Pgpool-II cluster to reload the<br>
&gt;&gt; &gt; &gt; &gt; configuration files at once.<br>
&gt;&gt; &gt; &gt; &gt; I think this is required because currently if some installation has<br>
&gt;&gt; &gt; &gt; &gt; let say 3 node Pgpool-II cluster<br>
&gt;&gt; &gt; &gt; &gt; and we want to change one specific configuration parameter ( for<br>
&gt;&gt; &gt; &gt; &gt; example: adding a new backend node)<br>
&gt;&gt; &gt; &gt; &gt; we would require to go on and edit Pgpool configuration files on each<br>
&gt;&gt; &gt; &gt; &gt; node separately and then<br>
&gt;&gt; &gt; &gt; &gt; issue pgpool reload on each node one by one.<br>
&gt;&gt; &gt; &gt; &gt;<br>
&gt;&gt; &gt; &gt; &gt; In my opinion, we need multiple enhancement in this area.<br>
&gt;&gt; &gt; &gt; &gt; We need some mechanisms to let pgpool-II cluster use the centralized<br>
&gt;&gt; &gt; &gt; &gt; configuration<br>
&gt;&gt; &gt; &gt; &gt; ( except watchdog config) or at least a mechanism to push the<br>
&gt;&gt; &gt; &gt; &gt; configuration settings using some<br>
&gt;&gt; &gt; &gt; &gt; new utility like &quot;pcp_load_config&quot; that could take a pgpool.conf file<br>
&gt;&gt; &gt; &gt; &gt; and propagate it to all nodes<br>
&gt;&gt; &gt; &gt; &gt; using watchdog.<br>
&gt;&gt; &gt; &gt;<br>
&gt;&gt; &gt; &gt; Yeah, we definitely need it.<br>
&gt;&gt; &gt; &gt;<br>
&gt;&gt; &gt; &gt; &gt; The centralized configuration part of the problem is a big task and I<br>
&gt;&gt; &gt; &gt; &gt; believe we could take it on after the<br>
&gt;&gt; &gt; &gt; &gt; &quot;simplifying watchdog config&quot; feature that Peng is working on.<br>
&gt;&gt; &gt; &gt; &gt; Meanwhile, I think we can work on pcp_reload_config utility in<br>
&gt;&gt; &gt; &gt; &gt; parallel to make reloading part easier.<br>
&gt;&gt; &gt; &gt;<br>
&gt;&gt; &gt; &gt; +1.<br>
&gt;&gt; &gt; &gt;<br>
&gt;&gt; &gt; &gt; &gt; 2- pcp_restart_pgpool [cluster]<br>
&gt;&gt; &gt; &gt; &gt; ==========================<br>
&gt;&gt; &gt; &gt; &gt; Sometimes, a simple restart solves more problems than any other<br>
&gt;&gt; remedy<br>
&gt;&gt; &gt; &gt; &gt; or debugging.<br>
&gt;&gt; &gt; &gt; &gt; So I think we should have a PCP utility to restart a Pgpool-II node<br>
&gt;&gt; or<br>
&gt;&gt; &gt; &gt; &gt; all node on Pgpool-II cluster.<br>
&gt;&gt; &gt; &gt; &gt; something similar in functionality like &quot;pg_ctl restart&quot; in postgres<br>
&gt;&gt; &gt; &gt;<br>
&gt;&gt; &gt; &gt; +1.<br>
&gt;&gt; &gt; &gt;<br>
&gt;&gt; &gt; &gt; &gt; 3- pcp_watchdog_re-elect<br>
&gt;&gt; &gt; &gt; &gt; ======================<br>
&gt;&gt; &gt; &gt; &gt; What if we want to change the master/coordinator node of the<br>
&gt;&gt; watchdog cluster.<br>
&gt;&gt; &gt; &gt; &gt; There exists no good way for that currently. So I purpose we should<br>
&gt;&gt; &gt; &gt; &gt; have a command to make<br>
&gt;&gt; &gt; &gt; &gt; watchdog re-do the elections. I already had a discussion on this one<br>
&gt;&gt; &gt; &gt; &gt; with Ishii-San sometimes<br>
&gt;&gt; &gt; &gt; &gt; back so I will send a separate proposal for that as well<br>
&gt;&gt; &gt; &gt;<br>
&gt;&gt; &gt; &gt; Looking forward to seeing it.<br>
&gt;&gt; &gt; &gt;<br>
&gt;&gt; &gt; &gt; &gt; 4- pcp_stop_pgpool enhancements<br>
&gt;&gt; &gt; &gt; &gt; ===================<br>
&gt;&gt; &gt; &gt; &gt; pcp_stop_pgpool should be able to stop all the Pgpool-II nodes in<br>
&gt;&gt; &gt; &gt; &gt; Pgpool-II cluster.<br>
&gt;&gt; &gt; &gt; &gt; And for that, I have also cooked up a POC patch which allows the<br>
&gt;&gt; &gt; &gt; &gt; [--all (-a)] option to<br>
&gt;&gt; &gt; &gt; &gt; pcp_stop_pgpool utility and when this --all is specified then the<br>
&gt;&gt; &gt; &gt; &gt; utility stops all the Pgpool-II<br>
&gt;&gt; &gt; &gt; &gt; nodes part of the watchdog cluster.<br>
&gt;&gt; &gt; &gt; &gt; So can you please try the patch and also suggest about the command<br>
&gt;&gt; &gt; &gt; &gt; line option we should<br>
&gt;&gt; &gt; &gt; &gt; use for making the PCP commands interact with cluster rather than the<br>
&gt;&gt; &gt; &gt; &gt; single node.<br>
&gt;&gt; &gt; &gt; &gt; Because I think --all would not be the best choice since it is used<br>
&gt;&gt; by<br>
&gt;&gt; &gt; &gt; &gt; other utilities for a different purpose,<br>
&gt;&gt; &gt; &gt; &gt; and we should come up with new standard command-line switch for all<br>
&gt;&gt; &gt; &gt; &gt; utilities if they want to operate on<br>
&gt;&gt; &gt; &gt; &gt; the cluster rather than on a single node.<br>
&gt;&gt; &gt; &gt;<br>
&gt;&gt; &gt; &gt; +1.<br>
&gt;&gt; &gt; &gt;<br>
&gt;&gt; &gt; &gt; What about:<br>
&gt;&gt; &gt; &gt;<br>
&gt;&gt; &gt; &gt; --cluster, -c   /* affect to whole watchdog cluster */<br>
&gt;&gt; &gt; &gt; --local, -l     /* affect only to local watchdog cluster */<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt; Yes seems like a good option since both -c and -l are currently not<br>
&gt;&gt; &gt; used by any PCP utility.<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt; &gt;<br>
&gt;&gt; &gt; &gt; We actually need --cluster only but for completeness I feel like we&#39;d<br>
&gt;&gt; &gt; &gt; want to have --local as well (it exactly behaves as PCP commands do<br>
&gt;&gt; &gt; &gt; today).<br>
&gt;&gt; &gt; &gt;<br>
&gt;&gt; &gt; &gt; &gt; Or another way would be to leave the pcp_stop_pgpool utility<br>
&gt;&gt; untouched<br>
&gt;&gt; &gt; &gt; &gt; and add a new utility pcp_stop_cluster ?<br>
&gt;&gt; &gt; &gt; &gt;<br>
&gt;&gt; &gt; &gt; &gt; So what are your suggestions and thoughts on this and do you have<br>
&gt;&gt; &gt; &gt; &gt; other functions especially related to<br>
&gt;&gt; &gt; &gt; &gt; cluster mode of pgpool-II that needed controlling/management using<br>
&gt;&gt; PCP system?<br>
&gt;&gt; &gt; &gt;<br>
&gt;&gt; &gt; &gt; I have tested your patch and it worked as expected. Great!<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt; Thanks, I will make the argument changes (-c and -l) and push it.<br>
&gt;&gt;<br>
&gt;&gt;<br>
&gt;&gt; I have been thinking about this further, Instead of introducing -c and -l<br>
&gt;&gt; how about adding new argument scope which can be assigned either local or<br>
&gt;&gt; cluster<br>
&gt;&gt;<br>
&gt;&gt; i.e.<br>
&gt;&gt;<br>
&gt;&gt; --scope=[local | cluster]<br>
&gt;&gt; and the short form would be -s=[l|c]<br>
&gt;&gt;<br>
&gt;&gt; Regards<br>
&gt;&gt; Muhammad Usama<br>
&gt;&gt;<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt; Best regards<br>
&gt;&gt; &gt; Muhammad Usama<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt; &gt;<br>
&gt;&gt; &gt; &gt; Best regards,<br>
&gt;&gt; &gt; &gt; --<br>
&gt;&gt; &gt; &gt; Tatsuo Ishii<br>
&gt;&gt; &gt; &gt; SRA OSS, Inc. Japan<br>
&gt;&gt; &gt; &gt; 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>
&gt;&gt; &gt; &gt; Japanese:<a href="http://www.sraoss.co.jp" rel="noreferrer" target="_blank">http://www.sraoss.co.jp</a><br>
&gt;&gt;<br>
</blockquote></div>