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