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
Date: Fri, 9 Oct 2020 14:53:25 -0400
Message-ID: <20201009185325.GA24870@momjian.us> (raw)
In-Reply-To: <20200925005531.GJ23631@telsasoft.com>
References: <20200925005531.GJ23631@telsasoft.com>
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 <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com
The usefulness of a cup is in its emptiness, Bruce Lee
Attachments:tablespace.diff (text/x-diff, 2.0K) downloaddiff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.cnew file mode 100644index 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.
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: bruce@momjian.us, pryzby@telsasoft.com
Subject: Re: pg_upgrade: fail early if a tablespace dir already exists for new cluster version
In-Reply-To: <20201009185325.GA24870@momjian.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox