From: Bruce Momjian Date: Tue, 26 Apr 2011 14:15:18 +0000 (-0400) Subject: Now that pg_upgrade uses -w in pg_ctl, remove loop that retried testing X-Git-Tag: REL9_1_BETA1~16 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=6c4d2bd9141034bd27977794f52003fd6f7d01f0;p=postgresql 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 358bf606b1..04f67e1e34 100644 --- 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 0c9914b413..935ce32a61 100644 --- 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() *