| From: | Bohyun Lee <bohyun(dot)lee(at)databricks(dot)com> |
|---|---|
| To: | harrisontbooth(at)gmail(dot)com |
| Cc: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Greg Sabino Mullane <htamfids(at)gmail(dot)com>, Daniel Gustafsson <daniel(at)yesql(dot)se>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: [PATCH] pg_upgrade: add --initdb option to create the new cluster automatically |
| Date: | 2026-10-14 08:32:05 |
| Message-ID: | CAMPh8MpLR8sax=fG6ShzkE_LF9JFL7MR8eyX3WrNguYp57Xpvw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> Let me know if this cleanup approach works for you.
Thanks, Harrison, for testing on both Windows architectures.
The cleanup approach looks good to me. I’ve integrated your supplement,
and the tests also pass on Linux and macOS.
Heikki, thanks again for the review and the comments.
Also incorporated the order change you proposed in this patch
<https://www.postgresql.org/message-id/6894f355-10e4-4756-89ad-ae5987f416a6@iki.fi>
.
> I don't think we should do this. If the user wants the new cluster to
> have group access, use --initdb-options='--allow-group-access'. There's
> no magic for that before this patch either, the user needs to use
> “initdb --allow_group_access" if they want the new cluster to have group
> access.
Right, removed the automatic inheritance of the old cluster's group-access
setting.
The new cluster now uses initdb's default permissions, and users can enable
group
access explicitly with --initdb-options='--allow-group-access'.
This way, the behavior of manual initdb --allow-group-access + pg_upgrade
execution
remains the same with pg_upgrade --initdb
--initdb-options='--allow-group-access'.
Updated the documentation and the existing test expectations as well.
> Why not write the logs under the new data directory, just like without
> --initdb?
Agreed, the normal logs should end up in the new datadir, as they do
without --initdb.
The patch captures early server and initdb output under
./pg_upgrade_output.d/<timestamp>/ in the current working directory,
because initdb requires an empty target directory.
But once initdb succeeds, it removes the temporary logs unless --retain is
specified.
Subsequent output is written under
<new datadir>/pg_upgrade_output.d/<timestamp>/ using the existing cleanup
and --retain handling.
If initdb fails, pg_upgrade reports initialization failure and points to
the retained log.
> But then you need write access to the current directory instead.
pg_upgrade already requires a writable current directory, even without
--initdb.
So, logging to a temporary location therefore add no new requirement for
write
access to that directory.
>> The new server must use a different port.
> Why?
The previous statement was an overclaim.
To clarify, not every --check --initdb invocation requires different ports.
Different ports are required only when checking a running old server.
This is an existing pg_upgrade requirement, unchanged by --initdb.
Offline checks can use the same port.
>> --check --initdb retains the initialized new cluster whether the
>> compatibility
>> checks succeed or fail. After a successful check, rerun pg_upgrade
>> against that
>> cluster without --check, --initdb, or --initdb-options.
> Does that mean that if you run "pg_upgrade .. --check --initdb" twice,
> it will fail the second time because the new cluster already exists?
> That doesn't sound nice.
Right, repeated checks now reuse the existing initialized cluster rather
than failing.
When it is reused, pg_upgrade prints a notice and skips --initdb-options,
while synchronization still honors --no-sync and --sync-method.
So the behavior is now as follows:
• Empty or missing target: initialize it, then check.
• Already initialized target: skip initialization and run the existing
checks.
• Invalid target or one containing user tables: reject it through those
checks.
> What if we just didn't do any of that validation and passed
> the initdb options directly to initdb, and let initdb fail if they're
> incorrect?
>
> If we must have some cross-checking like this, then I'd suggest adding a
> hidden "initdb --for-pg-upgrade" option or something, to let initdb know
> that it's been called as part of pg_ugprade, and have initdb itself
> reject the options that are not allowed with pg_upgrade.
Agreed, removed pg_upgrade's semantic validation and the duplicated option
table.
It still splits and quotes --initdb-options and appends them after the
generated
initdb arguments, so initdb validates the options and overlapping options
can
override those arguments.pg_upgrade runs its existing checks on the
resulting cluster.
While reworking the patch, I found additional issue regarding durability.
Previously, pg_upgrade --check --initdb returned successfully without
syncing
the retained new cluster to disk. The current patch's --check --initdb now
synchronizes the new cluster after stopping its server before the
successful return
by using initdb --sync-only and honoring --no-sync and --sync-method.
Best regards,
Bohyun
On Wed, Oct 14, 2026 at 7:58 AM <harrisontbooth(at)gmail(dot)com> wrote:
> I rebased the Windows test cleanup onto your branch at 170df44c. It
> detaches the two test-owned WAL junctions only after the test passes
> and preserves them for failed tests or PG_TEST_NOCLEAN.
>
> The full test passed on native ARM64 and emulated x64. I also verified
> both architectures retain the junctions after the existing long-path
> failures and with PG_TEST_NOCLEAN=0. Those path failures reproduce
> without this cleanup change.
>
> The attached mbox contains your five branch commits unchanged,
> followed by the cleanup supplement. It applies with git am on the
> recorded author base and master 6b3f8857; runtime testing used the
> recorded author-based builds.
>
> Let me know if this cleanup approach works for you.
>
> Best,
>
> Harrison
>
| Attachment | Content-Type | Size |
|---|---|---|
| v8-pg_upgrade-initdb.patch | application/octet-stream | 54.0 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Takeshi Ideriha | 2026-10-14 08:32:54 | Re: Fix doc: explanation of how postgres works when OOM killer is invoked |
| Previous Message | Fujii Masao | 2026-10-14 08:27:55 | Re: GIN jsonpath scans miss rows when methods follow a predicate |