Horton

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

Jonathan S. Katz <jkatz@postgresql.org>

View thread View on postgr.es
From: "Jonathan S. Katz" <jkatz@postgresql.org>
To: Amit Kapila <amit.kapila16@gmail.com>, vignesh C <vignesh21@gmail.com>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Julien Rouhaud <rjuju123@gmail.com>
Date: 2023-08-02T02:16:35Z
Lists: pgsql-hackers 
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

Commits

  1. Fix issues in binary_upgrade_logical_slot_has_caught_up().

  2. Fix a random failure in 003_logical_slots.pl.

  3. Fix a test in 003_logical_slots.

  4. Fix uninitialized slot array access during the upgrade.

  5. Fix the test 003_logical_slots.

  6. Commit b195e6d482 forgot to update meson.build.

  7. Use shorter file names in the upgrade logical slots test.

  8. Migrate logical slots to the new node during an upgrade.

  9. Flush logical slots to disk during a shutdown checkpoint if required.

  10. Prevent possibility of panics during shutdown checkpoint.