Make it reasonably safe to use pg_ctl to start the postmaster from a boot-time
script. To do this, have pg_ctl pass down its parent shell's PID in an environment variable PG_GRANDPARENT_PID, and teach CreateLockFile() to disregard that PID as a false match if it finds it in postmaster.pid. This allows us to cope with one level of postgres-owned shell process even with pg_ctl in the way, so it's just as safe as starting the postmaster directly. You still have to be careful about how you write the initscript though. Adjust the comments in contrib/start-scripts/ to not deprecate use of pg_ctl. Also, fix the ROTATELOGS option in the OSX script, which was indulging in exactly the sort of unsafe coding that renders this fix pointless :-(. A pipe inside the "sudo" will probably result in more than one postgres-owned process hanging around.
This commit is contained in:
Tom Lane
parent
0e3f0cbddc
commit
8f5500e6bd
|
|
@ -6,7 +6,7 @@
|
|||
# Created through merger of the Linux start script by Ryan Kirkpatrick
|
||||
# and the script in the FreeBSD ports collection.
|
||||
|
||||
# $PostgreSQL: pgsql/contrib/start-scripts/freebsd,v 1.4 2004/10/01 18:30:21 tgl Exp $
|
||||
# $PostgreSQL: pgsql/contrib/start-scripts/freebsd,v 1.5 2009/08/27 16:59:38 tgl Exp $
|
||||
|
||||
## EDIT FROM HERE
|
||||
|
||||
|
|
@ -27,9 +27,9 @@ PGLOG="$PGDATA/serverlog"
|
|||
# The path that is to be used for the script
|
||||
PATH=/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin
|
||||
|
||||
# What to use to start up the postmaster (we do NOT use pg_ctl for this,
|
||||
# as it adds no value and can cause the postmaster to misrecognize a stale
|
||||
# lock file)
|
||||
# What to use to start up the postmaster. (If you want the script to wait
|
||||
# until the server has started, you could use "pg_ctl start -w" here.
|
||||
# But without -w, pg_ctl adds no value.)
|
||||
DAEMON="$prefix/bin/postmaster"
|
||||
|
||||
# What to use to shut down the postmaster
|
||||
|
|
|
|||
|
|
@ -24,7 +24,7 @@
|
|||
|
||||
# Original author: Ryan Kirkpatrick <pgsql@rkirkpat.net>
|
||||
|
||||
# $PostgreSQL: pgsql/contrib/start-scripts/linux,v 1.8 2006/07/13 14:44:33 petere Exp $
|
||||
# $PostgreSQL: pgsql/contrib/start-scripts/linux,v 1.9 2009/08/27 16:59:38 tgl Exp $
|
||||
|
||||
## EDIT FROM HERE
|
||||
|
||||
|
|
@ -45,9 +45,9 @@ PGLOG="$PGDATA/serverlog"
|
|||
# The path that is to be used for the script
|
||||
PATH=/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin
|
||||
|
||||
# What to use to start up the postmaster (we do NOT use pg_ctl for this,
|
||||
# as it adds no value and can cause the postmaster to misrecognize a stale
|
||||
# lock file)
|
||||
# What to use to start up the postmaster. (If you want the script to wait
|
||||
# until the server has started, you could use "pg_ctl start -w" here.
|
||||
# But without -w, pg_ctl adds no value.)
|
||||
DAEMON="$prefix/bin/postmaster"
|
||||
|
||||
# What to use to shut down the postmaster
|
||||
|
|
|
|||
|
|
@ -68,9 +68,9 @@ ROTATESEC="604800"
|
|||
# The path that is to be used for the script
|
||||
PATH="$prefix/bin:/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin"
|
||||
|
||||
# What to use to start up the postmaster (we do NOT use pg_ctl for this,
|
||||
# as it adds no value and can cause the postmaster to misrecognize a stale
|
||||
# lock file)
|
||||
# What to use to start up the postmaster. (If you want the script to wait
|
||||
# until the server has started, you could use "pg_ctl start -w" here.
|
||||
# But without -w, pg_ctl adds no value.)
|
||||
DAEMON="$prefix/bin/postmaster"
|
||||
|
||||
# What to use to shut down the postmaster
|
||||
|
|
@ -85,7 +85,7 @@ StartService () {
|
|||
if [ "${POSTGRESQL:=-NO-}" = "-YES-" ]; then
|
||||
ConsoleMessage "Starting PostgreSQL database server"
|
||||
if [ "${ROTATELOGS}" = "1" ]; then
|
||||
sudo -u $PGUSER sh -c "${DAEMON} -D '${PGDATA}' 2>&1 | ${LOGUTIL} '${PGLOG}' ${ROTATESEC} &"
|
||||
sudo -u $PGUSER sh -c "${DAEMON} -D '${PGDATA}' &" 2>&1 | ${LOGUTIL} '${PGLOG}' ${ROTATESEC} &
|
||||
else
|
||||
sudo -u $PGUSER sh -c "${DAEMON} -D '${PGDATA}' &" >>$PGLOG 2>&1
|
||||
fi
|
||||
|
|
@ -104,7 +104,7 @@ RestartService () {
|
|||
sudo -u $PGUSER $PGCTL stop -D "$PGDATA" -s -m fast
|
||||
# should match StartService:
|
||||
if [ "${ROTATELOGS}" = "1" ]; then
|
||||
sudo -u $PGUSER sh -c "${DAEMON} -D '${PGDATA}' 2>&1 | ${LOGUTIL} '${PGLOG}' ${ROTATESEC} &"
|
||||
sudo -u $PGUSER sh -c "${DAEMON} -D '${PGDATA}' &" 2>&1 | ${LOGUTIL} '${PGLOG}' ${ROTATESEC} &
|
||||
else
|
||||
sudo -u $PGUSER sh -c "${DAEMON} -D '${PGDATA}' &" >>$PGLOG 2>&1
|
||||
fi
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@
|
|||
*
|
||||
*
|
||||
* IDENTIFICATION
|
||||
* $PostgreSQL: pgsql/src/backend/utils/init/miscinit.c,v 1.176 2009/08/12 20:53:30 tgl Exp $
|
||||
* $PostgreSQL: pgsql/src/backend/utils/init/miscinit.c,v 1.177 2009/08/27 16:59:38 tgl Exp $
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
|
|
@ -683,7 +683,46 @@ CreateLockFile(const char *filename, bool amPostmaster,
|
|||
int len;
|
||||
int encoded_pid;
|
||||
pid_t other_pid;
|
||||
pid_t my_pid = getpid();
|
||||
pid_t my_pid,
|
||||
my_p_pid,
|
||||
my_gp_pid;
|
||||
const char *envvar;
|
||||
|
||||
/*
|
||||
* If the PID in the lockfile is our own PID or our parent's or
|
||||
* grandparent's PID, then the file must be stale (probably left over from
|
||||
* a previous system boot cycle). We need to check this because of the
|
||||
* likelihood that a reboot will assign exactly the same PID as we had in
|
||||
* the previous reboot, or one that's only one or two counts larger and
|
||||
* hence the lockfile's PID now refers to an ancestor shell process. We
|
||||
* allow pg_ctl to pass down its parent shell PID (our grandparent PID)
|
||||
* via the environment variable PG_GRANDPARENT_PID; this is so that
|
||||
* launching the postmaster via pg_ctl can be just as reliable as
|
||||
* launching it directly. There is no provision for detecting
|
||||
* further-removed ancestor processes, but if the init script is written
|
||||
* carefully then all but the immediate parent shell will be root-owned
|
||||
* processes and so the kill test will fail with EPERM. Note that we
|
||||
* cannot get a false negative this way, because an existing postmaster
|
||||
* would surely never launch a competing postmaster or pg_ctl process
|
||||
* directly.
|
||||
*/
|
||||
my_pid = getpid();
|
||||
|
||||
#ifndef WIN32
|
||||
my_p_pid = getppid();
|
||||
#else
|
||||
/*
|
||||
* Windows hasn't got getppid(), but doesn't need it since it's not
|
||||
* using real kill() either...
|
||||
*/
|
||||
my_p_pid = 0;
|
||||
#endif
|
||||
|
||||
envvar = getenv("PG_GRANDPARENT_PID");
|
||||
if (envvar)
|
||||
my_gp_pid = atoi(envvar);
|
||||
else
|
||||
my_gp_pid = 0;
|
||||
|
||||
/*
|
||||
* We need a loop here because of race conditions. But don't loop forever
|
||||
|
|
@ -745,17 +784,11 @@ CreateLockFile(const char *filename, bool amPostmaster,
|
|||
/*
|
||||
* Check to see if the other process still exists
|
||||
*
|
||||
* If the PID in the lockfile is our own PID or our parent's PID, then
|
||||
* the file must be stale (probably left over from a previous system
|
||||
* boot cycle). We need this test because of the likelihood that a
|
||||
* reboot will assign exactly the same PID as we had in the previous
|
||||
* reboot. Also, if there is just one more process launch in this
|
||||
* reboot than in the previous one, the lockfile might mention our
|
||||
* parent's PID. We can reject that since we'd never be launched
|
||||
* directly by a competing postmaster. We can't detect grandparent
|
||||
* processes unfortunately, but if the init script is written
|
||||
* carefully then all but the immediate parent shell will be
|
||||
* root-owned processes and so the kill test will fail with EPERM.
|
||||
* Per discussion above, my_pid, my_p_pid, and my_gp_pid can be
|
||||
* ignored as false matches.
|
||||
*
|
||||
* Normally kill() will fail with ESRCH if the given PID doesn't
|
||||
* exist.
|
||||
*
|
||||
* We can treat the EPERM-error case as okay because that error
|
||||
* implies that the existing process has a different userid than we
|
||||
|
|
@ -772,18 +805,9 @@ CreateLockFile(const char *filename, bool amPostmaster,
|
|||
* Unix socket file belonging to an instance of Postgres being run by
|
||||
* someone else, at least on machines where /tmp hasn't got a
|
||||
* stickybit.)
|
||||
*
|
||||
* Windows hasn't got getppid(), but doesn't need it since it's not
|
||||
* using real kill() either...
|
||||
*
|
||||
* Normally kill() will fail with ESRCH if the given PID doesn't
|
||||
* exist.
|
||||
*/
|
||||
if (other_pid != my_pid
|
||||
#ifndef WIN32
|
||||
&& other_pid != getppid()
|
||||
#endif
|
||||
)
|
||||
if (other_pid != my_pid && other_pid != my_p_pid &&
|
||||
other_pid != my_gp_pid)
|
||||
{
|
||||
if (kill(other_pid, 0) == 0 ||
|
||||
(errno != ESRCH && errno != EPERM))
|
||||
|
|
|
|||
|
|
@ -4,7 +4,7 @@
|
|||
*
|
||||
* Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
|
||||
*
|
||||
* $PostgreSQL: pgsql/src/bin/pg_ctl/pg_ctl.c,v 1.111 2009/06/11 14:49:07 momjian Exp $
|
||||
* $PostgreSQL: pgsql/src/bin/pg_ctl/pg_ctl.c,v 1.112 2009/08/27 16:59:38 tgl Exp $
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
|
|
@ -672,6 +672,21 @@ do_start(void)
|
|||
unlimit_core_size();
|
||||
#endif
|
||||
|
||||
/*
|
||||
* If possible, tell the postmaster our parent shell's PID (see the
|
||||
* comments in CreateLockFile() for motivation). Windows hasn't got
|
||||
* getppid() unfortunately.
|
||||
*/
|
||||
#ifndef WIN32
|
||||
{
|
||||
static char env_var[32];
|
||||
|
||||
snprintf(env_var, sizeof(env_var), "PG_GRANDPARENT_PID=%d",
|
||||
(int) getppid());
|
||||
putenv(env_var);
|
||||
}
|
||||
#endif
|
||||
|
||||
exitcode = start_postmaster();
|
||||
if (exitcode != 0)
|
||||
{
|
||||
|
|
|
|||
Loading…
Reference in New Issue