Date: Fri, 9 Oct 2020 14:23:10 -0500
From: Justin Pryzby <pryzby@telsasoft.com>
To: Bruce Momjian <bruce@momjian.us>
Cc: pgsql-hackers@postgresql.org
Subject: Re: pg_upgrade: fail early if a tablespace dir already exists for
 new cluster version
Message-ID: <20201009192310.GS17626@telsasoft.com>
References: <20200925005531.GJ23631@telsasoft.com>
 <20201009185325.GA24870@momjian.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20201009185325.GA24870@momjian.us>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk

On Fri, Oct 09, 2020 at 02:53:25PM -0400, Bruce Momjian wrote:
> 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.

Thanks for looking.  It hits me a bunch of times every year.

> +  * 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.

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.

Cheers,
-- 
Justin