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.