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