Re: [PoC] pg_upgrade: allow to upgrade publisher node

On 8/1/23 5:39 AM, Amit Kapila wrote:
> On Fri, Jul 28, 2023 at 5:48 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>>
>> Here is a patch which checks that there are no WAL records other than
>> CHECKPOINT_SHUTDOWN WAL record to be consumed based on the discussion
>> from [1].
>>
>
> Few comments:
> =============

> 2.
> + if (dopt.logical_slots_only)
> + {
> + if (!dopt.binary_upgrade)
> + pg_fatal("options --logical-replication-slots-only requires option
> --binary-upgrade");
> +
> + if (dopt.dataOnly)
> + pg_fatal("options --logical-replication-slots-only and
> -a/--data-only cannot be used together");
> +
> + if (dopt.schemaOnly)
> + pg_fatal("options --logical-replication-slots-only and
> -s/--schema-only cannot be used together");
>
> Can you please explain why the patch imposes these restrictions? I
> guess the binary_upgrade is because you want this option to be used
> for the upgrade. Do we want to avoid giving any other option with
> logical_slots, if so, are the above checks sufficient and why?

Can I take this a step further on the user interface and ask why the
flag would be "--include-logical-replication-slots" vs. being enabled by
default?

Are there reasons why we wouldn't enable this feature by default on
pg_upgrade, and instead (if need be) have a flag that would be
"--exclude-logical-replication-slots"? Right now, not having the ability
to run pg_upgrade with logical replication slots enabled on the
publisher is a a very big pain point for users, so I would strongly
recommend against adding friction unless there is a very large challenge
with such an implementation.

Thanks,

Jonathan