Thread: BUG #1224: Restarting postgres appends extra -D argument
The following bug has been logged online: Bug reference: 1224 Logged by: David Maggard Email address: drm31415@charter.net PostgreSQL version: 7.4.3 Operating system: Redhat Enterprise Linux ES 3.0 Update 1 Description: Restarting postgres appends extra -D argument Details: I installed from the src, I am using /postgresql-7.4.3/contrib/startup-scripts/linux, I have found that restarting postgresql via the sysV script or pg_ctl like either of the following: service postgres restart pg_ctl restart -D /usr/local/pgsql/data results in an extra '-D /usr/local/pgsql/data' being appended each time it is done, you can see this by running either of the following after each restart: service postgres status pg_ctl status -D /usr/local/pgsql/data this isn't a problem for me, but I thought it might be a potential problem for someone else
I can't reproduce this problem on 7.4.4, and I don't see any changes that were made between 7.4.3 to 7.4.4. We had this bug, but it was fixed in 2001. The pg_ctl arguments are kept in /data/postmaster.opts. Can you show a reproducable test case? --------------------------------------------------------------------------- PostgreSQL Bugs List wrote: > > The following bug has been logged online: > > Bug reference: 1224 > Logged by: David Maggard > > Email address: drm31415@charter.net > > PostgreSQL version: 7.4.3 > > Operating system: Redhat Enterprise Linux ES 3.0 Update 1 > > Description: Restarting postgres appends extra -D argument > > Details: > > I installed from the src, I am using > /postgresql-7.4.3/contrib/startup-scripts/linux, I have found that > restarting postgresql via the sysV script or pg_ctl like either of the > following: > service postgres restart > pg_ctl restart -D /usr/local/pgsql/data > > results in an extra '-D /usr/local/pgsql/data' being appended each time it > is done, you can see this by running either of the following after each > restart: > service postgres status > pg_ctl status -D /usr/local/pgsql/data > > this isn't a problem for me, but I thought it might be a potential problem > for someone else > > > > ---------------------------(end of broadcast)--------------------------- > TIP 9: the planner will ignore your desire to choose an index scan if your > joining column's datatypes do not match > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
I can confirm this bug report now. I see it happens with:
pg_ctl -D /u/pg/data restart
You have to use '-D' and 'restart' to cause -D to duplicate in
postmaster.opts on every restart. It should be fixed because excessive
restarts could exceed the maximum command length, causing the server not
to restart.
The problem was actually introduced here:
revision 1.31
date: 2003/02/14 22:18:25; author: momjian; state: Exp; lines: +7 -4
Propogate pg_ctl -D to the postmaster as a -D flag for identification by
ps for multiple postmasters, for Kevin Brown.
It added -D to the command line so 'ps' showed it, rather than just
passing it via PGDATA. Of course on restart we should not be adding it
again.
I am attaching a patch which fixes the problem by not adding the -D
display addition when doing a restart. It is not needed for restart
because the -D comes from the postmaster.opts file.
We are packaging 7.4.5 now so I will wait to apply this after the
release.
Looking at 8.0 CVS, I see we are no longer adding the -D argument to
postmaster start, meaning 'ps' will not display the -D to distinguish
multiple postmasters. Seems I should fix that too.
---------------------------------------------------------------------------
David Maggard wrote:
> I reinstalled 7.4.3 and it still occured.
> I then installed 7.4.5 and it still occured.
>
> I went thru pg_ctl and believe I found why it is happening, the -D that is
> given to pg_ctl is appended to what is in postmaster.opts, I created a html
> file that should show you what I found, it is located at:
> http://www.intertelmark.com/linux/pg_ctlbug.htm
>
> ----- Original Message -----
> From: "Bruce Momjian" <pgman@candle.pha.pa.us>
> To: "David Maggard" <drm31415@charter.net>
> Cc: <pgsql-bugs@postgresql.org>
> Sent: Wednesday, August 18, 2004 9:15 PM
> Subject: Re: [BUGS] BUG #1224: Restarting postgres appends extra -D argument
>
>
> >
> > I can't reproduce this problem on 7.4.4, and I don't see any changes
> > that were made between 7.4.3 to 7.4.4. We had this bug, but it was
> > fixed in 2001.
> >
> > The pg_ctl arguments are kept in /data/postmaster.opts. Can you show a
> > reproducable test case?
> >
> > --------------------------------------------------------------------------
> -
> >
> > PostgreSQL Bugs List wrote:
> > >
> > > The following bug has been logged online:
> > >
> > > Bug reference: 1224
> > > Logged by: David Maggard
> > >
> > > Email address: drm31415@charter.net
> > >
> > > PostgreSQL version: 7.4.3
> > >
> > > Operating system: Redhat Enterprise Linux ES 3.0 Update 1
> > >
> > > Description: Restarting postgres appends extra -D argument
> > >
> > > Details:
> > >
> > > I installed from the src, I am using
> > > /postgresql-7.4.3/contrib/startup-scripts/linux, I have found that
> > > restarting postgresql via the sysV script or pg_ctl like either of the
> > > following:
> > > service postgres restart
> > > pg_ctl restart -D /usr/local/pgsql/data
> > >
> > > results in an extra '-D /usr/local/pgsql/data' being appended each time
> it
> > > is done, you can see this by running either of the following after each
> > > restart:
> > > service postgres status
> > > pg_ctl status -D /usr/local/pgsql/data
> > >
> > > this isn't a problem for me, but I thought it might be a potential
> problem
> > > for someone else
> > >
> > >
> > >
> > > ---------------------------(end of broadcast)---------------------------
> > > TIP 9: the planner will ignore your desire to choose an index scan if
> your
> > > joining column's datatypes do not match
> > >
> >
> > --
> > Bruce Momjian | http://candle.pha.pa.us
> > pgman@candle.pha.pa.us | (610) 359-1001
> > + If your life is a hard drive, | 13 Roberts Road
> > + Christ can be your backup. | Newtown Square, Pennsylvania
> 19073
>
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Index: src/bin/pg_ctl/pg_ctl.sh
===================================================================
RCS file: /cvsroot/pgsql-server/src/bin/pg_ctl/Attic/pg_ctl.sh,v
retrieving revision 1.36
diff -c -c -r1.36 pg_ctl.sh
*** src/bin/pg_ctl/pg_ctl.sh 14 Aug 2003 18:56:41 -0000 1.36
--- src/bin/pg_ctl/pg_ctl.sh 20 Aug 2004 01:38:57 -0000
***************
*** 8,14 ****
#
#
# IDENTIFICATION
! # $Header: /cvsroot/pgsql-server/src/bin/pg_ctl/Attic/pg_ctl.sh,v 1.36 2003/08/14 18:56:41 tgl Exp $
#
#-------------------------------------------------------------------------
--- 8,14 ----
#
#
# IDENTIFICATION
! # $Header: /cvsroot/pgsql-server/src/bin/pg_ctl/pg_ctl.sh,v 1.36 2003/08/14 18:56:41 tgl Exp $
#
#-------------------------------------------------------------------------
***************
*** 238,243 ****
--- 238,248 ----
wait=no
fi
+ # Prevent duplicate of -D flags on each restart
+ if [ "$op" = "restart" ];then
+ PGDATAOPTS=""
+ fi
+
DEFPOSTOPTS=$PGDATA/postmaster.opts.default
POSTOPTSFILE=$PGDATA/postmaster.opts
PIDFILE=$PGDATA/postmaster.pid
Bruce Momjian wrote: > > I can confirm this bug report now. I see it happens with: > > pg_ctl -D /u/pg/data restart > > You have to use '-D' and 'restart' to cause -D to duplicate in > postmaster.opts on every restart. It should be fixed because excessive > restarts could exceed the maximum command length, causing the server not > to restart. > > The problem was actually introduced here: > > revision 1.31 > date: 2003/02/14 22:18:25; author: momjian; state: Exp; lines: +7 -4 > Propogate pg_ctl -D to the postmaster as a -D flag for identification by > ps for multiple postmasters, for Kevin Brown. > > It added -D to the command line so 'ps' showed it, rather than just > passing it via PGDATA. Of course on restart we should not be adding it > again. > > I am attaching a patch which fixes the problem by not adding the -D > display addition when doing a restart. It is not needed for restart > because the -D comes from the postmaster.opts file. > > We are packaging 7.4.5 now so I will wait to apply this after the > release. Patch applied. It will appear in 7.4.6. > Looking at 8.0 CVS, I see we are no longer adding the -D argument to > postmaster start, meaning 'ps' will not display the -D to distinguish > multiple postmasters. Seems I should fix that too. I have applied the following patch to propogate pg_ctl -D to the postmaster command line. This worked in 7.4.X and should continue working in 8.0. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073 Index: src/bin/pg_ctl/pg_ctl.c =================================================================== RCS file: /cvsroot/pgsql-server/src/bin/pg_ctl/pg_ctl.c,v retrieving revision 1.26 diff -c -c -r1.26 pg_ctl.c *** src/bin/pg_ctl/pg_ctl.c 28 Aug 2004 21:01:38 -0000 1.26 --- src/bin/pg_ctl/pg_ctl.c 28 Aug 2004 21:57:29 -0000 *************** *** 67,72 **** --- 67,73 ---- static int sig = SIGTERM; /* default */ static CtlCommand ctl_command = NO_COMMAND; static char *pg_data = NULL; + static char *pgdata_opt = NULL; static char *post_opts = NULL; static const char *progname; static char *log_file = NULL; *************** *** 309,327 **** */ if (log_file != NULL) #ifndef WIN32 ! snprintf(cmd, MAXPGPATH, "%s\"%s\" %s < \"%s\" >> \"%s\" 2>&1 &%s", #else ! snprintf(cmd, MAXPGPATH, "%sSTART /B \"\" \"%s\" %s < \"%s\" >> \"%s\" 2>&1%s", #endif ! SYSTEMQUOTE, postgres_path, post_opts, DEVNULL, log_file, ! SYSTEMQUOTE); else #ifndef WIN32 ! snprintf(cmd, MAXPGPATH, "%s\"%s\" %s < \"%s\" 2>&1 &%s", #else ! snprintf(cmd, MAXPGPATH, "%sSTART /B \"\" \"%s\" %s < \"%s\" 2>&1%s", #endif ! SYSTEMQUOTE, postgres_path, post_opts, DEVNULL, SYSTEMQUOTE); return system(cmd); } --- 310,329 ---- */ if (log_file != NULL) #ifndef WIN32 ! snprintf(cmd, MAXPGPATH, "%s\"%s\" %s%s < \"%s\" >> \"%s\" 2>&1 &%s", #else ! snprintf(cmd, MAXPGPATH, "%sSTART /B \"\" \"%s\" %s%s < \"%s\" >> \"%s\" 2>&1%s", #endif ! SYSTEMQUOTE, postgres_path, pgdata_opt, post_opts, ! DEVNULL, log_file, SYSTEMQUOTE); else #ifndef WIN32 ! snprintf(cmd, MAXPGPATH, "%s\"%s\" %s%s < \"%s\" 2>&1 &%s", #else ! snprintf(cmd, MAXPGPATH, "%sSTART /B \"\" \"%s\" %s%s < \"%s\" 2>&1%s", #endif ! SYSTEMQUOTE, postgres_path, pgdata_opt, post_opts, ! DEVNULL, SYSTEMQUOTE); return system(cmd); } *************** *** 494,499 **** --- 496,505 ---- } } + /* No -D or -D already added during server start */ + if (ctl_command == RESTART_COMMAND || pgdata_opt == NULL) + pgdata_opt = ""; + if (postgres_path == NULL) { char *postmaster_path; *************** *** 1210,1215 **** --- 1216,1224 ---- env_var = xmalloc(len + 8); snprintf(env_var, len + 8, "PGDATA=%s", optarg); putenv(env_var); + /* Show -D for easier postmaster 'ps' identification */ + pgdata_opt = xmalloc(len + 7); + snprintf(pgdata_opt, len + 7, "-D \"%s\" ", optarg); break; } case 'l':