On Thu, Sep 24, 2020 at 07:55:31PM -0500, Justin Pryzby wrote:
> This should be caught during --check, or earlier in the upgrade, rather than
> only after dumping the schema.
>
> Typically, the tablespace dir would be left behind by a previous failed
> upgrade
> attempt, causing a cascade of failured upgrades. I guess I may not be the
> only
> one with a 3 year old wrapper which has a hack to check for this.
This is an interesting failure case I never considered --- running
pg_upgrade, having it fail, deleting and recreating the _new_ cluster
directory, but not removing the new cluster's tablepace directories. I
was able to recreate the failure, and verify that your patch properly
throws an error during 'check' for this case.
Modified patch attached. I plan to apply this to all supported Postgres
versions.
--
Bruce Momjian <[email protected]> https://momjian.us
EnterpriseDB https://enterprisedb.com
The usefulness of a cup is in its emptiness, Bruce Lee
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
new file mode 100644
index 168058a..0c845b7
*** a/src/bin/pg_upgrade/check.c
--- b/src/bin/pg_upgrade/check.c
*************** static void check_for_tables_with_oids(C
*** 27,32 ****
--- 27,33 ----
static void check_for_reg_data_type_usage(ClusterInfo *cluster);
static void check_for_jsonb_9_4_usage(ClusterInfo *cluster);
static void check_for_pg_role_prefix(ClusterInfo *cluster);
+ static void check_for_new_tablespace_dir(ClusterInfo *new_cluster);
static char *get_canonical_locale_name(int category, const char *locale);
*************** check_new_cluster(void)
*** 187,192 ****
--- 188,195 ----
check_is_install_user(&new_cluster);
check_for_prepared_transactions(&new_cluster);
+
+ check_for_new_tablespace_dir(&new_cluster);
}
*************** create_script_for_cluster_analyze(char *
*** 528,533 ****
--- 531,568 ----
/*
+ * Check that tablespace directories do not already exist for new cluster.
+ * If they do, it would cause an error while restoring global objects.
+ * This allows the failure to be detected at check time, rather than
+ * during schema restore.
+ *
+ * Note, v8.4 has no tablespace_suffix, which is fine as long as the new
+ * cluster version has a suffix.
+ */
+ static void
+ check_for_new_tablespace_dir(ClusterInfo *new_cluster)
+ {
+ char new_tablespace_dir[MAXPGPATH];
+
+ prep_status("Checking for new cluster tablespace directories");
+
+ for (int tblnum = 0; tblnum < os_info.num_old_tablespaces; tblnum++)
+ {
+ struct stat statbuf;
+
+ snprintf(new_tablespace_dir, MAXPGPATH, "%s%s",
+ os_info.old_tablespaces[tblnum],
+ new_cluster->tablespace_suffix);
+
+ if (stat(new_tablespace_dir, &statbuf) == 0 || errno != ENOENT)
+ pg_fatal("new cluster tablespace directory already exists: \"%s\"\n",
+ new_tablespace_dir);
+ }
+
+ check_ok();
+ }
+
+ /*
* create_script_for_old_cluster_deletion()
*
* This is particularly useful for tablespace deletion.
