On Mon, Sep 25, 2023 at 1:23 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> On Mon, Sep 25, 2023 at 12:32 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > > > Is there anything else that stops this patch from supporting migration
> > > > of logical replication slots from PG versions < 17?
> > >
> > > IMHO one of the main change we are doing in PG 17 is that on shutdown
> > > checkpoint we are ensuring that if the confirmed flush lsn is updated
> > > since the last checkpoint and that is not yet synched to the disk then
> > > we are doing so. I think this is the most important change otherwise
> > > many slots for which we have already streamed all the WAL might give
> > > an error assuming that there are pending WAL from the slots which are
> > > not yet confirmed.
> > >
> >
> > You might need to refer to [1] for the change I am talking about
> >
> > [1]
https://www.postgresql.org/message-id/CAA4eK1%2BLtWDKXvxS7gnJ562VX%2Bs3C6%2B0uQWamqu%3DUuD8hMfORg%40mail.gmail.com
>
> I see. IIUC, without that commit e0b2eed [1], it may happen that the
> slot's on-disk confirmed_flush LSN value can be higher than the WAL
> LSN that's flushed to disk, no?
>
No, without that commit, there is a very high possibility that even if
we have sent the WAL to the subscriber and got the acknowledgment of
the same, we would miss updating it before shutdown. This would lead
to upgrade failures because upgrades have no way to later identify
whether the remaining WAL records are sent to the subscriber.
> If so, can't it be detected if the WAL
> at confirmed_flush LSN is valid or not when reading WAL with
> xlogreader machinery?
>
> What if the commit e0b2eed [1] is treated to be fixing a bug with the
> reasoning [2] and backpatch? When done so, it's easy to support
> upgradation/migration of logical replication slots from PG versions <
> 17, no?
>
Yeah, we could try to make a case to backpatch it but when I raised
that point there was not much consensus on backpatching it. We are
aware and understand that if we could backpatch it then the prior
version slots be upgraded but the case to backpatch needs broader
consensus. For now, the idea is to get the core of the functionality
to be committed and then we can see if we get the consensus on
backpatching the commit you mentioned and probably changing the
version checks in this work.
--
With Regards,
Amit Kapila.