Not sure what the normal process is for patches, but I put together a
few small patches for pg_upgrade after trying to use it earlier today
and staring a non-helpful error message before I finally figured out
what was going on.

0001 is just a simple typo fix, but didn't want to mix it in with the rest.
0002 moves a function around to be declared in the only place it is
needed, and prevents a "sh: /oasdfpt/pgsql-8.4/bin/pg_config: No such
file or directory" error message when you give it a bogus bindir.

0003 is what I really wanted to solve, which was my failure with
pg_upgrade. The call to pg_ctl didn't succeed because the binaries
didn't match the data directory, thus resulting in this:

$ pg_upgrade --check -d /tmp/olddata -D /tmp/newdata -b /usr/bin/ -B /usr/bin/
Performing Consistency Checks
-----------------------------
Checking old data directory (/tmp/olddata)                  ok
Checking old bin directory (/usr/bin)                       ok
Checking new data directory (/tmp/newdata)                  ok
Checking new bin directory (/usr/bin)                       ok
pg_resetxlog: pg_control exists but is broken or unknown version; ignoring it
Trying to start old server                                  .................ok

 Unable to start old postmaster with the command: "/usr/bin/pg_ctl" -l
"/dev/null" -D "/tmp/olddata" -o "-p 5432 -c autovacuum=off -c
autovacuum_freeze_max_age=2000000000" start >> "/dev/null" 2>&1
Perhaps pg_hba.conf was not set to "trust".

The error had nothing to do with "trust" at all; it was simply that I
tried to use 9.0 binaries with an 8.4 data directory. My patch checks
for this and ensures that the -D bindir is the correct version, just
as the -B datadir has to be the correct version.

I'm not on the mailing list nor do I have a lot of free time to keep
up with normal development, but if there are quick things I can do to
get these patches in let me know.

-Dan
From 840bdd22b62c8d45796abf7eb9e7b3da0329dce8 Mon Sep 17 00:00:00 2001
From: Dan McGee <d...@archlinux.org>
Date: Tue, 21 Jun 2011 18:48:01 -0500
Subject: [PATCH 1/3] pg_upgrade: fix typo in consistency check message

---
 contrib/pg_upgrade/check.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/contrib/pg_upgrade/check.c b/contrib/pg_upgrade/check.c
index 376d25a..2b481da 100644
--- a/contrib/pg_upgrade/check.c
+++ b/contrib/pg_upgrade/check.c
@@ -30,7 +30,7 @@ output_check_banner(bool *live_check)
 		if (old_cluster.port == new_cluster.port)
 			pg_log(PG_FATAL, "When checking a live server, "
 				   "the old and new port numbers must be different.\n");
-		pg_log(PG_REPORT, "PerForming Consistency Checks on Old Live Server\n");
+		pg_log(PG_REPORT, "Performing Consistency Checks on Old Live Server\n");
 		pg_log(PG_REPORT, "------------------------------------------------\n");
 	}
 	else
-- 
1.7.5.4

From f3e393318ba36ef543f77fb8983902d466ebe8c8 Mon Sep 17 00:00:00 2001
From: Dan McGee <d...@archlinux.org>
Date: Tue, 21 Jun 2011 18:49:47 -0500
Subject: [PATCH 2/3] pg_upgrade: remove libpath from cluster info struct

We only use this item in one check and then no longer need it.
Additionally, get_pkglibdir() is currently run before we do our checks
on the bin/ directory, so an incorrectly specified bin/ directory will
evoke failures at the "wrong" point.

Move the entire function into the file that uses it so it can remain
static.
---
 contrib/pg_upgrade/check.c      |   35 +++++++++++++++++++++++++++++-
 contrib/pg_upgrade/option.c     |   45 ---------------------------------------
 contrib/pg_upgrade/pg_upgrade.h |    1 -
 3 files changed, 34 insertions(+), 47 deletions(-)

diff --git a/contrib/pg_upgrade/check.c b/contrib/pg_upgrade/check.c
index 2b481da..5ff2d81 100644
--- a/contrib/pg_upgrade/check.c
+++ b/contrib/pg_upgrade/check.c
@@ -19,6 +19,7 @@ static void check_is_super_user(ClusterInfo *cluster);
 static void check_for_prepared_transactions(ClusterInfo *cluster);
 static void check_for_isn_and_int8_passing_mismatch(ClusterInfo *cluster);
 static void check_for_reg_data_type_usage(ClusterInfo *cluster);
+static char *get_pkglibdir(const char *bindir);
 
 
 void
@@ -246,14 +247,17 @@ void
 check_cluster_compatibility(bool live_check)
 {
 	char		libfile[MAXPGPATH];
+	char	   *libpath;
 	FILE	   *lib_test;
 
 	/*
 	 * Test pg_upgrade_support.so is in the proper place.	 We cannot copy it
 	 * ourselves because install directories are typically root-owned.
 	 */
-	snprintf(libfile, sizeof(libfile), "%s/pg_upgrade_support%s", new_cluster.libpath,
+	libpath = get_pkglibdir(new_cluster.bindir);
+	snprintf(libfile, sizeof(libfile), "%s/pg_upgrade_support%s", libpath,
 			 DLSUFFIX);
+	pg_free(libpath);
 
 	if ((lib_test = fopen(libfile, "r")) == NULL)
 		pg_log(PG_FATAL,
@@ -730,3 +734,32 @@ check_for_reg_data_type_usage(ClusterInfo *cluster)
 	else
 		check_ok();
 }
+
+
+static char *
+get_pkglibdir(const char *bindir)
+{
+	char		cmd[MAXPGPATH];
+	char		bufin[MAX_STRING];
+	FILE	   *output;
+	int			i;
+
+	snprintf(cmd, sizeof(cmd), "\"%s/pg_config\" --pkglibdir", bindir);
+
+	if ((output = popen(cmd, "r")) == NULL)
+		pg_log(PG_FATAL, "Could not get pkglibdir data: %s\n",
+			   getErrorText(errno));
+
+	fgets(bufin, sizeof(bufin), output);
+
+	if (output)
+		pclose(output);
+
+	/* Remove trailing newline */
+	i = strlen(bufin) - 1;
+
+	if (bufin[i] == '\n')
+		bufin[i] = '\0';
+
+	return pg_strdup(bufin);
+}
diff --git a/contrib/pg_upgrade/option.c b/contrib/pg_upgrade/option.c
index 8153e30..18ce9d7 100644
--- a/contrib/pg_upgrade/option.c
+++ b/contrib/pg_upgrade/option.c
@@ -19,8 +19,6 @@
 static void usage(void);
 static void validateDirectoryOption(char **dirpath,
 				   char *envVarName, char *cmdLineOption, char *description);
-static void get_pkglibdirs(void);
-static char *get_pkglibdir(const char *bindir);
 
 
 UserOpts	user_opts;
@@ -213,8 +211,6 @@ parseCommandLine(int argc, char *argv[])
 							"old cluster data resides");
 	validateDirectoryOption(&new_cluster.pgdata, "NEWDATADIR", "-D",
 							"new cluster data resides");
-
-	get_pkglibdirs();
 }
 
 
@@ -314,44 +310,3 @@ validateDirectoryOption(char **dirpath, char *envVarName,
 #endif
 		(*dirpath)[strlen(*dirpath) - 1] = 0;
 }
-
-
-static void
-get_pkglibdirs(void)
-{
-	/*
-	 * we do not need to know the libpath in the old cluster, and might not
-	 * have a working pg_config to ask for it anyway.
-	 */
-	old_cluster.libpath = NULL;
-	new_cluster.libpath = get_pkglibdir(new_cluster.bindir);
-}
-
-
-static char *
-get_pkglibdir(const char *bindir)
-{
-	char		cmd[MAXPGPATH];
-	char		bufin[MAX_STRING];
-	FILE	   *output;
-	int			i;
-
-	snprintf(cmd, sizeof(cmd), "\"%s/pg_config\" --pkglibdir", bindir);
-
-	if ((output = popen(cmd, "r")) == NULL)
-		pg_log(PG_FATAL, "Could not get pkglibdir data: %s\n",
-			   getErrorText(errno));
-
-	fgets(bufin, sizeof(bufin), output);
-
-	if (output)
-		pclose(output);
-
-	/* Remove trailing newline */
-	i = strlen(bufin) - 1;
-
-	if (bufin[i] == '\n')
-		bufin[i] = '\0';
-
-	return pg_strdup(bufin);
-}
diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h
index a3a0856..c27b58a 100644
--- a/contrib/pg_upgrade/pg_upgrade.h
+++ b/contrib/pg_upgrade/pg_upgrade.h
@@ -185,7 +185,6 @@ typedef struct
 	uint32		major_version;	/* PG_VERSION of cluster */
 	char		major_version_str[64];	/* string PG_VERSION of cluster */
 	Oid			pg_database_oid;	/* OID of pg_database relation */
-	char	   *libpath;		/* pathname for cluster's pkglibdir */
 	char	   *tablespace_suffix;		/* directory specification */
 } ClusterInfo;
 
-- 
1.7.5.4

From a2f04776839228bfde8d1a7ed16173e5bccd129a Mon Sep 17 00:00:00 2001
From: Dan McGee <d...@archlinux.org>
Date: Tue, 21 Jun 2011 18:53:42 -0500
Subject: [PATCH 3/3] Add a function to check the provided binary versions

This is similar to our cluster version checks, except that we are
ensuring the binaries from our respective paths both match the versions
of the data directories they are to act on, as well as ensuring the new
binaries are compatible with this verison of pg_upgrade.
---
 contrib/pg_upgrade/check.c      |   56 +++++++++++++++++++++++++++++++++++++++
 contrib/pg_upgrade/pg_upgrade.h |    2 +
 2 files changed, 58 insertions(+), 0 deletions(-)

diff --git a/contrib/pg_upgrade/check.c b/contrib/pg_upgrade/check.c
index 5ff2d81..9163ff6 100644
--- a/contrib/pg_upgrade/check.c
+++ b/contrib/pg_upgrade/check.c
@@ -20,6 +20,7 @@ static void check_for_prepared_transactions(ClusterInfo *cluster);
 static void check_for_isn_and_int8_passing_mismatch(ClusterInfo *cluster);
 static void check_for_reg_data_type_usage(ClusterInfo *cluster);
 static char *get_pkglibdir(const char *bindir);
+static void get_pg_ctl_version(ClusterInfo *cluster);
 
 
 void
@@ -217,9 +218,14 @@ output_completion_banner(char *deletion_script_file_name)
 void
 check_cluster_versions(void)
 {
+	prep_status("Checking cluster versions");
+
 	/* get old and new cluster versions */
 	old_cluster.major_version = get_major_server_version(&old_cluster);
 	new_cluster.major_version = get_major_server_version(&new_cluster);
+	/* get old and new binary versions */
+	get_pg_ctl_version(&old_cluster);
+	get_pg_ctl_version(&new_cluster);
 
 	/*
 	 * We allow upgrades from/to the same major version for alpha/beta
@@ -240,6 +246,19 @@ check_cluster_versions(void)
 	 */
 	if (old_cluster.major_version > new_cluster.major_version)
 		pg_log(PG_FATAL, "This utility cannot be used to downgrade to older major PostgreSQL versions.\n");
+
+	/* Ensure binaries match the designated data directories */
+	if (GET_MAJOR_VERSION(old_cluster.major_version) != GET_MAJOR_VERSION(old_cluster.bin_version))
+		pg_log(PG_FATAL, "Old cluster binaries and data are not compatible.\n");
+	if (GET_MAJOR_VERSION(new_cluster.major_version) != GET_MAJOR_VERSION(new_cluster.bin_version))
+		pg_log(PG_FATAL, "New cluster binaries and data are not compatible.\n");
+
+	/* And ensure new binaries are the expected PG version */
+	if (GET_MAJOR_VERSION(new_cluster.bin_version) != GET_MAJOR_VERSION(PG_VERSION_NUM))
+		pg_log(PG_FATAL, "This utility can only upgrade to PostgreSQL version %s.\n",
+			   PG_MAJORVERSION);
+
+	check_ok();
 }
 
 
@@ -763,3 +782,40 @@ get_pkglibdir(const char *bindir)
 
 	return pg_strdup(bufin);
 }
+
+
+static void
+get_pg_ctl_version(ClusterInfo *cluster)
+{
+	char		cmd[MAXPGPATH];
+	char		bufin[MAX_STRING];
+	FILE	   *output;
+	int			i;
+	int			integer_version = 0;
+	int			fractional_version = 0;
+	int			minor_version = 0;
+
+	snprintf(cmd, sizeof(cmd), "\"%s/pg_ctl\" --version", cluster->bindir);
+
+	if ((output = popen(cmd, "r")) == NULL)
+		pg_log(PG_FATAL, "Could not get pg_ctl version data: %s\n",
+			   getErrorText(errno));
+
+	fgets(bufin, sizeof(bufin), output);
+
+	if (output)
+		pclose(output);
+
+	/* Remove trailing newline */
+	i = strlen(bufin) - 1;
+
+	if (bufin[i] == '\n')
+		bufin[i] = '\0';
+
+	if (sscanf(bufin, "pg_ctl (PostgreSQL) %d.%d.%d", &integer_version,
+			   &fractional_version, &minor_version) != 3)
+		pg_log(PG_FATAL, "could not get version from %s\n", cmd);
+
+	cluster->bin_version_str = pg_strdup(bufin);
+	cluster->bin_version = ((integer_version * 100 + fractional_version) * 100) + minor_version;
+}
diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h
index c27b58a..d9ea3f9 100644
--- a/contrib/pg_upgrade/pg_upgrade.h
+++ b/contrib/pg_upgrade/pg_upgrade.h
@@ -184,6 +184,8 @@ typedef struct
 	unsigned short port;		/* port number where postmaster is waiting */
 	uint32		major_version;	/* PG_VERSION of cluster */
 	char		major_version_str[64];	/* string PG_VERSION of cluster */
+	uint32		bin_version;	/* version returned from pg_ctl */
+	char	   *bin_version_str;	/* string version returned from pg_ctl */
 	Oid			pg_database_oid;	/* OID of pg_database relation */
 	char	   *tablespace_suffix;		/* directory specification */
 } ClusterInfo;
-- 
1.7.5.4

-- 
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