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@gmail.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