Re: A recent message added to pg_upgade - Mailing list pgsql-hackers

From Amit Kapila
Subject Re: A recent message added to pg_upgade
Date
Msg-id CAA4eK1Ls+VP1AEUOKQtXSX3gTxvF2Zz0ObJGnky2BpCFtyfp=A@mail.gmail.com
Whole thread Raw
In response to Re: A recent message added to pg_upgade  (Michael Paquier <michael@paquier.xyz>)
Responses Re: A recent message added to pg_upgade
Re: A recent message added to pg_upgade
List pgsql-hackers
On Mon, Nov 13, 2023 at 10:19 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Mon, Nov 13, 2023 at 08:45:12AM +0530, Amit Kapila wrote:
> > I think we can be specific about logical replication stuff. I have not
> > done any study on autovacuum behavior related to this, so we can
> > update about it separately if required.
>
> Autovacuum, as far as I recall, could decide to do some work before
> files are physically copied from the old to the new cluster,
> corrupting the new cluster.  Per 76dd09bbec89:
>
> +        *  If we have lost the autovacuum launcher, try to start a new one.
> +        *  We don't want autovacuum to run in binary upgrade mode because
> +        *  autovacuum might update relfrozenxid for empty tables before
> +        *  the physical files are put in place.
>
> > -       /* Use -b to disable autovacuum. */
> > +       /*
> > +        * Use -b to disable autovacuum and logical replication launcher
> > +        * (effective in PG17 or later for the latter).
> > +        *
> > +        * Logical replication workers can stream data during the
> > upgrade which can
> > +        * cause replication origins to move forward after we have copied them.
> > +        * It can cause the system to request the data which is already present
> > +        * in the new cluster.
> > +        */
> >
> > Now, ideally, such a comment change makes more sense along with the
> > main patch, so either we can go without this comment change or
> > probably wait till the main patch is ready and merge just before it or
> > along with it. I am fine either way.
>
> Another location would be to document that stuff directly in
> launcher.c where the check for IsBinaryUpgrade would be added.  You
> are right that it makes little sense to document that now, so how
> about:
> 1) keeping pg_upgrade.c minimal, say:
> -       /* Use -b to disable autovacuum. */
> +       /*
> +        * Use -b to disable autovacuum and logical replication
> +        * launcher (in 17~).
> +        */
> With a removal of the comment block related to
> max_logical_replication_workers=0?
> 2) Document that in ApplyLauncherRegister() as part of the main patch
> for the subscribers?
>

I am fine with this but there is no harm in doing this before or along
with the main patch. As of now, I don't see any problem but as the
main patch is still under review, so thought we could even wait for
the patch to become "Ready For Committer".

--
With Regards,
Amit Kapila.



pgsql-hackers by date:

Previous
From: zhihuifan1213@163.com
Date:
Subject: Re: POC PATCH: copy from ... exceptions to: (was Re: VLDB Features)
Next
From: Michael Paquier
Date:
Subject: Re: A recent message added to pg_upgade