[pgpool-hackers: 2402] Re: Problems with the relative paths in daemon mode
Muhammad Usama
m.usama at gmail.com
Tue Jun 20 03:14:32 JST 2017
Hi Ishii-San
Can you please have a look at the attached patch.
The reason I want your consent before committing it is because apart from
using the absolute paths for pgpool.conf, pcp.conf and pool_hba.conf files,
I have also modified the behaviour of pid_file_name configuration parameter.
Now when the relative path is specified for the pid file in pgpool.conf
file that relative path is calculated with reference to the location of
pgpool.conf file.
For example:
if pgpool.conf file sets pid_file_name parameter as
pid_file_name = 'somedir/pgpool.pid'
And pgpool.conf file is located in the "/etc/configurations/" directory
than the Pgpool-II will create the pgpool.pid file
in "/etc/configurations/somedir" directory
While if absolute path is specified than it will work as it works before
pid_file_name = '/etc/mydir/pgpool.pid' # this will create the pgpool.pid
in /etc/mydir/ directory
I have done this change because I think it will make the configuration of
pid_file_name more intuitive and less confusing.
P.S The documentation changes are also part of the patch.
Thoughts and comments?
Thanks
Best regards
Muhammad Usama
On Wed, Jun 14, 2017 at 3:27 AM, Tatsuo Ishii <ishii at sraoss.co.jp> wrote:
> Good catch! Please go ahead.
>
> Best regards,
> --
> Tatsuo Ishii
> SRA OSS, Inc. Japan
> English: http://www.sraoss.co.jp/index_en.php
> Japanese:http://www.sraoss.co.jp
>
> > Hi
> >
> > Pgpool-II does not properly handle the relative paths especially when the
> > pgpool.conf file is explicitly specified using the command line (-f
> switch)
> > while its patch is given relative to the current directory.
> >
> > For example:
> >
> > Suppose we invoke pgpool-II from "/home/work/installed/pgpool/bin"
> directory
> >
> > $ pwd
> > /home/work//installed/pgpool/bin
> > $ ./pgpool -f simple_conf/pgpool.conf
> >
> > Now we are expecting here is that the Pgpool-II will look for pgpool.conf
> > file in the
> > /home/work/installed/pgpool/bin/simple_conf/ directory, which it does
> > correctly :-), and also latter in the startup phase when it will require
> > the pool_passwd file it will look in the same (
> > /home/work/installed/pgpool/bin/simple_conf/)directory.
> > So in the code what Pgpool-II does is, it take out the directory portion
> > from the command line option provided to it for specifying the
> pgpool.conf
> > file and look for the pool_passwd file in that directory. which in this
> > case is just simple_conf/
> >
> > Now the problem here is since the pgpool.conf path used in the command
> line
> > option was a relative path, so it will only be able to go to the correct
> > directory location if the current directory of Pgpool-II remains the same
> > from where it was invoked from.
> >
> > And since we do not change the current directory when Pgpool-II is
> started
> > with "-n" switch ( non daemon mode) so everything works as expected in
> that
> > mode.
> > But when the Pgpool-II is started in the daemon mode the daemonize()
> > function changes the current directory to "/"
> >
> > main.c:380 if(chdir("/"))
> >
> > and after that all relative path becomes invalid and Pgpool-II will start
> > looking for files in invalid or non-existent location. which as per our
> > example in daemon mode it will try to find the pool_passwd file in
> >
> > /simple_conf/pool_passed
> >
> > instead of
> >
> > /home/work//installed/pgpool/bin/simple_conf/pool_passwd
> > The solution to this is to convert the relative path of the pgpool.conf
> to
> > the absolute path at the startup and later use that absolute path for all
> > other path calculations.
> >
> > If you do not have any reservation, than I will start working on the fix.
> >
> > Thanks
> > Best Regards
> > Muhammad Usama
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.sraoss.jp/pipermail/pgpool-hackers/attachments/20170619/9c00b9a1/attachment-0001.html>
-------------- next part --------------
diff --git a/doc/src/sgml/misc-config.sgml b/doc/src/sgml/misc-config.sgml
index e155b4fa..230736eb 100644
--- a/doc/src/sgml/misc-config.sgml
+++ b/doc/src/sgml/misc-config.sgml
@@ -131,6 +131,8 @@
<para>
Specifies the full path to a file to store the <productname>Pgpool-II
</productname> process id.
+ The pid_file_name path can be specified as relative to the
+ location of pgpool.conf file or as an absolute path
Default is <literal>"/var/run/pgpool/pgpool.pid"</literal>.
</para>
diff --git a/src/include/utils/pool_path.h b/src/include/utils/pool_path.h
index 3919bd14..e40830c7 100644
--- a/src/include/utils/pool_path.h
+++ b/src/include/utils/pool_path.h
@@ -51,5 +51,7 @@ extern void canonicalize_path(char *path);
extern char *last_dir_separator(const char *filename);
extern bool get_home_directory(char *buf, int bufsize);
extern bool get_os_username(char *buf, int bufsize);
+extern char *get_current_working_dir(void);
+extern char *make_absolute_path(const char *path, const char* base_dir);
#endif /* POOL_PATH_H */
diff --git a/src/main/main.c b/src/main/main.c
index 5aa2c2fb..9dd0cc30 100644
--- a/src/main/main.c
+++ b/src/main/main.c
@@ -38,6 +38,7 @@
#include "utils/elog.h"
#include "utils/palloc.h"
#include "utils/memutils.h"
+#include "utils/pool_path.h"
#include "version.h"
#include "auth/pool_passwd.h"
@@ -46,6 +47,7 @@
#include "pool_config_variables.h"
static void daemonize(void);
+static char *get_pid_file_path(void);
static int read_pid_file(void);
static void write_pid_file(void);
static void usage(void);
@@ -53,9 +55,10 @@ static void show_version(void);
static void stop_me(void);
static void FileUnlink(int code, Datum path);
-char pcp_conf_file[POOLMAXPATHLEN+1]; /* path for pcp.conf */
-char conf_file[POOLMAXPATHLEN+1];
-char hba_file[POOLMAXPATHLEN+1];
+char *pcp_conf_file = NULL; /* absolute path of the pcp.conf */
+char *conf_file = NULL; /* absolute path of the pgpool.conf */
+char *hba_file = NULL; /* absolute path of the hba.conf */
+char *base_dir = NULL; /* The working dir from where pgpool was invoked from */
static int not_detach = 0; /* non 0 if non detach option (-n) is given */
int stop_sig = SIGTERM; /* stopping signal default value */
@@ -71,6 +74,10 @@ int main(int argc, char **argv)
bool discard_status = false;
bool clear_memcache_oidmaps = false;
+ char pcp_conf_file_path[POOLMAXPATHLEN+1];
+ char conf_file_path[POOLMAXPATHLEN+1];
+ char hba_file_path[POOLMAXPATHLEN+1];
+
static struct option long_options[] = {
{"hba-file", required_argument, NULL, 'a'},
{"debug", no_argument, NULL, 'd'},
@@ -89,9 +96,9 @@ int main(int argc, char **argv)
myargc = argc;
myargv = argv;
- snprintf(conf_file, sizeof(conf_file), "%s/%s", DEFAULT_CONFIGDIR, POOL_CONF_FILE_NAME);
- snprintf(pcp_conf_file, sizeof(pcp_conf_file), "%s/%s", DEFAULT_CONFIGDIR, PCP_PASSWD_FILE_NAME);
- snprintf(hba_file, sizeof(hba_file), "%s/%s", DEFAULT_CONFIGDIR, HBA_CONF_FILE_NAME);
+ snprintf(conf_file_path, sizeof(conf_file_path), "%s/%s", DEFAULT_CONFIGDIR, POOL_CONF_FILE_NAME);
+ snprintf(pcp_conf_file_path, sizeof(pcp_conf_file_path), "%s/%s", DEFAULT_CONFIGDIR, PCP_PASSWD_FILE_NAME);
+ snprintf(hba_file_path, sizeof(hba_file_path), "%s/%s", DEFAULT_CONFIGDIR, HBA_CONF_FILE_NAME);
while ((opt = getopt_long(argc, argv, "a:df:F:hm:nDCxv", long_options, &optindex)) != -1)
{
switch (opt)
@@ -102,7 +109,7 @@ int main(int argc, char **argv)
usage();
exit(1);
}
- strlcpy(hba_file, optarg, sizeof(hba_file));
+ strlcpy(hba_file_path, optarg, sizeof(hba_file_path));
break;
case 'x': /* enable cassert */
@@ -119,7 +126,7 @@ int main(int argc, char **argv)
usage();
exit(1);
}
- strlcpy(conf_file, optarg, sizeof(conf_file));
+ strlcpy(conf_file_path, optarg, sizeof(conf_file_path));
break;
case 'F': /* specify PCP password file */
@@ -128,7 +135,7 @@ int main(int argc, char **argv)
usage();
exit(1);
}
- strlcpy(pcp_conf_file, optarg, sizeof(pcp_conf_file));
+ strlcpy(pcp_conf_file_path, optarg, sizeof(pcp_conf_file_path));
break;
case 'h':
@@ -186,6 +193,13 @@ int main(int argc, char **argv)
/* create MemoryContexts */
MemoryContextInit();
+ /* load the CWD before it is changed */
+ base_dir = get_current_working_dir();
+ /* convert all the paths to absolute paths*/
+ conf_file = make_absolute_path(conf_file_path, base_dir);
+ pcp_conf_file = make_absolute_path(pcp_conf_file_path, base_dir);
+ hba_file = make_absolute_path(hba_file_path, base_dir);
+
mypid = getpid();
pool_init_config();
@@ -227,7 +241,6 @@ int main(int argc, char **argv)
if (!strcmp(argv[optind], "stop"))
{
stop_me();
- unlink(pool_config->pid_file_name);
exit(0);
}
else
@@ -414,6 +427,7 @@ static void daemonize(void)
static void stop_me(void)
{
pid_t pid;
+ char *pid_file;
pid = read_pid_file();
if (pid < 0)
@@ -437,6 +451,49 @@ static void stop_me(void)
sleep(1);
}
fprintf(stderr, "done.\n");
+ pid_file = get_pid_file_path();
+ unlink(pid_file);
+ pfree(pid_file);
+}
+
+/*
+ * The function returns the palloc'd copy of pid_file_path,
+ * caller must free it after use
+ */
+static char *get_pid_file_path(void)
+{
+ char *new = NULL;
+ if (!is_absolute_path(pool_config->pid_file_name))
+ {
+ /*
+ * some implementations of dirname() may modify the
+ * string argument passed to it, so do not use the original
+ * conf_file as an argument
+ */
+ char *conf_file_copy = pstrdup(conf_file);
+ char *conf_dir = dirname(conf_file_copy);
+ size_t path_size;
+ if (conf_dir == NULL)
+ {
+ ereport(LOG,
+ (errmsg("failed to get the dirname of pid file:\"%s\". reason: %s",
+ pool_config->pid_file_name, strerror(errno))));
+ return NULL;
+ }
+ path_size = strlen(conf_dir) + strlen(pool_config->pid_file_name) + 1 + 1;
+ new = palloc(path_size);
+ snprintf(new, path_size, "%s/%s",conf_dir, pool_config->pid_file_name);
+
+ ereport(DEBUG1,
+ (errmsg("pid file location is \"%s\"",
+ new)));
+ }
+ else
+ {
+ new = pstrdup(pool_config->pid_file_name);
+ }
+
+ return new;
}
/*
@@ -447,15 +504,25 @@ static int read_pid_file(void)
int fd;
int readlen;
char pidbuf[128];
+ char *pid_file = get_pid_file_path();
- fd = open(pool_config->pid_file_name, O_RDONLY);
+ if (pid_file == NULL)
+ {
+ ereport(FATAL,
+ (errmsg("failed to read pid file"),
+ errdetail("failed to get pid file path from \"%s\"",
+ pool_config->pid_file_name)));
+ }
+ fd = open(pid_file, O_RDONLY);
if (fd == -1)
{
+ pfree(pid_file);
return -1;
}
if ((readlen = read(fd, pidbuf, sizeof(pidbuf))) == -1)
{
close(fd);
+ pfree(pid_file);
ereport(FATAL,
(errmsg("could not read pid file as \"%s\". reason: %s",
pool_config->pid_file_name, strerror(errno))));
@@ -463,10 +530,12 @@ static int read_pid_file(void)
else if (readlen == 0)
{
close(fd);
+ pfree(pid_file);
ereport(FATAL,
(errmsg("EOF detected while reading pid file \"%s\". reason: %s",
pool_config->pid_file_name, strerror(errno))));
}
+ pfree(pid_file);
close(fd);
return(atoi(pidbuf));
}
@@ -478,8 +547,17 @@ static void write_pid_file(void)
{
int fd;
char pidbuf[128];
+ char *pid_file = get_pid_file_path();
+
+ if (pid_file == NULL)
+ {
+ ereport(FATAL,
+ (errmsg("failed to write pid file"),
+ errdetail("failed to get pid file path from \"%s\"",
+ pool_config->pid_file_name)));
+ }
- fd = open(pool_config->pid_file_name, O_CREAT|O_WRONLY, S_IRUSR|S_IWUSR);
+ fd = open(pid_file, O_CREAT|O_WRONLY, S_IRUSR|S_IWUSR);
if (fd == -1)
{
ereport(FATAL,
@@ -490,6 +568,7 @@ static void write_pid_file(void)
if (write(fd, pidbuf, strlen(pidbuf)+1) == -1)
{
close(fd);
+ pfree(pid_file);
ereport(FATAL,
(errmsg("could not write pid file as %s. reason: %s",
pool_config->pid_file_name, strerror(errno))));
@@ -497,18 +576,20 @@ static void write_pid_file(void)
if (fsync(fd) == -1)
{
close(fd);
+ pfree(pid_file);
ereport(FATAL,
(errmsg("could not fsync pid file as %s. reason: %s",
pool_config->pid_file_name, strerror(errno))));
}
if (close(fd) == -1)
{
+ pfree(pid_file);
ereport(FATAL,
(errmsg("could not close pid file as %s. reason: %s",
pool_config->pid_file_name, strerror(errno))));
}
/* register the call back to delete the pid file at system exit */
- on_proc_exit(FileUnlink, (Datum) pool_config->pid_file_name);
+ on_proc_exit(FileUnlink, (Datum) pid_file);
}
/*
diff --git a/src/main/pgpool_main.c b/src/main/pgpool_main.c
index 5ae8a5b7..32d84e9e 100644
--- a/src/main/pgpool_main.c
+++ b/src/main/pgpool_main.c
@@ -170,9 +170,9 @@ static int *fds; /* listening file descriptors (UNIX socket, inet domain sockets
static int pcp_unix_fd; /* unix domain socket fd for PCP (not used) */
static int pcp_inet_fd; /* inet domain socket fd for PCP */
-extern char pcp_conf_file[POOLMAXPATHLEN+1]; /* path for pcp.conf */
-extern char conf_file[POOLMAXPATHLEN+1];
-extern char hba_file[POOLMAXPATHLEN+1];
+extern char *pcp_conf_file; /* path for pcp.conf */
+extern char *conf_file;
+extern char *hba_file;
static int exiting = 0; /* non 0 if I'm exiting */
static int switching = 0; /* non 0 if I'm failing over or degenerating */
diff --git a/src/pcp_con/pcp_worker.c b/src/pcp_con/pcp_worker.c
index e50a5765..54219a72 100644
--- a/src/pcp_con/pcp_worker.c
+++ b/src/pcp_con/pcp_worker.c
@@ -52,7 +52,7 @@
#define MAX_FILE_LINE_LEN 512
-extern char pcp_conf_file[POOLMAXPATHLEN+1]; /* global variable defined in main.c holds the path for pcp.conf */
+extern char *pcp_conf_file; /* global variable defined in main.c holds the path for pcp.conf */
volatile sig_atomic_t pcp_worker_wakeup_request = 0;
PCP_CONNECTION* volatile pcp_frontend = NULL;
diff --git a/src/redhat/pgpool.conf.sample.patch b/src/redhat/pgpool.conf.sample.patch
index d47674b6..515f26fc 100644
--- a/src/redhat/pgpool.conf.sample.patch
+++ b/src/redhat/pgpool.conf.sample.patch
@@ -37,6 +37,9 @@
*** 214,220 ****
pid_file_name = '/var/run/pgpool/pgpool.pid'
# PID file name
+ # Can be specified as relative to the"
+ # location of pgpool.conf file or
+ # as an absolute path
# (change requires restart)
! logdir = '/tmp'
# Directory of pgPool status file
@@ -45,6 +48,9 @@
--- 214,220 ----
pid_file_name = '/var/run/pgpool/pgpool.pid'
# PID file name
+ # Can be specified as relative to the"
+ # location of pgpool.conf file or
+ # or as absolute path
# (change requires restart)
! logdir = '/var/log/pgpool'
# Directory of pgPool status file
diff --git a/src/sample/pgpool.conf.sample b/src/sample/pgpool.conf.sample
index 479f1072..d73c9bca 100644
--- a/src/sample/pgpool.conf.sample
+++ b/src/sample/pgpool.conf.sample
@@ -212,6 +212,9 @@ syslog_ident = 'pgpool'
pid_file_name = '/var/run/pgpool/pgpool.pid'
# PID file name
+ # Can be specified as relative to the"
+ # location of pgpool.conf file or
+ # as an absolute path
# (change requires restart)
logdir = '/var/log/pgpool'
# Directory of pgPool status file
diff --git a/src/sample/pgpool.conf.sample-master-slave b/src/sample/pgpool.conf.sample-master-slave
index 6246ec6d..612c10a7 100644
--- a/src/sample/pgpool.conf.sample-master-slave
+++ b/src/sample/pgpool.conf.sample-master-slave
@@ -212,6 +212,9 @@ syslog_ident = 'pgpool'
pid_file_name = '/var/run/pgpool/pgpool.pid'
# PID file name
+ # Can be specified as relative to the"
+ # location of pgpool.conf file or
+ # as an absolute path
# (change requires restart)
logdir = '/tmp'
# Directory of pgPool status file
diff --git a/src/sample/pgpool.conf.sample-replication b/src/sample/pgpool.conf.sample-replication
index 1e0d6761..f47a7957 100644
--- a/src/sample/pgpool.conf.sample-replication
+++ b/src/sample/pgpool.conf.sample-replication
@@ -212,6 +212,9 @@ syslog_ident = 'pgpool'
pid_file_name = '/var/run/pgpool/pgpool.pid'
# PID file name
+ # Can be specified as relative to the"
+ # location of pgpool.conf file or
+ # as an absolute path
# (change requires restart)
logdir = '/tmp'
# Directory of pgPool status file
diff --git a/src/sample/pgpool.conf.sample-stream b/src/sample/pgpool.conf.sample-stream
index 5b44ba13..d57226b0 100644
--- a/src/sample/pgpool.conf.sample-stream
+++ b/src/sample/pgpool.conf.sample-stream
@@ -213,6 +213,9 @@ syslog_ident = 'pgpool'
pid_file_name = '/var/run/pgpool/pgpool.pid'
# PID file name
+ # Can be specified as relative to the"
+ # location of pgpool.conf file or
+ # as an absolute path
# (change requires restart)
logdir = '/tmp'
# Directory of pgPool status file
diff --git a/src/utils/pool_path.c b/src/utils/pool_path.c
index f2829ccb..3c2c7c21 100644
--- a/src/utils/pool_path.c
+++ b/src/utils/pool_path.c
@@ -6,7 +6,7 @@
* pgpool: a language independent connection pool server for PostgreSQL
* written by Tatsuo Ishii
*
- * Portions Copyright (c) 2003-2008, PgPool Global Development Group
+ * Portions Copyright (c) 2003-2017, PgPool Global Development Group
* Portions Copyright (c) 2004, PostgreSQL Global Development Group
*
* Permission to use, copy, modify, and distribute this software and
@@ -30,6 +30,11 @@
#include <string.h>
#include <unistd.h>
#include <pwd.h>
+#include <errno.h>
+#include "utils/elog.h"
+#include "utils/palloc.h"
+#include "utils/memutils.h"
+
static void trim_directory(char *path);
static void trim_trailing_separator(char *path);
@@ -275,3 +280,83 @@ static void trim_trailing_separator(char *path)
for (p--; p > path && IS_DIR_SEP(*p); p--)
*p = '\0';
}
+
+char *
+get_current_working_dir(void)
+{
+ char *buf = NULL;
+ size_t buflen = MAXPGPATH;
+
+ for (;;)
+ {
+ buf = palloc(buflen);
+
+ if (getcwd(buf, buflen))
+ break;
+
+ else if (errno == ERANGE)
+ {
+ pfree(buf);
+ buflen *= 2;
+ continue;
+ }
+ else
+ {
+ int save_errno = errno;
+
+ pfree(buf);
+ errno = save_errno;
+ ereport(ERROR,
+ (errmsg("could not get current working directory: %m")));
+ return NULL;
+ }
+ }
+ return buf;
+}
+/*
+ * make_absolute_path
+ *
+ * If the given pathname isn't already absolute, make it so, interpreting
+ * it relative to the current working directory.
+ *
+ * if arg base_dir is NULL, The function gets the current cwd
+ * Also canonicalizes the path. The result is always a palloc'd copy.
+ *
+ * Logic borrowed from PostgreSQL source
+ */
+char *
+make_absolute_path(const char *path, const char* base_dir)
+{
+ char *new;
+ /* Returning null for null input is convenient for some callers */
+ if (path == NULL)
+ return NULL;
+
+ if (!is_absolute_path(path))
+ {
+ const char *cwd = NULL;
+ if (base_dir == NULL)
+ {
+ cwd = get_current_working_dir();
+ if (!cwd)
+ return NULL;
+ }
+ else
+ cwd = base_dir;
+
+ new = palloc(strlen(cwd) + strlen(path) + 2);
+ sprintf(new, "%s/%s", cwd, path);
+
+ if (!base_dir)
+ pfree((void*)cwd);
+ }
+ else
+ {
+ new = pstrdup(path);
+ }
+
+ /* Make sure punctuation is canonical, too */
+ canonicalize_path(new);
+
+ return new;
+}
More information about the pgpool-hackers
mailing list