Date: Fri, 9 Oct 2020 19:42:51 -0400
From: Bruce Momjian <bruce@momjian.us>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: pg_upgrade: fail early if a tablespace dir already exists for
 new cluster version
Message-ID: <20201009234251.GA8419@momjian.us>
References: <20200925005531.GJ23631@telsasoft.com>
 <20201009185325.GA24870@momjian.us>
 <20201009192310.GS17626@telsasoft.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="4Ckj6UjgE2iN1+kY"
Content-Disposition: inline
In-Reply-To: <20201009192310.GS17626@telsasoft.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk


--4Ckj6UjgE2iN1+kY
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Fri, Oct  9, 2020 at 02:23:10PM -0500, Justin Pryzby wrote:
> In my local branch, I had revised this comment to say:
> 
> + * Note, v8.4 has no tablespace_suffix, which is fine so long as the version we
> + * being upgraded *to* has a suffix, since it's not allowed to pg_upgrade from
> + * a version to the same version if tablespaces are in use.

OK, updated patch attached.  Also, from your original patch, I didn't
need to call canonicalize_path() since we are not comparing paths, and I
didn't need to include common/relpath.h.  I also renamed a variable.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EnterpriseDB                             https://enterprisedb.com

  The usefulness of a cup is in its emptiness, Bruce Lee


--4Ckj6UjgE2iN1+kY
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="tablespace.diff"

diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 168058a873..df4274d89a 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -27,6 +27,7 @@ static void check_for_tables_with_oids(ClusterInfo *cluster);
 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);
 
 
@@ -187,6 +188,8 @@ check_new_cluster(void)
 	check_is_install_user(&new_cluster);
 
 	check_for_prepared_transactions(&new_cluster);
+
+	check_for_new_tablespace_dir(&new_cluster);
 }
 
 
@@ -527,6 +530,43 @@ create_script_for_cluster_analyze(char **analyze_script_file_name)
 }
 
 
+/*
+ * A previous run of pg_upgrade might have failed and the new cluster
+ * directory recreated, but they might have forgotten to remove
+ * the new cluser's tablespace directories.  Therefore, check that
+ * new cluster tablespace directories do not already exist.  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 so long as the
+ * version we being upgraded *to* has a suffix, since it's not allowed
+ * to pg_upgrade from a version to the same version if tablespaces are
+ * in use.
+ */
+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()
  *

--4Ckj6UjgE2iN1+kY--