Now that pg_upgrade 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 structure leak that was in the testing loop.

diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h
index 358bf606b187dc5907aaa025025b65e179a55f36..04f67e1e34561c968299bdfac7bd4fdb791eb1bf 100644 (file)
--- a/contrib/pg_upgrade/pg_upgrade.h
+++ b/contrib/pg_upgrade/pg_upgrade.h
@@ -227,7 +227,6 @@ typedef struct
    int         num_tablespaces;
    char      **libraries;      /* loadable libraries */
    int         num_libraries;
-   pgpid_t     postmasterPID;  /* PID of currently running postmaster */
    ClusterInfo *running_cluster;
 } OSInfo;
 

diff --git a/contrib/pg_upgrade/server.c b/contrib/pg_upgrade/server.c
index 0c9914b413e22dea1eae2581a0c33760968945ef..935ce32a611425f91d6a5f1fe477afd1ccaf445a 100644 (file)
--- a/contrib/pg_upgrade/server.c
+++ b/contrib/pg_upgrade/server.c
@@ -9,13 +9,8 @@
 
 #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);
+static PGconn *get_db_conn(ClusterInfo *cluster, const char *db_name);
 
 
 /*
@@ -28,14 +23,7 @@ static bool test_server_conn(ClusterInfo *cluster, int timeout);
 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);
+   PGconn     *conn = get_db_conn(cluster, db_name);
 
    if (conn == NULL || PQstatus(conn) != CONNECTION_OK)
    {
@@ -53,6 +41,24 @@ connectToServer(ClusterInfo *cluster, const char *db_name)
 }
 
 
+/*
+ * 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()
  *
@@ -90,38 +96,6 @@ executeQueryOrDie(PGconn *conn, const char *fmt,...)
 }
 
 
-/*
- * 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()
  *
@@ -169,20 +143,20 @@ void
 start_postmaster(ClusterInfo *cluster)
 {
    char        cmd[MAXPGPATH];
-   const char *bindir;
-   const char *datadir;
-   unsigned short port;
+   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
 
-   bindir = cluster->bindir;
-   datadir = cluster->pgdata;
-   port = cluster->port;
-
    if (!exit_hook_registered)
    {
 #ifdef HAVE_ATEXIT
@@ -194,10 +168,6 @@ start_postmaster(ClusterInfo *cluster)
    }
 
    /*
-    * 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
@@ -207,7 +177,7 @@ start_postmaster(ClusterInfo *cluster)
    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->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",
@@ -215,14 +185,18 @@ start_postmaster(ClusterInfo *cluster)
 
    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\".",
+   /* 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);
 
-   if ((os_info.postmasterPID = get_postmaster_pid(datadir)) == 0)
-       pg_log(PG_FATAL, " Unable to get postmaster pid\n");
    os_info.running_cluster = cluster;
 }
 
@@ -233,6 +207,12 @@ stop_postmaster(bool fast)
    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)
    {
@@ -247,69 +227,18 @@ stop_postmaster(bool fast)
    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
+            bindir, output_filename, datadir, fast ? "-m fast" : "",
+            output_filename);
+
    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()
  *