Re: pg_upgrade --clone error checking

On Fri, May 3, 2019 at 3:53 AM Peter Eisentraut <
peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:

> On 2019-05-02 20:03, Jeff Janes wrote:
> > It looks like it was designed for early checking, it just wasn't placed
> > early enough. So changing it is pretty easy, as check_file_clone does
> > not need to be invented, and there is no additional code duplication
> > over what was already there.
> >
> > This patch moves the checking to near the beginning.
>
> I think the reason it was ordered that way is that it wants to do all
> the checks of the old cluster before doing any checks touching the new
> cluster.
>

But is there a reason to want to do that? I understand we don't want to
keep starting and stopping the clusters needlessly, so we should do
everything we can in one before moving to the other. But for checks that
don't need a running cluster, why would it matter? The existence and
contents of PG_VERSION of the new cluster directory is already checked at
the very beginning (and even tries to start it up and shuts it down again
if a pid file also exists), so there is precedence for touching the new
cluster directory at the filesystem level early (albeit in a readonly
manner) and if a pid file exists then doing even more than that. I didn't
move check_file_clone to before the liveness check is done, out of a
abundance of caution. But creating a transient file with a name of no
significance ("PG_VERSION.clonetest") in a cluster that is not even running
seems like a very low risk thing to do. The pay off is that we get an
inevitable error message much sooner.

Cheers,

Jeff