On Tue, Oct 31, 2023 at 4:00 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> > > 5. A possible problem with this check_hook approach is that it doesn't
> > > let anyone setting max_slot_wal_keep_size to a value other than -1
> > > during pg_ugprade even if someone doesn't have logical slots or
> > > doesn't want to upgrade logical slots in which case the WAL file
> > > growth during pg_upgrade may be huge (transiently) unless the
> > > pg_resetwal step of pg_upgrade removes it at the end.
> >
> > While I doubt anyone wishes to set the variable to a specific value
> > during upgrade, think there are individuals who might be reluctant to
> > edit the config file due to unclear reasons. While we could consider
> > an alternative - checking for logical slots during binary upgrade-
> > it's debatable if the effort is justified. (I haven't verified its
> > feasibility, however.)
>
> Checking for logical slots during binary upgrade doesn't help - what
> if there are logical slots present but no upgrade is wanted (via a new
> pg_uprade option)? Basically, how will the postgres server know
> whether someone wants pg_upgrade of logical slots or not? Can we check
> if someone is overriding max_slot_wal_keep_size in pg_upgrade itself
> (via pg_settings query from the server)? If yes, if logical slots
> exist and upgrade is wanted, then disallow the upgrade if GUC is set
> to value other than -1.
>
I feel we can try to extend the functionality if we really see some
user demand. It is not that we can't do it now but it doesn't seem
prudent to make the functionality/code more complex than really
required.
> I believe disallowing setting max_slot_wal_keep_size to a value other
> than -1 during binary upgrade may have serious consequences as it
> impacts WAL retention before the pg_resetwal comes into picture as
> part of pg_upgrade.
>
I don't think this is completely true because this setting will only
impact if there are active slots and those slots need some WAL which
we want to remove. This setting shouldn't be used as often as you are
imagining.
> Or what if we just live with what we have right now? I mean with ERROR
> in InvalidatePossiblyObsoleteSlot().
>
> Or what if we just remove ERROR in InvalidatePossiblyObsoleteSlot or
> make it an Assert and say do not override max_slot_wal_keep_size in
> docs? Even if someone did override, let the pg_upgrade report the slot
> as invalidated and let the user delete the slot or decide what to do
> with it.
>
The problem is this can happen in the background so it can happen at
the time of shutdown when all the upgrade is complete.
--
With Regards,
Amit Kapila.