Re: [PATCH] pg_upgrade: add --initdb option to create the new cluster automatically

Hi,

>> When I run the pg_upgrade command with the following options I'd
>> expect it to not create a directory and initdb.
>>
>> ```
>> pg_upgrade --check --initdb -d /old -D /new
>> ```
>
>
> I see your point. Maybe we don't allow --check with --initdb at all?

Yes, that would make sense. My point is that, we should check if
--initdb is possible or not when the operator/tool executes it via the
--check option.

>> also mention that -B flag points outs to target version correctly
>> since there is a risk of doing an initdb with the wrong version. If -B
>> points to an older version, pg_upgrade may create the wrong cluster.
>
>
> I'm not clear what pointing this out for --initdb only accomplishes. A wrong -B is going to cause problems regardless of the mode.

My point is that --new-bindir at the wrong major version's initdb, run
pg_upgrade --initdb. The wrong-version initdb succeeds and populates
new_cluster.pgdata fully. Then check_cluster_versions() fires
pg_fatal(), and the process exits with a fully-initialized,
wrong-version, orphaned data directory on disk — and no cleanup code
runs anywhere in the patch.

We need to enhance the patch so once it fails it should also clean up
the orphan directories. Because current failure in pg_upgrade doesn't
lead to orphan directories. Otherwise, if pg_upgrade fails and
operator wants to execute it again it won't be able to do it since the
directory already is not empty and initdb won't work. We need to
decide if pg_upgrade will wipe out orphan initdb directories if
pg_upgrade fails or leave it to the operator.

Summary of v5 changes

v4 had a shortcut that skipped re-reading the old cluster's control
data if it had already been read once. Turns out this was skipping a
real read later in the process that the upgrade actually depends on.
Removed the shortcut since it wasn't needed anyway, reading control
data is cheap.

Fixed the orphan directory problem. This was the main issue from the
review. If --initdb creates the new cluster but something fails
afterward (wrong binary version, etc.), the directory used to get left
behind, and a retry would fail because pg_upgrade refuses to overwrite
it. Now we automatically clean it up if something goes wrong, but only
up until the point where real data has actually been copied over.
After that, we leave it alone, because deleting real data is worse
than leaving an orphan folder.

--check --initdb now works as a dry run, instead of being blocked. It
tells you whether --initdb would succeed, checking the right binary
version, an empty target folder, and that the old cluster is
reachable, without actually creating anything. Added a version check
up front. As part of #3, we now check the new binary's version before
doing anything else. This is actually what stops the orphan directory
problem from happening in the first place, not just for the dry run.
If the version is wrong, we fail immediately, before touching the
disk.

Updated the docs to explain all of the above. Ran the full build and
test suite. New test file covers all the scenarios above, 26 out of 26
passing, no regressions elsewhere.

Patch attached. I've renamed the patch file to
v5-0001-pg_upgrade-initdb.patch to match our naming convention. No
content change, just the filename. Happy to walk through any of this
in more detail.

Regards,
Demir.