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

Hi Daniel,

Thanks for the review. v3 attached.

*On external wrapper vs. in-tree:* the point of --initdb is deriving the
new cluster's settings (WAL segment size, checksums, encoding, locale) from
the old cluster, exactly what people get wrong by hand and only discover
when check_control_data() fails. pg_upgrade already reads the old control
data, so doing it here reuses that rather than making a wrapper rediscover
it. I'm open to being convinced otherwise.

The rest:
*- --check --initdb:* you're right it was broken (ran before live-check was
determined). Since --check is read-only and --initdb creates the cluster,
the combination doesn't make sense — v3 rejects it at option parsing, which
removes the live-check problem entirely.
*- get_control_data() twice:* the early --initdb read populates the old
cluster's control data, so the later call in check_cluster_compatibility()
returns early when ctrl_ver is already set. New cluster is still read fresh.
*- adjust_data_dir(): *moved adjust_data_dir(&new_cluster) before the
initdb call so a config-only -D is resolved first.
*- check_bindir(): *meant get_bin_version(). Fixed.
*- Docs:* noted that only initdb-accepted settings are configured,
postmaster-only options need manual creation, and the --check restriction.

TAP test covers the --check rejection. pgindent-clean, tests pass.
https://github.com/LeeBohyun/postgres/tree/pg_upgrade_initdb
Best regards,
Bohyun

On Tue, Jul 14, 2026 at 12:06 PM Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:

> I am not convinced that all this complexity and overhead isn't better
> suited
> for external wrappers to pg_upgrade like Debian's pg_upgradecluster etc.
>
> + if (user_opts.initdb_new_cluster)
> + create_new_cluster_via_initdb();
> +
> adjust_data_dir(&new_cluster);
>
> If pg_upgrade controls how initdb was invoked for the new cluster,
> shouldn't it
> work such that adjust_data_dir isn't required?
>
>
> + /*
> + * get_control_data() selects pg_resetwal vs. pg_resetxlog via
> + * bin_version, which check_bindir() normally fills in later.
> Seed it now
> + * so the right binary name is used in this early call.
> + */
> + if (old_cluster.bin_version == 0)
> + old_cluster.bin_version = old_cluster.major_version;
> +
> + get_control_data(&old_cluster);
>
> This can't be done unconditionally, the old cluster can still be running at
> this point. For example if someone wants to do a live-check:
>
> $ ./bin/pg_upgrade -b ./bin/ -B ./bin/ -d ./data_old/ -D ./data_new/
> --check --initdb
>
> The source cluster was not shut down cleanly, state reported as: "in
> production"
> Failure, exiting
>
> get_control_data is big, expensive, and really designed to be run once. I
> don't think it's Ok to run it an extra time here without at least being
> able to
> tell the later invocation that it has already been executed. Also,
> check_bindir() as referred to in the comment does not exist.
>
>
> + prep_status("Inspecting old cluster locale for new cluster creation");
> + start_postmaster(&old_cluster, true);
> + get_template0_info(&old_cluster);
> + stop_postmaster(false);
> + check_ok();
>
> Again, cannot be done unconditionally.
>
>
> + * Users needing options that only the postmaster accepts can
> create the
> + * new cluster manually and omit --initdb.
>
> This should probably be expanded upon in the documentation.
>
> --
> Daniel Gustafsson
>
>