The attached, applied patch changes pg_upgrade:  now that it uses -w in
pg_ctl, remove loop that retried testing the connection;  also
restructure the libpq connection code.

This patch also removes the unused variable postmasterPID and fixes a
libpq PQconn structure leak that was in the testing loop.

Yeah, I got a fix into that patch.  ;-)

-- 
  Bruce Momjian  <br...@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + It's impossible for everything to be true. +
diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h
new file mode 100644
index 358bf60..04f67e1
*** a/contrib/pg_upgrade/pg_upgrade.h
--- b/contrib/pg_upgrade/pg_upgrade.h
*************** typedef struct
*** 227,233 ****
  	int			num_tablespaces;
  	char	  **libraries;		/* loadable libraries */
  	int			num_libraries;
- 	pgpid_t		postmasterPID;	/* PID of currently running postmaster */
  	ClusterInfo *running_cluster;
  } OSInfo;
  
--- 227,232 ----
diff --git a/contrib/pg_upgrade/server.c b/contrib/pg_upgrade/server.c
new file mode 100644
index 0c9914b..935ce32
*** a/contrib/pg_upgrade/server.c
--- b/contrib/pg_upgrade/server.c
***************
*** 9,21 ****
  
  #include "pg_upgrade.h"
  
- #define POSTMASTER_UPTIME 20
- 
- #define STARTUP_WARNING_TRIES 2
  
! 
! static pgpid_t get_postmaster_pid(const char *datadir);
! static bool test_server_conn(ClusterInfo *cluster, int timeout);
  
  
  /*
--- 9,16 ----
  
  #include "pg_upgrade.h"
  
  
! static PGconn *get_db_conn(ClusterInfo *cluster, const char *db_name);
  
  
  /*
*************** static bool test_server_conn(ClusterInfo
*** 28,41 ****
  PGconn *
  connectToServer(ClusterInfo *cluster, const char *db_name)
  {
! 	unsigned short port = cluster->port;
! 	char		connectString[MAXPGPATH];
! 	PGconn	   *conn;
! 
! 	snprintf(connectString, sizeof(connectString),
! 		 "dbname = '%s' user = '%s' port = %d", db_name, os_info.user, port);
! 
! 	conn = PQconnectdb(connectString);
  
  	if (conn == NULL || PQstatus(conn) != CONNECTION_OK)
  	{
--- 23,29 ----
  PGconn *
  connectToServer(ClusterInfo *cluster, const char *db_name)
  {
! 	PGconn	   *conn = get_db_conn(cluster, db_name);
  
  	if (conn == NULL || PQstatus(conn) != CONNECTION_OK)
  	{
*************** connectToServer(ClusterInfo *cluster, co
*** 54,59 ****
--- 42,65 ----
  
  
  /*
+  * get_db_conn()
+  *
+  * get database connection
+  */
+ static PGconn *
+ get_db_conn(ClusterInfo *cluster, const char *db_name)
+ {
+ 	char		conn_opts[MAXPGPATH];
+ 
+ 	snprintf(conn_opts, sizeof(conn_opts),
+ 		 "dbname = '%s' user = '%s' port = %d", db_name, os_info.user,
+ 		 cluster->port);
+ 
+ 	return PQconnectdb(conn_opts);
+ }
+ 
+ 
+ /*
   * executeQueryOrDie()
   *
   *	Formats a query string from the given arguments and executes the
*************** executeQueryOrDie(PGconn *conn, const ch
*** 91,128 ****
  
  
  /*
-  * get_postmaster_pid()
-  *
-  * Returns the pid of the postmaster running on datadir. pid is retrieved
-  * from the postmaster.pid file
-  */
- static pgpid_t
- get_postmaster_pid(const char *datadir)
- {
- 	FILE	   *pidf;
- 	long		pid;
- 	char		pid_file[MAXPGPATH];
- 
- 	snprintf(pid_file, sizeof(pid_file), "%s/postmaster.pid", datadir);
- 	pidf = fopen(pid_file, "r");
- 
- 	if (pidf == NULL)
- 		return (pgpid_t) 0;
- 
- 	if (fscanf(pidf, "%ld", &pid) != 1)
- 	{
- 		fclose(pidf);
- 		pg_log(PG_FATAL, "%s: invalid data in PID file \"%s\"\n",
- 			   os_info.progname, pid_file);
- 	}
- 
- 	fclose(pidf);
- 
- 	return (pgpid_t) pid;
- }
- 
- 
- /*
   * get_major_server_version()
   *
   * gets the version (in unsigned int form) for the given "datadir". Assumes
--- 97,102 ----
*************** void
*** 169,188 ****
  start_postmaster(ClusterInfo *cluster)
  {
  	char		cmd[MAXPGPATH];
! 	const char *bindir;
! 	const char *datadir;
! 	unsigned short port;
  	bool		exit_hook_registered = false;
  #ifndef WIN32
  	char		*output_filename = log_opts.filename;
  #else
  	char		*output_filename = DEVNULL;
  #endif
  
- 	bindir = cluster->bindir;
- 	datadir = cluster->pgdata;
- 	port = cluster->port;
- 
  	if (!exit_hook_registered)
  	{
  #ifdef HAVE_ATEXIT
--- 143,162 ----
  start_postmaster(ClusterInfo *cluster)
  {
  	char		cmd[MAXPGPATH];
! 	PGconn	   *conn;
  	bool		exit_hook_registered = false;
  #ifndef WIN32
  	char		*output_filename = log_opts.filename;
  #else
+ 	/*
+ 	 * On Win32, we can't send both pg_upgrade output and pg_ctl output to the
+ 	 * same file because we get the error: "The process cannot access the file
+ 	 * because it is being used by another process." so we have to send all
+ 	 * other output to 'nul'.
+ 	 */
  	char		*output_filename = DEVNULL;
  #endif
  
  	if (!exit_hook_registered)
  	{
  #ifdef HAVE_ATEXIT
*************** start_postmaster(ClusterInfo *cluster)
*** 194,203 ****
  	}
  
  	/*
- 	 * On Win32, we can't send both pg_upgrade output and pg_ctl output to the
- 	 * same file because we get the error: "The process cannot access the file
- 	 * because it is being used by another process." so we have to send all
- 	 * other output to 'nul'.
  	 * Using autovacuum=off disables cleanup vacuum and analyze, but freeze
  	 * vacuums can still happen, so we set autovacuum_freeze_max_age to its
  	 * maximum.  We assume all datfrozenxid and relfrozen values are less than
--- 168,173 ----
*************** start_postmaster(ClusterInfo *cluster)
*** 207,213 ****
  	snprintf(cmd, sizeof(cmd),
  			 SYSTEMQUOTE "\"%s/pg_ctl\" -w -l \"%s\" -D \"%s\" "
  			 "-o \"-p %d %s\" start >> \"%s\" 2>&1" SYSTEMQUOTE,
! 			 bindir, output_filename, datadir, port,
  			 (cluster->controldata.cat_ver >=
  				BINARY_UPGRADE_SERVER_FLAG_CAT_VER) ? "-b" :
  				"-c autovacuum=off -c autovacuum_freeze_max_age=2000000000",
--- 177,183 ----
  	snprintf(cmd, sizeof(cmd),
  			 SYSTEMQUOTE "\"%s/pg_ctl\" -w -l \"%s\" -D \"%s\" "
  			 "-o \"-p %d %s\" start >> \"%s\" 2>&1" SYSTEMQUOTE,
! 			 cluster->bindir, output_filename, cluster->pgdata, cluster->port,
  			 (cluster->controldata.cat_ver >=
  				BINARY_UPGRADE_SERVER_FLAG_CAT_VER) ? "-b" :
  				"-c autovacuum=off -c autovacuum_freeze_max_age=2000000000",
*************** start_postmaster(ClusterInfo *cluster)
*** 215,228 ****
  
  	exec_prog(true, "%s", cmd);
  
! 	/* wait for the server to start properly */
! 
! 	if (test_server_conn(cluster, POSTMASTER_UPTIME) == false)
! 		pg_log(PG_FATAL, " Unable to start %s postmaster with the command: %s\nPerhaps pg_hba.conf was not set to \"trust\".",
  			   CLUSTER_NAME(cluster), cmd);
  
- 	if ((os_info.postmasterPID = get_postmaster_pid(datadir)) == 0)
- 		pg_log(PG_FATAL, " Unable to get postmaster pid\n");
  	os_info.running_cluster = cluster;
  }
  
--- 185,202 ----
  
  	exec_prog(true, "%s", cmd);
  
! 	/* Check to see if we can connect to the server; if not, report it. */
! 	if ((conn = get_db_conn(cluster, "template1")) == NULL ||
! 		PQstatus(conn) != CONNECTION_OK)
! 	{
! 		if (conn)
! 			PQfinish(conn);
! 		pg_log(PG_FATAL, "unable to connect to %s postmaster started with the command: %s\n"
! 			   "Perhaps pg_hba.conf was not set to \"trust\".",
  			   CLUSTER_NAME(cluster), cmd);
+ 	}
+ 	PQfinish(conn);
  
  	os_info.running_cluster = cluster;
  }
  
*************** stop_postmaster(bool fast)
*** 233,238 ****
--- 207,218 ----
  	char		cmd[MAXPGPATH];
  	const char *bindir;
  	const char *datadir;
+ #ifndef WIN32
+ 	char		*output_filename = log_opts.filename;
+ #else
+ 	/* See comment in start_postmaster() about why win32 output is ignored. */
+ 	char		*output_filename = DEVNULL;
+ #endif
  
  	if (os_info.running_cluster == &old_cluster)
  	{
*************** stop_postmaster(bool fast)
*** 247,316 ****
  	else
  		return;					/* no cluster running */
  
- 	/* See comment in start_postmaster() about why win32 output is ignored. */
  	snprintf(cmd, sizeof(cmd),
  			 SYSTEMQUOTE "\"%s/pg_ctl\" -w -l \"%s\" -D \"%s\" %s stop >> "
  			 "\"%s\" 2>&1" SYSTEMQUOTE,
! 			 bindir,
! #ifndef WIN32
! 	   log_opts.filename, datadir, fast ? "-m fast" : "", log_opts.filename);
! #else
! 			 DEVNULL, datadir, fast ? "-m fast" : "", DEVNULL);
! #endif
  	exec_prog(fast ? false : true, "%s", cmd);
  
- 	os_info.postmasterPID = 0;
  	os_info.running_cluster = NULL;
  }
  
  
  /*
-  * test_server_conn()
-  *
-  * tests whether postmaster is running or not by trying to connect
-  * to it. If connection is unsuccessfull we do a sleep of 1 sec and then
-  * try the connection again. This process continues "timeout" times.
-  *
-  * Returns true if the connection attempt was successfull, false otherwise.
-  */
- static bool
- test_server_conn(ClusterInfo *cluster, int timeout)
- {
- 	unsigned short port = cluster->port;
- 	PGconn	   *conn = NULL;
- 	char		con_opts[MAX_STRING];
- 	int			tries;
- 	bool		ret = false;
- 
- 	snprintf(con_opts, sizeof(con_opts),
- 		  "dbname = 'template1' user = '%s' port = %d ", os_info.user, port);
- 
- 	for (tries = 0; tries < timeout; tries++)
- 	{
- 		sleep(1);
- 		if ((conn = PQconnectdb(con_opts)) != NULL &&
- 			PQstatus(conn) == CONNECTION_OK)
- 		{
- 			PQfinish(conn);
- 			ret = true;
- 			break;
- 		}
- 
- 		if (tries == STARTUP_WARNING_TRIES)
- 			prep_status("Trying to start %s server ",
- 						CLUSTER_NAME(cluster));
- 		else if (tries > STARTUP_WARNING_TRIES)
- 			pg_log(PG_REPORT, ".");
- 	}
- 
- 	if (tries > STARTUP_WARNING_TRIES)
- 		check_ok();
- 
- 	return ret;
- }
- 
- 
- /*
   * check_for_libpq_envvars()
   *
   * tests whether any libpq environment variables are set.
--- 227,245 ----
  	else
  		return;					/* no cluster running */
  
  	snprintf(cmd, sizeof(cmd),
  			 SYSTEMQUOTE "\"%s/pg_ctl\" -w -l \"%s\" -D \"%s\" %s stop >> "
  			 "\"%s\" 2>&1" SYSTEMQUOTE,
! 			 bindir, output_filename, datadir, fast ? "-m fast" : "",
! 			 output_filename);
! 
  	exec_prog(fast ? false : true, "%s", cmd);
  
  	os_info.running_cluster = NULL;
  }
  
  
  /*
   * check_for_libpq_envvars()
   *
   * tests whether any libpq environment variables are set.
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to