From e7e03154ba1e0f6d30b30cbec771a5fc5c27068d Mon Sep 17 00:00:00 2001 From: Chen Ningwei Date: Tue, 6 Jun 2023 14:38:49 +0900 Subject: [PATCH] Feature: Support mutiple directories specification for pcp_socket_dir. --- doc.ja/src/sgml/connection-settings.sgml | 6 +- doc/src/sgml/connection-settings.sgml | 5 +- src/config/pool_config_variables.c | 23 ++++--- src/include/pool_config.h | 3 +- src/main/pgpool_main.c | 69 ++++++++++++------- src/pcp_con/pcp_child.c | 17 ++++- src/sample/pgpool.conf.sample-stream | 2 +- .../regression/tests/035.unix_sockets/test.sh | 42 ++++++++++- src/utils/pool_process_reporting.c | 10 ++- 9 files changed, 132 insertions(+), 45 deletions(-) diff --git a/doc.ja/src/sgml/connection-settings.sgml b/doc.ja/src/sgml/connection-settings.sgml index 22031978d..fd3c3fae2 100644 --- a/doc.ja/src/sgml/connection-settings.sgml +++ b/doc.ja/src/sgml/connection-settings.sgml @@ -295,13 +295,15 @@ PCPプロセスが接続を受け付けるUNIXドメインソケットを置くディレクトリです。 + コンマで区切った複数のディレクトリのリストを設定すると複数のUNIXドメインソケットファイルを作成します。 デフォルト値は/tmpです。 このソケットは、cron によって削除されることがあるので注意してください。 /var/runなどのディレクトリに変更することをお勧めします。 diff --git a/doc/src/sgml/connection-settings.sgml b/doc/src/sgml/connection-settings.sgml index af068ccec..5b0af0a49 100644 --- a/doc/src/sgml/connection-settings.sgml +++ b/doc/src/sgml/connection-settings.sgml @@ -189,8 +189,9 @@ The directory where the UNIX domain socket accepting - connections for PCP process will be created. Default - is /tmp. Be aware that this socket + connections for PCP process will be created. Multiple sockets + can be created by listing multiple directories separated by commas. + Default is /tmp. Be aware that this socket might be deleted by a cron job. We recommend to set this value to /var/run or such directory. diff --git a/src/config/pool_config_variables.c b/src/config/pool_config_variables.c index 9fb53c17e..049cd73d5 100644 --- a/src/config/pool_config_variables.c +++ b/src/config/pool_config_variables.c @@ -862,16 +862,6 @@ static struct config_string ConfigureNamesString[] = NULL, NULL, NULL, NULL }, - { - {"pcp_socket_dir", CFGCXT_INIT, CONNECTION_CONFIG, - "The directory to create the UNIX domain socket for accepting pgpool-II PCP connections.", - CONFIG_VAR_TYPE_STRING, false, 0 - }, - &g_pool_config.pcp_socket_dir, - DEFAULT_SOCKET_DIR, - NULL, NULL, NULL, NULL - }, - { {"wd_ipc_socket_dir", CFGCXT_INIT, CONNECTION_CONFIG, "The directory to create the UNIX domain socket for accepting pgpool-II watchdog IPC connections.", @@ -1414,6 +1404,19 @@ static struct config_string_list ConfigureNamesStringList[] = NULL, NULL, NULL }, + { + {"pcp_socket_dir", CFGCXT_INIT, CONNECTION_CONFIG, + "The directories to create the UNIX domain socket for accepting pgpool-II PCP connections.", + CONFIG_VAR_TYPE_STRING_LIST, false, 0 + }, + &g_pool_config.pcp_socket_dir, + &g_pool_config.num_pcp_socket_directories, + (const char *) default_unix_socket_directories_list, + ",", + false, + NULL, NULL, NULL + }, + { {"read_only_function_list", CFGCXT_RELOAD, CONNECTION_POOL_CONFIG, "list of functions that does not writes to database.", diff --git a/src/include/pool_config.h b/src/include/pool_config.h index 50ffb3389..07c6d7678 100644 --- a/src/include/pool_config.h +++ b/src/include/pool_config.h @@ -226,7 +226,7 @@ typedef struct char *unix_socket_group; /* owner group of pgpool sockets */ int unix_socket_permissions; /* pgpool sockets permissions */ char *wd_ipc_socket_dir; /* watchdog command IPC socket directory */ - char *pcp_socket_dir; /* PCP socket directory */ + char **pcp_socket_dir; /* PCP socket directory */ int num_init_children; /* Maximum number of child to * accept connections */ int min_spare_children; /* Minimum number of idle children */ @@ -417,6 +417,7 @@ typedef struct int num_listen_addresses; /* number of entries in listen_addresses */ int num_pcp_listen_addresses; /* number of entries in pcp_listen_addresses */ int num_unix_socket_directories; /* number of entries in unix_socket_directories */ + int num_pcp_socket_directories; /* number of entries in pcp_socket_dir */ int num_read_only_function_list; /* number of functions in * read_only_function_list */ int num_write_function_list; /* number of functions in diff --git a/src/main/pgpool_main.c b/src/main/pgpool_main.c index 439ebec38..dc2ff1fab 100644 --- a/src/main/pgpool_main.c +++ b/src/main/pgpool_main.c @@ -204,7 +204,7 @@ static void service_child_processes(void); static int select_victim_processes(int *process_info_idxs, int count); static struct sockaddr_un *un_addrs; /* unix domain socket path */ -static struct sockaddr_un pcp_un_addr; /* unix domain socket path for PCP */ +static struct sockaddr_un *pcp_un_addrs; /* unix domain socket path for PCP */ ProcessInfo *process_info = NULL; /* Per child info table on shmem */ volatile User1SignalSlot *user1SignalSlot = NULL; /* User 1 signal slot on * shmem */ @@ -287,9 +287,10 @@ PgpoolMain(bool discard_status, bool clear_memcache_oidmaps) { int num_inet_fds = 0; int num_unix_fds = 0; + int num_pcp_fds = 0; int *unix_fds; int *inet_fds; - int pcp_unix_fd; + int *pcp_unix_fds; int *pcp_inet_fds; int i; char unix_domain_socket_path[UNIXSOCK_PATH_BUFLEN + 1024]; @@ -348,25 +349,39 @@ PgpoolMain(bool discard_status, bool clear_memcache_oidmaps) } /* set unix domain socket path for pgpool PCP communication */ - memset(unix_domain_socket_path, 0, sizeof(unix_domain_socket_path)); - snprintf(unix_domain_socket_path, sizeof(unix_domain_socket_path), "%s/.s.PGSQL.%d", - pool_config->pcp_socket_dir, - pool_config->pcp_port); + for (i = 0; i < pool_config->num_pcp_socket_directories; i++) + { + memset(unix_domain_socket_path, 0, sizeof(unix_domain_socket_path)); + snprintf(unix_domain_socket_path, sizeof(unix_domain_socket_path), "%s/.s.PGSQL.%d", + pool_config->pcp_socket_dir[i], + pool_config->pcp_port); - if (strlen(unix_domain_socket_path) >= UNIXSOCK_PATH_BUFLEN) + if (strlen(unix_domain_socket_path) >= UNIXSOCK_PATH_BUFLEN) + { + ereport(WARNING, + (errmsg("PCP Unix-domain socket path \"%s\" is too long (maximum %d bytes)", + unix_domain_socket_path, + (int) (UNIXSOCK_PATH_BUFLEN - 1)))); + continue; + } + pcp_un_addrs = realloc(pcp_un_addrs, sizeof(struct sockaddr_un) * (num_pcp_fds + 1)); + if (pcp_un_addrs == NULL) + ereport(FATAL, + (errmsg("failed to allocate memory in startup process"))); + + snprintf(pcp_un_addrs[i].sun_path, sizeof(pcp_un_addrs[i].sun_path), "%s", unix_domain_socket_path); + num_pcp_fds++; + } + + if (num_pcp_fds == 0) { ereport(FATAL, - (errmsg("could not create PCP Unix-domain sockets"), - errdetail("PCP Unix-domain socket path \"%s\" is too long (maximum %d bytes)", - unix_domain_socket_path, - (int) (UNIXSOCK_PATH_BUFLEN - 1)))); + (errmsg("could not create any PCP Unix-domain sockets"))); } - snprintf(pcp_un_addr.sun_path, sizeof(pcp_un_addr.sun_path), "%s", unix_domain_socket_path); /* set up signal handlers */ pool_signal(SIGPIPE, SIG_IGN); - /* start the log collector if enabled */ pgpool_logger_pid = SysLogger_Start(); /* @@ -560,19 +575,21 @@ PgpoolMain(bool discard_status, bool clear_memcache_oidmaps) } /* create pcp unix domain socket */ - num_unix_fds = 1; num_inet_fds = 0; - pcp_unix_fd = create_unix_domain_socket(pcp_un_addr, "", 0777); - on_proc_exit(FileUnlink, (Datum) pcp_un_addr.sun_path); - - pcp_fds = malloc(sizeof(int) * (num_unix_fds + 1)); + pcp_unix_fds = create_unix_domain_sockets_by_list(pcp_un_addrs, "", 0777, num_pcp_fds); + for (i = 0; i < num_pcp_fds; i++) + { + on_proc_exit(FileUnlink, (Datum) pcp_un_addrs[i].sun_path); + } + pcp_fds = malloc(sizeof(int) * (num_pcp_fds + 1)); if (pcp_fds == NULL) ereport(FATAL, (errmsg("failed to allocate memory in startup process"))); - pcp_fds[0] = pcp_unix_fd; - pcp_fds[num_unix_fds] = -1; + memcpy(pcp_fds, pcp_unix_fds, sizeof(int) * num_pcp_fds); + pcp_fds[num_pcp_fds] = -1; + free(pcp_unix_fds); /* create inet domain socket if any */ pcp_inet_fds = create_inet_domain_sockets_by_list(pool_config->pcp_listen_addresses, @@ -581,13 +598,13 @@ PgpoolMain(bool discard_status, bool clear_memcache_oidmaps) if (num_inet_fds > 0) { - pcp_fds = realloc(pcp_fds, sizeof(int) * (num_inet_fds + num_unix_fds + 1)); + pcp_fds = realloc(pcp_fds, sizeof(int) * (num_inet_fds + num_pcp_fds + 1)); if (pcp_fds == NULL) ereport(FATAL, (errmsg("failed to expand memory for pcp_fds"))); - memcpy(&pcp_fds[num_unix_fds], pcp_inet_fds, sizeof(int) * num_inet_fds); - pcp_fds[num_inet_fds + num_unix_fds] = -1; + memcpy(&pcp_fds[num_pcp_fds], pcp_inet_fds, sizeof(int) * num_inet_fds); + pcp_fds[num_inet_fds + num_pcp_fds] = -1; free(pcp_inet_fds); } @@ -4779,8 +4796,8 @@ exec_notice_pcp_child(FAILOVER_CONTEXT *failover_context) /* - * Create UNIX domain sockets by unix_socket_directories, which is an array of - string. The number of elements is + * Create UNIX domain sockets by unix_socket_directories/pcp_socket_dir, + * which is an array of string. The number of elements is * "n_listen_addresses". "port" is the port number. A socket array is returned. * The number of elements in the socket array is "n_sockets". */ @@ -4802,7 +4819,7 @@ create_unix_domain_sockets_by_list(struct sockaddr_un *un_addrs, for (i = 0; i < n_sockets; i++) { ereport(LOG, - (errmsg("unix_socket_directories[%d]: %s", i, un_addrs[i].sun_path))); + (errmsg("create socket files[%d]: %s", i, un_addrs[i].sun_path))); sockets[i] = create_unix_domain_socket(un_addrs[i], group, permissions); } diff --git a/src/pcp_con/pcp_child.c b/src/pcp_con/pcp_child.c index 6c3f87092..363b6bd30 100644 --- a/src/pcp_con/pcp_child.c +++ b/src/pcp_con/pcp_child.c @@ -86,6 +86,7 @@ static void start_pcp_command_processor_process(int port, int *fds); static void pcp_child_will_die(int code, Datum arg); static void pcp_kill_all_children(int sig); static void reaper(void); +static bool pcp_unix_fds_not_isset(int *fds, int num_pcp_fds, fd_set* opt); #define CHECK_RESTART_REQUEST \ @@ -250,7 +251,7 @@ pcp_do_accept(int *fds) * Set no delay if AF_INET socket. Not sure if this is really necessary * but PostgreSQL does this. */ - if (!FD_ISSET(fds[0], &rmask)) /* fds[0] is UNIX domain socket */ + if (pcp_unix_fds_not_isset(fds, pool_config->num_pcp_socket_directories, &rmask)) /* fds are UNIX domain socket for pcp process */ { int on; @@ -269,6 +270,20 @@ pcp_do_accept(int *fds) return afd; } +static bool +pcp_unix_fds_not_isset(int* fds, int num_pcp_fds, fd_set* opt) +{ + int i; + for (i = 0; i < num_pcp_fds; i++) + { + if (!FD_ISSET(fds[i], opt)) + continue; + + return false; + } + return true; +} + /* * forks a new pcp worker child */ diff --git a/src/sample/pgpool.conf.sample-stream b/src/sample/pgpool.conf.sample-stream index 072ee7d5e..f4397d2ff 100644 --- a/src/sample/pgpool.conf.sample-stream +++ b/src/sample/pgpool.conf.sample-stream @@ -73,7 +73,7 @@ backend_clustering_mode = 'streaming_replication' # Port number for pcp # (change requires restart) #pcp_socket_dir = '/tmp' - # Unix domain socket path for pcp + # Unix domain socket path(s) for pcp # The Debian package defaults to # /var/run/postgresql # (change requires restart) diff --git a/src/test/regression/tests/035.unix_sockets/test.sh b/src/test/regression/tests/035.unix_sockets/test.sh index 4d36a0725..f59356db2 100755 --- a/src/test/regression/tests/035.unix_sockets/test.sh +++ b/src/test/regression/tests/035.unix_sockets/test.sh @@ -1,12 +1,15 @@ #!/usr/bin/env bash #------------------------------------------------------------------- -# test script for unix_socket_directories, unix_socket_group and unix_socket_permissions. +# test script for unix_socket_directories, unix_socket_group, unix_socket_permissions +# and pcp_socket_dir. # unix_socket_group test works if UNIX_SOCK_GROUP exists and the user running # this test belongs to it. Therefore, we usually comment out it. # source $TESTLIBS TESTDIR=testdir PSQL=$PGBIN/psql +PGPOOLBIN=$PGPOOL_INSTALL_DIR/bin +PCP_PORT=11001 for mode in s do @@ -22,24 +25,31 @@ do source ./bashrc.ports dir=`pwd` UNIX_SOCK_PATH1=/tmp + PCP_SOCK_PATH1=/tmp if [ -d $HOME/tmp ];then UNIX_SOCK_PATH2=$HOME/tmp + PCP_SOCK_PATH2=$HOME/tmp else UNIX_SOCK_PATH2=$HOME + PCP_SOCK_PATH2=$HOME fi UNIX_SOCK_FILE=.s.PGSQL.$PGPOOL_PORT + PCP_SOCK_FILE=.s.PGSQL.$PCP_PORT UNIX_SOCK_GROUP=wheel USER=`whoami` echo "unix_socket_directories = '$UNIX_SOCK_PATH1,$UNIX_SOCK_PATH2'" >> etc/pgpool.conf + echo "pcp_socket_dir = '$PCP_SOCK_PATH1,$PCP_SOCK_PATH2'" >> etc/pgpool.conf <> etc/pgpool.conf COMMENT_OUT echo "unix_socket_permissions = 0770" >> etc/pgpool.conf + sed -i 's/localhost/*/g' ./pcppass ./startall export PGPORT=$PGPOOL_PORT + export PCPPASSFILE=./pcppass wait_for_pgpool_startup @@ -108,6 +118,36 @@ COMMENT_OUT echo ok: socket files permission echo ok: unix_socket_directories and related parameters are working. + echo check: multiple unix domain sokets for pcp connections + if [ ! -e $PCP_SOCK_PATH1/$PCP_SOCK_FILE ]; then + echo "fail: not exist $PCP_SOCK_PATH1/$PCP_SOCK_FILE" + ./shutdownall + exit 1 + fi + + if [ ! -e $PCP_SOCK_PATH2/$PCP_SOCK_FILE ]; then + echo "fail: not exist $PCP_SOCK_PATH2/$PCP_SOCK_FILE" + ./shutdownall + exit 1 + fi + echo ok: multiple unix domain sockets for pcp connections + + echo check: pcp command connection to unix domain sockets + res=`$PGPOOLBIN/pcp_node_info -h $PCP_SOCK_PATH1 -w -p $PCP_PORT|egrep "primary|standby"|wc -l` + if [ $res -ne 2 ]; then + echo "fail: cannot connect to $PCP_SOCK_PATH1/$PCP_SOCK_FILE" + ./shutdownall + exit 1 + fi + + res=`$PGPOOLBIN/pcp_node_info -h $PCP_SOCK_PATH2 -w -p $PCP_PORT|egrep "primary|standby"|wc -l` + if [ $res -ne 2 ]; then + echo "fail: cannot connect to $PCP_SOCK_PATH2/$PCP_SOCK_FILE" + ./shutdownall + exit 1 + fi + echo ok: pcp commmand connection to unix domain sockets + ./shutdownall done diff --git a/src/utils/pool_process_reporting.c b/src/utils/pool_process_reporting.c index f5c4f7918..bd9323fe3 100644 --- a/src/utils/pool_process_reporting.c +++ b/src/utils/pool_process_reporting.c @@ -251,7 +251,15 @@ get_config(int *nrows) i++; StrNCpy(status[i].name, "pcp_socket_dir", POOLCONFIG_MAXNAMELEN); - snprintf(status[i].value, POOLCONFIG_MAXVALLEN, "%s", pool_config->pcp_socket_dir); + *(status[i].value) = '\0'; + for (j = 0; j < pool_config->num_pcp_socket_directories; j++) + { + len = POOLCONFIG_MAXVALLEN - strlen(status[i].value); + strncat(status[i].value, pool_config->pcp_socket_dir[j], len); + len = POOLCONFIG_MAXVALLEN - strlen(status[i].value); + if (j != pool_config->num_pcp_socket_directories - 1) + strncat(status[i].value, ",", len); + } StrNCpy(status[i].desc, "PCP socket directory", POOLCONFIG_MAXDESCLEN); i++; -- 2.39.5