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

Hi,

> v5 resolved this as a dry run rather than a hard block. --check
> --initdb reports the initdb command it would run and validates the
> preconditions (correct new-binary version, empty target directory, old
> cluster reachable) without creating the new cluster. v6 keeps that,
> and the docs now describe it.

Thanks, appreciated.

> > I'm not clear what pointing this out for --initdb only accomplishes.
> > A wrong -B is going to cause problems regardless of the mode.
> I agree, and I believe this meets Demir's point about orphan directories.
> v5 checks the new-binary version up front, so a wrong -B fails before anything is
> written. An atexit handler removes the created directory if a later
> check fails, up until the point where real data has been copied. So
> there is no need for a -B note specific to --initdb in the docs.
> v6 carries both.

Got it, great.

>
> > Eliminating what exactly? And why would a vacuum or checkpoint cause
> > problems?
> The old server is started here with the same start_postmaster() that
> pg_upgrade uses elsewhere -- including -b, which disables autovacuum --
> only to read template0's encoding and locale, then stopped before the
> new cluster is created. So autovacuum will not run, and a checkpoint
> would be harmless. It is a brief, read-only start using pg_upgrade's
> existing mechanism, so it adds no risk beyond the old-server starts
> pg_upgrade already performs. The docs now mention this brief start.

Thanks for handling this.

> Beyond the review points, v6 also tightens two things I noticed while
> reworking this:

> - The empty-directory guard now rejects any non-empty target directory,
> not just one already containing a PG_VERSION file. This closes a gap
> in the orphan-cleanup path. The cleanup handler removes the whole new
> data directory on failure, so it must not run against a directory that
> already held the user's files.

> - Because --initdb reads template0 by briefly starting the old server, it
> now checks in advance that the old server is shut down and fails with the
> usual "shut down that postmaster" message, rather than failing later
> when pg_resetwal refuses to run against a live server.

Good catches, I have no objections to v6.

Regards,
Demir.