Re: A recent message added to pg_upgade

On Thu, Nov 9, 2023 at 3:55 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Thu, Nov 09, 2023 at 09:53:07AM +0530, Amit Kapila wrote:
> > On Tue, Nov 7, 2023 at 4:16 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >> But then we don't need the hardcoded value of
> >> max_logical_replication_workers as zero by pg_upgrade. I think doing
> >> IsBinaryUpgrade for slots won't be neat, so we anyway need to keep
> >> using the special value of max_slot_wal_keep_size GUC. Though the
> >> handling for both won't be the same but I guess given the situation,
> >> that seems like a reasonable thing to do. If we follow that then we
> >> can have this special GUC hook only for max_slot_wal_keep_size GUC.
> >
> > Michael, Horiguchi-San, and others, do you have any thoughts on what
> > is the best way to proceed?
>
> No problem for me to use a GUC hook for the WAL retention GUCs if you
> feel strongly about it at the end, but I'd rather use an approach
> based on IsBinaryUpgrade for the logical worker launcher to be
> consistent with autovacuum (where there's also an argument to refactor
> it to use a bgworker registration, centralizing the checks on
> IsBinaryUpgrade for all bgworkers, but that would be material for a
> different thread, if there's interest in doing that).
>
> The two situations we are trying to prevent (slot invalidation and
> bgworker launch) can be triggered under different contexts, so they
> don't have to use the same mechanisms to prevent what should not
> happen during an upgrade.
> --

Having a GUC hook for the "max_slot_wal_keep_size" seemed OK to me. If
the user overrides a GUC value (admittedly, maybe there is no reason
why they would want to) then at least the hook will give an error,
rather than us silently overwriting the user's value with -1.

So, patch v4 LGTM, except it is better to include a test case.

~

Meanwhile, if preventing the apply worker launch is considered better
to be implemented differently in ApplyLauncherRegister, then so be it.

======
Kind Regards,
Peter Smith.
Fujitsu Australia