Improve "pg_ctl -w start" server detection by writing the postmaster

bmomjian · bmomjian · commit 075354ad1bc0 · 2010-12-24T09:45:52.000-05:00
port and socket directory into postmaster.pid, and have pg_ctl read from
that file, for use by PQping().
diff --git a/doc/src/sgml/ref/pg_ctl-ref.sgml b/doc/src/sgml/ref/pg_ctl-ref.sgml
@@ -348,21 +348,12 @@ PostgreSQL documentation
        <para>
         Wait for the startup or shutdown to complete.
         Waiting is the default option for shutdowns, but not startups.
+        When waiting for startup, <command>pg_ctl</command> repeatedly
+        attempts to connect to the server.
         When waiting for shutdown, <command>pg_ctl</command> waits for
         the server to remove its <acronym>PID</acronym> file.
-        When waiting for startup, <command>pg_ctl</command> repeatedly
-        attempts to connect to the server via <application>psql</>, and
-        reports success when this is successful.
-        <command>pg_ctl</command> will attempt to use the proper port for
-        <application>psql</>. If the environment variable
-        <envar>PGPORT</envar> exists, that is used.  Otherwise,
-        <command>pg_ctl</command> will see if a port has been set in the
-        <filename>postgresql.conf</filename> file.  If not, it will use the
-        default port that <productname>PostgreSQL</productname> was compiled
-        with (5432 by default).
-        When waiting, <command>pg_ctl</command> will
-        return an exit code based on the success of the startup
-        or shutdown.
+        <command>pg_ctl</command> returns an exit code based on the
+        success of the startup or shutdown.
        </para>
       </listitem>
      </varlistentry>
@@ -442,28 +433,6 @@ PostgreSQL documentation
     </listitem>
    </varlistentry>
 
-   <varlistentry>
-    <term><envar>PGHOST</envar></term>
-
-    <listitem>
-     <para>
-      Default host name or Unix-domain socket location for <xref
-      linkend="app-psql"> (used when waiting for startup).
-     </para>
-    </listitem>
-   </varlistentry>
-
-   <varlistentry>
-    <term><envar>PGPORT</envar></term>
-
-    <listitem>
-     <para>
-      Default port number for <xref linkend="app-psql">
-      (used when waiting for startup).
-     </para>
-    </listitem>
-   </varlistentry>
-
   </variablelist>
 
   <para>
@@ -506,18 +475,6 @@ PostgreSQL documentation
     </listitem>
    </varlistentry>
 
-   <varlistentry>
-    <term><filename>postgresql.conf</filename></term>
-
-    <listitem>
-     <para>
-      This file, located in the data directory, is parsed to find the
-      proper port to use with <application>psql</application>
-      when waiting for startup.
-     </para>
-    </listitem>
-   </varlistentry>
-
   </variablelist>
  </refsect1>
 
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
@@ -33,6 +33,7 @@
 #include "mb/pg_wchar.h"
 #include "miscadmin.h"
 #include "postmaster/autovacuum.h"
+#include "postmaster/postmaster.h"
 #include "storage/fd.h"
 #include "storage/ipc.h"
 #include "storage/pg_shmem.h"
@@ -658,7 +659,7 @@ CreateLockFile(const char *filename, bool amPostmaster,
 			   bool isDDLock, const char *refName)
 {
 	int			fd;
-	char		buffer[MAXPGPATH + 100];
+	char		buffer[MAXPGPATH * 2 + 256];
 	int			ntries;
 	int			len;
 	int			encoded_pid;
@@ -868,9 +869,9 @@ CreateLockFile(const char *filename, bool amPostmaster,
 	/*
 	 * Successfully created the file, now fill it.
 	 */
-	snprintf(buffer, sizeof(buffer), "%d\n%s\n",
+	snprintf(buffer, sizeof(buffer), "%d\n%s\n%d\n%s\n",
 			 amPostmaster ? (int) my_pid : -((int) my_pid),
-			 DataDir);
+			 DataDir, PostPortNumber, UnixSocketDir);
 	errno = 0;
 	if (write(fd, buffer, strlen(buffer)) != strlen(buffer))
 	{
@@ -994,8 +995,9 @@ RecordSharedMemoryInLockFile(unsigned long id1, unsigned long id2)
 {
 	int			fd;
 	int			len;
+	int			lineno;
 	char	   *ptr;
-	char		buffer[BLCKSZ];
+	char		buffer[MAXPGPATH * 2 + 256];
 
 	fd = open(DIRECTORY_LOCK_FILE, O_RDWR | PG_BINARY, 0);
 	if (fd < 0)
@@ -1019,18 +1021,20 @@ RecordSharedMemoryInLockFile(unsigned long id1, unsigned long id2)
 	buffer[len] = '\0';
 
 	/*
-	 * Skip over first two lines (PID and path).
+	 * Skip over first four lines (PID, pgdata, portnum, socketdir).
 	 */
-	ptr = strchr(buffer, '\n');
-	if (ptr == NULL ||
-		(ptr = strchr(ptr + 1, '\n')) == NULL)
+	ptr = buffer;
+	for (lineno = 1; lineno <= 4; lineno++)
 	{
-		elog(LOG, "bogus data in \"%s\"", DIRECTORY_LOCK_FILE);
-		close(fd);
-		return;
+		if ((ptr = strchr(ptr, '\n')) == NULL)
+		{
+			elog(LOG, "bogus data in \"%s\"", DIRECTORY_LOCK_FILE);
+			close(fd);
+			return;
+		}
+		ptr++;
 	}
-	ptr++;
-
+	
 	/*
 	 * Append key information.	Format to try to keep it the same length
 	 * always (trailing junk won't hurt, but might confuse humans).
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
@@ -141,7 +141,6 @@ static bool postmaster_is_alive(pid_t pid);
 
 static char postopts_file[MAXPGPATH];
 static char pid_file[MAXPGPATH];
-static char conf_file[MAXPGPATH];
 static char backup_file[MAXPGPATH];
 static char recovery_file[MAXPGPATH];
 
@@ -404,113 +403,108 @@ start_postmaster(void)
 static PGPing
 test_postmaster_connection(bool do_checkpoint)
 {
+	int			portnum = 0;
+	char		socket_dir[MAXPGPATH];
+	char		connstr[MAXPGPATH + 256];
 	PGPing		ret = PQPING_OK;	/* assume success for wait == zero */
+	char	  **optlines;
 	int			i;
-	char		portstr[32];
-	char	   *p;
-	char	   *q;
-	char		connstr[128];	/* Should be way more than enough! */
 
-	portstr[0] = '\0';
-
-	/*
-	 * Look in post_opts for a -p switch.
-	 *
-	 * This parsing code is not amazingly bright; it could for instance get
-	 * fooled if ' -p' occurs within a quoted argument value.  Given that few
-	 * people pass complicated settings in post_opts, it's probably good
-	 * enough.
-	 */
-	for (p = post_opts; *p;)
+	socket_dir[0] = '\0';
+	connstr[0] = '\0';
+	
+	for (i = 0; i < wait_seconds; i++)
 	{
-		/* advance past whitespace */
-		while (isspace((unsigned char) *p))
-			p++;
-
-		if (strncmp(p, "-p", 2) == 0)
+		/* Do we need a connection string? */
+		if (connstr[0] == '\0')
 		{
-			p += 2;
-			/* advance past any whitespace/quoting */
-			while (isspace((unsigned char) *p) || *p == '\'' || *p == '"')
-				p++;
-			/* find end of value (not including any ending quote!) */
-			q = p;
-			while (*q &&
-				   !(isspace((unsigned char) *q) || *q == '\'' || *q == '"'))
-				q++;
-			/* and save the argument value */
-			strlcpy(portstr, p, Min((q - p) + 1, sizeof(portstr)));
-			/* keep looking, maybe there is another -p */
-			p = q;
-		}
-		/* Advance to next whitespace */
-		while (*p && !isspace((unsigned char) *p))
-			p++;
-	}
+			/*
+			 * 	The number of lines in postmaster.pid tells us several things:
+			 *
+			 *	# of lines
+			 *		0	lock file created but status not written
+			 *		2	pre-9.1 server, shared memory not created
+			 *		3	pre-9.1 server, shared memory created
+			 *		4	9.1+ server, shared memory not created
+			 *		5	9.1+ server, shared memory created
+			 *
+			 *	For pre-9.1 Unix servers, we grab the port number from the
+			 *	shmem key (first value on line 3).  Pre-9.1 Win32 has no
+			 *	written shmem key, so we fail.  9.1+ writes both the port
+			 *	number and socket address in the file for us to use.
+			 *	(PG_VERSION could also have told us the major version.)
+			 */
+		
+			/* Try to read a completed postmaster.pid file */
+			if ((optlines = readfile(pid_file)) != NULL &&
+				optlines[0] != NULL &&
+				optlines[1] != NULL &&
+				optlines[2] != NULL)
+			{				
+				/* A 3-line file? */
+				if (optlines[3] == NULL)
+				{
+					/*
+					 *	Pre-9.1:  on Unix, we get the port number by
+					 *	deriving it from the shmem key (the first number on
+					 *	on the line);  see
+					 *	miscinit.c::RecordSharedMemoryInLockFile().
+					 */
+					portnum = atoi(optlines[2]) / 1000;
+					/* Win32 does not give us a shmem key, so we fail. */
+					if (portnum == 0)
+					{
+						write_stderr(_("%s: -w option is not supported on this platform\nwhen connecting to a pre-9.1 server\n"),
+									 progname);
+						return PQPING_NO_ATTEMPT;
+					}
+				}
+				else	/* 9.1+ server */
+				{
+					portnum = atoi(optlines[2]);
+	
+					/* Get socket directory, if specified. */
+					if (optlines[3][0] != '\n')
+					{
+						/*
+						 *	While unix_socket_directory can accept relative
+						 *	directories, libpq's host must have a leading slash
+						 *	to indicate a socket directory.
+						 */
+						if (optlines[3][0] != '/')
+						{
+							write_stderr(_("%s: -w option cannot use a relative socket directory specification\n"),
+										 progname);
+							return PQPING_NO_ATTEMPT;
+						}
+						strlcpy(socket_dir, optlines[3], MAXPGPATH);
+						/* remove newline */
+						if (strchr(socket_dir, '\n') != NULL)
+							*strchr(socket_dir, '\n') = '\0';
+					}
+				}
 
-	/*
-	 * Search config file for a 'port' option.
-	 *
-	 * This parsing code isn't amazingly bright either, but it should be okay
-	 * for valid port settings.
-	 */
-	if (!portstr[0])
-	{
-		char	  **optlines;
+				/*
+				 * We need to set connect_timeout otherwise on Windows the
+				 * Service Control Manager (SCM) will probably timeout first.
+				 */
+				snprintf(connstr, sizeof(connstr),
+						 "dbname=postgres port=%d connect_timeout=5", portnum);
 
-		optlines = readfile(conf_file);
-		if (optlines != NULL)
-		{
-			for (; *optlines != NULL; optlines++)
-			{
-				p = *optlines;
-
-				while (isspace((unsigned char) *p))
-					p++;
-				if (strncmp(p, "port", 4) != 0)
-					continue;
-				p += 4;
-				while (isspace((unsigned char) *p))
-					p++;
-				if (*p != '=')
-					continue;
-				p++;
-				/* advance past any whitespace/quoting */
-				while (isspace((unsigned char) *p) || *p == '\'' || *p == '"')
-					p++;
-				/* find end of value (not including any ending quote/comment!) */
-				q = p;
-				while (*q &&
-					   !(isspace((unsigned char) *q) ||
-						 *q == '\'' || *q == '"' || *q == '#'))
-					q++;
-				/* and save the argument value */
-				strlcpy(portstr, p, Min((q - p) + 1, sizeof(portstr)));
-				/* keep looking, maybe there is another */
+				if (socket_dir[0] != '\0')
+					snprintf(connstr + strlen(connstr), sizeof(connstr) - strlen(connstr),
+						" host='%s'", socket_dir);
 			}
 		}
-	}
 
-	/* Check environment */
-	if (!portstr[0] && getenv("PGPORT") != NULL)
-		strlcpy(portstr, getenv("PGPORT"), sizeof(portstr));
-
-	/* Else use compiled-in default */
-	if (!portstr[0])
-		snprintf(portstr, sizeof(portstr), "%d", DEF_PGPORT);
-
-	/*
-	 * We need to set a connect timeout otherwise on Windows the SCM will
-	 * probably timeout first
-	 */
-	snprintf(connstr, sizeof(connstr),
-			 "dbname=postgres port=%s connect_timeout=5", portstr);
+		/* If we have a connection string, ping the server */
+		if (connstr[0] != '\0')
+		{
+			ret = PQping(connstr);
+			if (ret == PQPING_OK || ret == PQPING_NO_ATTEMPT)
+				break;
+		}
 
-	for (i = 0; i < wait_seconds; i++)
-	{
-		ret = PQping(connstr);
-		if (ret == PQPING_OK || ret == PQPING_NO_ATTEMPT)
-			break;
 		/* No response, or startup still in process; wait */
 #if defined(WIN32)
 		if (do_checkpoint)
@@ -2009,7 +2003,6 @@ main(int argc, char **argv)
 	{
 		snprintf(postopts_file, MAXPGPATH, "%s/postmaster.opts", pg_data);
 		snprintf(pid_file, MAXPGPATH, "%s/postmaster.pid", pg_data);
-		snprintf(conf_file, MAXPGPATH, "%s/postgresql.conf", pg_data);
 		snprintf(backup_file, MAXPGPATH, "%s/backup_label", pg_data);
 		snprintf(recovery_file, MAXPGPATH, "%s/recovery.conf", pg_data);
 	}
