On 8/1/23 5:39 AM, Amit Kapila wrote:
On Fri, Jul 28, 2023 at 5:48 PM vignesh C <vignes...@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
OpenPGP_signature
Description: OpenPGP digital signature