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

On Mon, Sep 25, 2023 at 12:30 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Mon, Sep 25, 2023 at 11:15 AM Bharath Rupireddy
> <bharath.rupireddyforpostgres@gmail.com> wrote:
> >
> > On Fri, Sep 22, 2023 at 12:11 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > Yeah, both by tests and manually verifying the WAL records. Basically,
> > > we need to care about records that could be generated by background
> > > processes like checkpointer/bgwriter or can be generated during system
> > > table scans. You may want to read my latest email for a summary on how
> > > we reached at this design choice [1].
> > >
> > > [1] - https://www.postgresql.org/message-id/CAA4eK1JVKZGRHLOEotWi%2Be%2B09jucNedqpkkc-Do4dh5FTAU%2B5w%40mail.gmail.com
> >
> > +    /* Logical slots can be migrated since PG17. */
> > +    if (GET_MAJOR_VERSION(old_cluster.major_version) <= 1600)
> > +    {
> >
> > Why can't the patch allow migration of logical replication slots from
> > PG versions < 17 to say 17 or later? If done, it will be a main
> > advantage of the patch since it will enable seamless major version
> > upgrades of postgres database instances with logical replication
> > slots.
> >
> > I'm looking at the changes to the postgres backend that this patch
> > does - AFICS, it does 2 things 1) implements
> > binary_upgrade_validate_wal_logical_end function, 2) adds an assertion
> > that the logical slots won't get invalidated. For (1), pg_upgrade can
> > itself can read the WAL from the old cluster to determine the logical
> > WAL end (i.e. implement the functionality of
> > binary_upgrade_validate_wal_logical_end ) because the xlogreader is
> > available to FRONTEND tools. For (2), it's just an assertion and
> > logical WAL end determining logic will anyway determine whether or not
> > the slots are valid; if needed, the assertion can be backported.
> >
> > 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

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com