Dan McGee wrote:
> 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.
Thanks for the detailed report and patches. Let me address each one
individually.
> 0001 is just a simple typo fix, but didn't want to mix it in with the rest.
I applied this message capitalization fix to 9.0, 9.1, and master (9.2).
> 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.
You are right that I was calling pg_ctl before I had validated the bin
directory, and you were right that the function wasn't in an ideal
C file.
What I did was to move the function, and bundle all the
pg_upgrade_support test into that single function, so the function now
either errors out or returns void.
Patch attached and applied to 9.1 and master. Good catch.
> 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 had not thought about testing if the bin and data directory were the
same major version, but you are right that it generates an odd error if
they are not.
I changed your sscanf format string because the version can be just
"9.2dev" and there is no need for the minor version. I saw no reason
to test if the binary version matches the pg_upgrade version because we
already test the cluster version, and we test the cluster version is the
same as the binary version.
Patch attached and applied to 9.1 and master.
> 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.
All done!
--
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/check.c b/contrib/pg_upgrade/check.c
new file mode 100644
index 2b481da..377dea2
*** a/contrib/pg_upgrade/check.c
--- b/contrib/pg_upgrade/check.c
*************** static void check_is_super_user(ClusterI
*** 19,24 ****
--- 19,25 ----
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 void check_for_support_lib(ClusterInfo *cluster);
void
*************** check_cluster_versions(void)
*** 245,265 ****
void
check_cluster_compatibility(bool live_check)
{
! char libfile[MAXPGPATH];
! 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,
! DLSUFFIX);
!
! if ((lib_test = fopen(libfile, "r")) == NULL)
! pg_log(PG_FATAL,
! "pg_upgrade_support%s must be created and installed in %s\n", DLSUFFIX, libfile);
! else
! fclose(lib_test);
/* get/check pg_control data of servers */
get_control_data(&old_cluster, live_check);
--- 246,252 ----
void
check_cluster_compatibility(bool live_check)
{
! check_for_support_lib(&new_cluster);
/* get/check pg_control data of servers */
get_control_data(&old_cluster, live_check);
*************** check_for_reg_data_type_usage(ClusterInf
*** 730,732 ****
--- 717,758 ----
else
check_ok();
}
+
+
+ /*
+ * Test pg_upgrade_support.so is in the proper place. We cannot copy it
+ * ourselves because install directories are typically root-owned.
+ */
+ static void
+ check_for_support_lib(ClusterInfo *cluster)
+ {
+ char cmd[MAXPGPATH];
+ char libdir[MAX_STRING];
+ char libfile[MAXPGPATH];
+ FILE *lib_test;
+ FILE *output;
+
+ snprintf(cmd, sizeof(cmd), "\"%s/pg_config\" --pkglibdir", cluster->bindir);
+
+ if ((output = popen(cmd, "r")) == NULL)
+ pg_log(PG_FATAL, "Could not get pkglibdir data: %s\n",
+ getErrorText(errno));
+
+ fgets(libdir, sizeof(libdir), output);
+
+ pclose(output);
+
+ /* Remove trailing newline */
+ if (strchr(libdir, '\n') != NULL)
+ *strchr(libdir, '\n') = '\0';
+
+ snprintf(libfile, sizeof(libfile), "%s/pg_upgrade_support%s", libdir,
+ DLSUFFIX);
+
+ if ((lib_test = fopen(libfile, "r")) == NULL)
+ pg_log(PG_FATAL,
+ "The pg_upgrade_support module must be created and installed in the %s cluster.\n",
+ CLUSTER_NAME(cluster));
+
+ fclose(lib_test);
+ }
diff --git a/contrib/pg_upgrade/option.c b/contrib/pg_upgrade/option.c
new file mode 100644
index 8153e30..18ce9d7
*** a/contrib/pg_upgrade/option.c
--- b/contrib/pg_upgrade/option.c
***************
*** 19,26 ****
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;
--- 19,24 ----
*************** parseCommandLine(int argc, char *argv[])
*** 213,220 ****
"old cluster data resides");
validateDirectoryOption(&new_cluster.pgdata, "NEWDATADIR", "-D",
"new cluster data resides");
-
- get_pkglibdirs();
}
--- 211,216 ----
*************** validateDirectoryOption(char **dirpath,
*** 314,357 ****
#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);
- }
--- 310,312 ----
diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h
new file mode 100644
index a3a0856..c27b58a
*** a/contrib/pg_upgrade/pg_upgrade.h
--- b/contrib/pg_upgrade/pg_upgrade.h
*************** typedef struct
*** 185,191 ****
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;
--- 185,190 ----
diff --git a/contrib/pg_upgrade/check.c b/contrib/pg_upgrade/check.c
new file mode 100644
index 1c3f589..1ee2aca
*** a/contrib/pg_upgrade/check.c
--- b/contrib/pg_upgrade/check.c
*************** static void check_for_prepared_transacti
*** 20,25 ****
--- 20,26 ----
static void check_for_isn_and_int8_passing_mismatch(ClusterInfo *cluster);
static void check_for_reg_data_type_usage(ClusterInfo *cluster);
static void check_for_support_lib(ClusterInfo *cluster);
+ static void get_bin_version(ClusterInfo *cluster);
void
*************** output_completion_banner(char *deletion_
*** 216,221 ****
--- 217,224 ----
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);
*************** check_cluster_versions(void)
*** 235,244 ****
/*
* We can't allow downgrading because we use the target pg_dumpall, and
! * pg_dumpall cannot operate on new datbase versions, only older versions.
*/
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");
}
--- 238,263 ----
/*
* We can't allow downgrading because we use the target pg_dumpall, and
! * pg_dumpall cannot operate on new database versions, only older versions.
*/
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");
+
+ /* get old and new binary versions */
+ get_bin_version(&old_cluster);
+ get_bin_version(&new_cluster);
+
+ /* 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 data and binary directories are from different major versions.\n");
+ if (GET_MAJOR_VERSION(new_cluster.major_version) !=
+ GET_MAJOR_VERSION(new_cluster.bin_version))
+ pg_log(PG_FATAL,
+ "New cluster data and binary directories are from different major versions.\n");
+
+ check_ok();
}
*************** check_for_support_lib(ClusterInfo *clust
*** 754,756 ****
--- 773,804 ----
fclose(lib_test);
}
+
+
+ static void
+ get_bin_version(ClusterInfo *cluster)
+ {
+ char cmd[MAXPGPATH], cmd_output[MAX_STRING];
+ FILE *output;
+ int pre_dot, post_dot;
+
+ 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(cmd_output, sizeof(cmd_output), output);
+
+ pclose(output);
+
+ /* Remove trailing newline */
+ if (strchr(cmd_output, '\n') != NULL)
+ *strchr(cmd_output, '\n') = '\0';
+
+ if (sscanf(cmd_output, "%*s %*s %d.%d", &pre_dot, &post_dot) != 2)
+ pg_log(PG_FATAL, "could not get version from %s\n", cmd);
+
+ cluster->bin_version = (pre_dot * 100 + post_dot) * 100;
+ }
+
diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h
new file mode 100644
index c27b58a..613ddbd
*** a/contrib/pg_upgrade/pg_upgrade.h
--- b/contrib/pg_upgrade/pg_upgrade.h
*************** typedef struct
*** 184,189 ****
--- 184,190 ----
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 */
Oid pg_database_oid; /* OID of pg_database relation */
char *tablespace_suffix; /* directory specification */
} ClusterInfo;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
