Thread: Re: [HACKERS] Fix PID file location?

Re: [HACKERS] Fix PID file location?

From
Bruce Momjian
Date:
I have generated the following patch that moves postmaster.pid into the
configuration directory.  pg_ctl only knows about the configuration
directory because it can't read postgresql.conf, so it seems that is the
right place to move it.

I have tested it and it seems to work. I would like to backpatch this to
8.0.X because it is currently broken without it.  This patch does
require that the postgres unix user have write permission in the
configuration directory to create the pid file on startup.

---------------------------------------------------------------------------

Josh Berkus wrote:
> Hey, folks,
>
> I've noticed a problem with alternate PGDATA locations.  Here's how to
> reproduce:
>
> On 8.0.2 on RHAS4:
>
> 1) Initdb a directory (on my system, /pgdata/pgdata)
> 2) Move the .conf files to an alternate location ( /etc/pgsql/)
> 3) Set $PGDATA to the alternate location ( /etc/pgsql )
> 4) Edit postgresql.conf to support this file arrangement
>     data_directory = '/pgdata/pgdata'
> 5) pg_ctl start PostgreSQL
> 6) pg_ctl stop PostgreSQL
> 7) Get an error:  "No PID file found".
>
> The problem seems to be that pg_ctl expects the PID file to be in $PGDATA, but
> the file actually gets written by the postmaster to the actual data
> directory.  You can work around this by setting "external_pid_file", but this
> then prevents you from using external_pid_file for another purpose.
>
> Seems like it should be a relatively easy fix, although I'm not sure whether
> the postmaster should write the PID to $PGDATA, or whether pg_ctl should be
> made to look in the right place.  Probably the latter.

---------------------------------------------------------------------------

> More about this: due to the PID file not being in the right place, pg_ctl stop
> never reports success:
>
> waiting for postmaster to shut
> down............................................................... failed
> pg_ctl: postmaster does not shut down
>
> This appears to be because the duplicate PID in the conf directory is not
> removed on shutdown.


--
  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: doc/src/sgml/runtime.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/runtime.sgml,v
retrieving revision 1.321
diff -c -c -r1.321 runtime.sgml
*** doc/src/sgml/runtime.sgml    25 May 2005 02:56:15 -0000    1.321
--- doc/src/sgml/runtime.sgml    27 May 2005 20:46:42 -0000
***************
*** 309,318 ****
     <para>
      While the <command>postmaster</command> is running, its
      <acronym>PID</acronym> is stored in the file
!     <filename>postmaster.pid</filename> in the data directory. This is
!     used to prevent multiple <command>postmaster</command> processes
!     running in the same data directory and can also be used for
!     shutting down the <command>postmaster</command> process.
     </para>

     <sect2 id="postmaster-start-failures">
--- 309,319 ----
     <para>
      While the <command>postmaster</command> is running, its
      <acronym>PID</acronym> is stored in the file
!     <filename>postmaster.pid</filename> in the data or configuration
!     directory. This is used to prevent multiple
!     <command>postmaster</command> processes running in the same data
!     directory and can also be used for shutting down the
!     <command>postmaster</command> process.
     </para>

     <sect2 id="postmaster-start-failures">
***************
*** 4933,4940 ****
     Alternatively, you can send the signal directly using <command>kill</>.
     The <acronym>PID</> of the <command>postmaster</command> process can be
     found using the <command>ps</command> program, or from the file
!    <filename>postmaster.pid</filename> in the data directory. For
!    example, to do a fast shutdown:
  <screen>
  $ <userinput>kill -INT `head -1 /usr/local/pgsql/data/postmaster.pid`</userinput>
  </screen>
--- 4934,4941 ----
     Alternatively, you can send the signal directly using <command>kill</>.
     The <acronym>PID</> of the <command>postmaster</command> process can be
     found using the <command>ps</command> program, or from the file
!    <filename>postmaster.pid</filename> in the data or configuration directory.
!    For example, to do a fast shutdown:
  <screen>
  $ <userinput>kill -INT `head -1 /usr/local/pgsql/data/postmaster.pid`</userinput>
  </screen>
Index: src/backend/bootstrap/bootstrap.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/bootstrap/bootstrap.c,v
retrieving revision 1.204
diff -c -c -r1.204 bootstrap.c
*** src/backend/bootstrap/bootstrap.c    6 May 2005 17:24:52 -0000    1.204
--- src/backend/bootstrap/bootstrap.c    27 May 2005 20:46:43 -0000
***************
*** 367,373 ****

      /* If standalone, create lockfile for data directory */
      if (!IsUnderPostmaster)
!         CreateDataDirLockFile(DataDir, false);

      SetProcessingMode(BootstrapProcessing);
      IgnoreSystemIndexes(true);
--- 367,373 ----

      /* If standalone, create lockfile for data directory */
      if (!IsUnderPostmaster)
!         CreateDataDirLockFile(userDoption, false);

      SetProcessingMode(BootstrapProcessing);
      IgnoreSystemIndexes(true);
Index: src/backend/postmaster/postmaster.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v
retrieving revision 1.451
diff -c -c -r1.451 postmaster.c
*** src/backend/postmaster/postmaster.c    15 May 2005 00:26:18 -0000    1.451
--- src/backend/postmaster/postmaster.c    27 May 2005 20:46:45 -0000
***************
*** 689,695 ****
       * :-(). For the same reason, it's best to grab the TCP socket(s)
       * before the Unix socket.
       */
!     CreateDataDirLockFile(DataDir, true);

      /*
       * Remove old temporary files.    At this point there can be no other
--- 689,695 ----
       * :-(). For the same reason, it's best to grab the TCP socket(s)
       * before the Unix socket.
       */
!     CreateDataDirLockFile(userDoption, true);

      /*
       * Remove old temporary files.    At this point there can be no other
Index: src/backend/tcop/postgres.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/tcop/postgres.c,v
retrieving revision 1.444
diff -c -c -r1.444 postgres.c
*** src/backend/tcop/postgres.c    24 May 2005 04:18:04 -0000    1.444
--- src/backend/tcop/postgres.c    27 May 2005 20:46:47 -0000
***************
*** 2795,2801 ****
          /*
           * Create lockfile for data directory.
           */
!         CreateDataDirLockFile(DataDir, false);

          XLOGPathInit();
          BaseInit();
--- 2795,2801 ----
          /*
           * Create lockfile for data directory.
           */
!         CreateDataDirLockFile(userDoption, false);

          XLOGPathInit();
          BaseInit();
Index: src/backend/utils/init/miscinit.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/init/miscinit.c,v
retrieving revision 1.139
diff -c -c -r1.139 miscinit.c
*** src/backend/utils/init/miscinit.c    14 Apr 2005 20:03:26 -0000    1.139
--- src/backend/utils/init/miscinit.c    27 May 2005 20:46:47 -0000
***************
*** 706,719 ****
  }

  void
! CreateDataDirLockFile(const char *datadir, bool amPostmaster)
  {
      char        lockfile[MAXPGPATH];

!     snprintf(lockfile, sizeof(lockfile), "%s/postmaster.pid", datadir);
!     CreateLockFile(lockfile, amPostmaster, true, datadir);
      /* Save name of lockfile for RecordSharedMemoryInLockFile */
      strcpy(directoryLockFile, lockfile);
  }

  void
--- 706,733 ----
  }

  void
! CreateDataDirLockFile(const char *userDoption, bool amPostmaster)
  {
      char        lockfile[MAXPGPATH];

!     char       *configdir;
!
!     /*    configdir is -D option, or $PGDATA if no -D.
!      *    The pid file has to be created in the -D/PGDATA
!      *    location because that is the only location pg_ctl
!      *    knows about.
!      */
!     if (userDoption)
!         configdir = make_absolute_path(userDoption);
!     else
!         configdir = make_absolute_path(getenv("PGDATA"));
!
!     snprintf(lockfile, sizeof(lockfile), "%s/postmaster.pid", configdir);
!     CreateLockFile(lockfile, amPostmaster, true, configdir);
      /* Save name of lockfile for RecordSharedMemoryInLockFile */
      strcpy(directoryLockFile, lockfile);
+
+     free(configdir);
  }

  void
Index: src/include/miscadmin.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/miscadmin.h,v
retrieving revision 1.175
diff -c -c -r1.175 miscadmin.h
*** src/include/miscadmin.h    26 Feb 2005 18:43:34 -0000    1.175
--- src/include/miscadmin.h    27 May 2005 20:46:49 -0000
***************
*** 309,315 ****
  extern void ResetReindexProcessing(void);
  extern bool ReindexIsProcessingHeap(Oid heapOid);
  extern bool ReindexIsProcessingIndex(Oid indexOid);
! extern void CreateDataDirLockFile(const char *datadir, bool amPostmaster);
  extern void CreateSocketLockFile(const char *socketfile, bool amPostmaster);
  extern void TouchSocketLockFile(void);
  extern void RecordSharedMemoryInLockFile(unsigned long id1,
--- 309,315 ----
  extern void ResetReindexProcessing(void);
  extern bool ReindexIsProcessingHeap(Oid heapOid);
  extern bool ReindexIsProcessingIndex(Oid indexOid);
! extern void CreateDataDirLockFile(const char *userDoption, bool amPostmaster);
  extern void CreateSocketLockFile(const char *socketfile, bool amPostmaster);
  extern void TouchSocketLockFile(void);
  extern void RecordSharedMemoryInLockFile(unsigned long id1,

Re: [HACKERS] Fix PID file location?

From
Peter Eisentraut
Date:
Bruce Momjian wrote:
> I have generated the following patch that moves postmaster.pid into
> the configuration directory.  pg_ctl only knows about the
> configuration directory because it can't read postgresql.conf, so it
> seems that is the right place to move it.

Files that are not actually configuration files, to be edited by users,
do not belong in the configuration directory.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

Re: [HACKERS] Fix PID file location?

From
"Andrew Dunstan"
Date:
Bruce Momjian said:
>
> I have generated the following patch that moves postmaster.pid into the
> configuration directory.  pg_ctl only knows about the configuration
> directory because it can't read postgresql.conf, so it seems that is
> the right place to move it.

this seems wrong ... wouldn't it be better to teach pg_ctl at least enough
about the config file to enable it to find the data dir?

cheers

andrew






Re: [HACKERS] Fix PID file location?

From
Josh Berkus
Date:
Peter,

> Files that are not actually configuration files, to be edited by users,
> do not belong in the configuration directory.

Do you have an alternate suggestion then?   Current behavior is broken.

--
--Josh

Josh Berkus
Aglio Database Solutions
San Francisco

Re: [HACKERS] Fix PID file location?

From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> I have generated the following patch that moves postmaster.pid into the
> configuration directory.  pg_ctl only knows about the configuration
> directory because it can't read postgresql.conf, so it seems that is the
> right place to move it.

Unfortunately, that is *absolutely* unsafe.  If we do that it will break
the safety property that the lock file is meant to enforce in the first
place, namely only one postmaster running in a given data directory.
It's not too hard to imagine people getting burnt by that, either:
initdb, create new config files in another directory, forget to remove
the original postgresql.conf in the data directory, and you have every
ingredient needed for disaster.  Just start two postmasters with both
direct and indirect -D arguments, and kaboom.

> This patch does
> require that the postgres unix user have write permission in the
> configuration directory to create the pid file on startup.

That assumption is unacceptable, too.  One of the prime reasons for
having config files somewhere else is that the somewhere else can be
read-only, thus reducing your exposure in case of a security breach.
(Otherwise, we could possibly fix this by generating a second
postmaster.pid in the config directory.)

I really think we have only two choices: teach pg_ctl how to dig the
data directory location out of postgresql.conf, or revert the
separate-config-file-location patch.

            regards, tom lane

Re: [HACKERS] Fix PID file location?

From
Alvaro Herrera
Date:
On Fri, May 27, 2005 at 07:40:17PM -0400, Tom Lane wrote:

> I really think we have only two choices: teach pg_ctl how to dig the
> data directory location out of postgresql.conf,

I don't think this is extremely hard, isn't it?

--
Alvaro Herrera (<alvherre[a]surnet.cl>)
"El realista sabe lo que quiere; el idealista quiere lo que sabe" (Anónimo)

Re: [HACKERS] Fix PID file location?

From
Tom Lane
Date:
Alvaro Herrera <alvherre@surnet.cl> writes:
> On Fri, May 27, 2005 at 07:40:17PM -0400, Tom Lane wrote:
>> I really think we have only two choices: teach pg_ctl how to dig the
>> data directory location out of postgresql.conf,

> I don't think this is extremely hard, isn't it?

One small problem is that I think the current definition allows the data
directory to be specified relative to the original postmaster working
directory.  Of course, that's not different from "-D ." on the
postmaster command line, so possibly the answer is "if you want to use
pg_ctl, don't do that".

            regards, tom lane

Re: [HACKERS] Fix PID file location?

From
Bruce Momjian
Date:
Tom Lane wrote:
> Alvaro Herrera <alvherre@surnet.cl> writes:
> > On Fri, May 27, 2005 at 07:40:17PM -0400, Tom Lane wrote:
> >> I really think we have only two choices: teach pg_ctl how to dig the
> >> data directory location out of postgresql.conf,
>
> > I don't think this is extremely hard, isn't it?
>
> One small problem is that I think the current definition allows the data
> directory to be specified relative to the original postmaster working
> directory.  Of course, that's not different from "-D ." on the
> postmaster command line, so possibly the answer is "if you want to use
> pg_ctl, don't do that".

I don't see any way to accurately find the data directory location.
Reading postgresql.conf is one way, but what if they set data_directory
on the command line using postmaster -o?  Is reading postgresql.conf
from pg_ctl without a parser really accurate?  Shame we can't attach and
do "SHOW data_directory" on a backend.

--
  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

Re: [HACKERS] Fix PID file location?

From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Is reading postgresql.conf
> from pg_ctl without a parser really accurate?

The brute-force solution is to duplicate guc-file.l.

That seems pretty ugly but in the long run it might be the most
maintainable solution.  We eventually gave up trying to have a
cut-rate SQL lexer in psql, and duplicated parser/scan.l.
Might be best to just go for that solution up front in this case.

            regards, tom lane

Re: [HACKERS] Fix PID file location?

From
Bruce Momjian
Date:
Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Is reading postgresql.conf
> > from pg_ctl without a parser really accurate?
>
> The brute-force solution is to duplicate guc-file.l.
>
> That seems pretty ugly but in the long run it might be the most
> maintainable solution.  We eventually gave up trying to have a
> cut-rate SQL lexer in psql, and duplicated parser/scan.l.
> Might be best to just go for that solution up front in this case.

Added to TODO:

> * Allow pg_ctl to work properly with configuration files located outside
>   the PGDATA directory
>
>   pg_ctl can not read the pid file because it isn't located in the
>   config directory but in the PGDATA directory.  The solution is to
>   allow pg_ctl to read and understand postgresql.conf to find the
>   data_directory value.

--
  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