On Wed, Nov 15, 2023 at 07:58:06AM +0530, Amit Kapila wrote: > 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".
My apologies for the delay. Now that 9a17be1e244a is in the tree, please find attached a patch to restrict the startup of the launcher using IsBinaryUpgrade in ApplyLauncherRegister(), with adjustments to the surrounding comments. Was there anything else you wanted to be covered and/or updated? -- Michael
diff --git a/src/backend/replication/logical/launcher.c b/src/backend/replication/logical/launcher.c
index 33f07f674d..fc09795467 100644
--- a/src/backend/replication/logical/launcher.c
+++ b/src/backend/replication/logical/launcher.c
@@ -925,7 +925,14 @@ ApplyLauncherRegister(void)
{
BackgroundWorker bgw;
- if (max_logical_replication_workers == 0)
+ /*
+ * The logical replication launcher is disabled during binary upgrades,
+ * as 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.
+ */
+ if (max_logical_replication_workers == 0 || IsBinaryUpgrade)
return;
memset(&bgw, 0, sizeof(bgw));
diff --git a/src/bin/pg_upgrade/server.c b/src/bin/pg_upgrade/server.c
index 64b24270e3..91bcb4dbc7 100644
--- a/src/bin/pg_upgrade/server.c
+++ b/src/bin/pg_upgrade/server.c
@@ -248,19 +248,14 @@ start_postmaster(ClusterInfo *cluster, bool report_and_exit_on_error)
* invalidation of slots during the upgrade. We set this option when
* cluster is PG17 or later because logical replication slots can only be
* migrated since then. Besides, max_slot_wal_keep_size is added in PG13.
- *
- * Use max_logical_replication_workers as 0 to prevent a startup of the
- * logical replication launcher while upgrading because it may start apply
- * workers that could start receiving changes from the publisher before
- * the physical files are put in place, causing corruption on the new
- * cluster upgrading to. Like the previous parameter, this is set only
- * when a cluster is PG17 or later as logical slots can only be migrated
- * since this version.
*/
if (GET_MAJOR_VERSION(cluster->major_version) >= 1700)
- appendPQExpBufferStr(&pgoptions, " -c max_slot_wal_keep_size=-1 -c max_logical_replication_workers=0");
+ appendPQExpBufferStr(&pgoptions, " -c max_slot_wal_keep_size=-1");
- /* Use -b to disable autovacuum. */
+ /*
+ * Use -b to disable autovacuum and logical replication launcher
+ * (effective in PG17 or later for the latter).
+ */
snprintf(cmd, sizeof(cmd),
"\"%s/pg_ctl\" -w -l \"%s/%s\" -D \"%s\" -o \"-p %d -b%s %s%s\" start",
cluster->bindir,
signature.asc
Description: PGP signature
