On Mon, Nov 13, 2023 at 10:19 AM Michael Paquier <mich...@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.
