On Fri, Sep 08, 2023 at 03:30:23AM +0000, Zhijie Hou (Fujitsu) wrote:
> I feel adding a check at pg_upgrade may not 100% detect the slot invalidation
> if we check by querying the old cluster to get the slot info, because the
> invalidation can happen before the first time we fetch the info or after the
> last time we fetch the info(e.g. shutdown checkpoint could also invalidate
> slots)
>
> Personally, I think if we really want to add a check, it might be better to put
> it at server side, Like: reporting an ERROR at server side when invalidating
> the slot(InvalidatePossiblyObsoleteSlot) if in upgrade mode.
Yeah, that may be enough to paint one isBinaryUpgrade in the
invalidation path of the backend, with en elog(ERROR) as that would be
an unexpected state.
> Having said that I feel it's fine if we don't add this check as setting
> max_slot_wal_keep_size to -1 looks sufficient.
I would do both, FWIW, to stay on the safe side. And both are
non-invasive.
--
Michael