From df4719db9a5432968bd00bdb44020b10cf2c2d97 Mon Sep 17 00:00:00 2001
From: Alena Vinter <dlaaren8@gmail.com>
Date: Tue, 2 Sep 2025 18:15:13 +0700
Subject: [PATCH v4 2/2] Reseting recovery target parameters in
 pg_createsubscriber

The utility sets recovery target params for correct recovery before
conversion a physical replica to a logical one but does not reset them
afterward. It may cause recovery failures in certain scenarios.
For example, if recovery begins from a checkpoint where no WAL records
need to be applied, the system may incorrectly determine that the
recovery target was never reached because these parameters remain
active.

This change ensures all recovery parameters are properly reset after
conversion to prevent such edge cases.
---
 src/bin/pg_basebackup/pg_createsubscriber.c   | 38 +++++++++++++++++++
 .../t/040_pg_createsubscriber.pl              | 18 +++++++++
 2 files changed, 56 insertions(+)

diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c b/src/bin/pg_basebackup/pg_createsubscriber.c
index 3986882f042..460871cdc64 100644
--- a/src/bin/pg_basebackup/pg_createsubscriber.c
+++ b/src/bin/pg_basebackup/pg_createsubscriber.c
@@ -161,6 +161,13 @@ enum WaitPMResult
 	POSTMASTER_STILL_STARTING
 };
 
+/*
+ * Buffer to preserve the original recovery conf contents before modifying
+ * recovery parameters. This allows restoration of the original configuration
+ * after the logical replication process completes, maintaining the system's
+ * previous recovery state.
+ */
+static PQExpBuffer savedrecoveryconfcontents;
 
 /*
  * Cleanup objects that were created by pg_createsubscriber if there is an
@@ -1236,6 +1243,9 @@ setup_recovery(const struct LogicalRepInfo *dbinfo, const char *datadir, const c
 	 */
 	conn = connect_database(dbinfo[0].pubconninfo, true);
 
+	/* Before setting up the recovery parameters save the original content. */
+	savedrecoveryconfcontents = GetRecoveryConfig(conn, datadir);
+
 	/*
 	 * Write recovery parameters.
 	 *
@@ -1285,6 +1295,31 @@ setup_recovery(const struct LogicalRepInfo *dbinfo, const char *datadir, const c
 	pg_log_debug("recovery parameters:\n%s", recoveryconfcontents->data);
 }
 
+/*
+ * Reset the previously set recovery parameters.
+ */
+static void
+reset_recovery_params(const struct LogicalRepInfo *dbinfo, const char *datadir)
+{
+	PGconn	   *conn;
+	PQExpBuffer recoveryconfcontents;
+
+	conn = connect_database(dbinfo[0].pubconninfo, true);
+
+	recoveryconfcontents = GenerateRecoveryConfig(conn, NULL, NULL);
+
+	appendPQExpBuffer(savedrecoveryconfcontents, "%s",
+					  recoveryconfcontents->data);
+
+	if (dry_run)
+		appendPQExpBufferStr(savedrecoveryconfcontents, "# dry run mode");
+	else
+		ReplaceRecoveryConfig(conn, datadir, savedrecoveryconfcontents);
+	disconnect_database(conn, false);
+
+	pg_log_debug("recovery parameters were reset");
+}
+
 /*
  * Drop physical replication slot on primary if the standby was using it. After
  * the transformation, it has no use.
@@ -2458,6 +2493,9 @@ main(int argc, char **argv)
 	pg_log_info("stopping the subscriber");
 	stop_standby_server(subscriber_dir);
 
+	/* Reset recovery parameters */
+	reset_recovery_params(dbinfos.dbinfo, subscriber_dir);
+
 	/* Change system identifier from subscriber */
 	modify_subscriber_sysid(&opt);
 
diff --git a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
index 229fef5b3b5..099f1553a5f 100644
--- a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
+++ b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
@@ -41,6 +41,17 @@ sub generate_db
 	return $dbname;
 }
 
+sub test_param_absent
+{
+	my ($node, $param) = @_;
+	my $auto_conf = $node->data_dir . '/postgresql.auto.conf';
+
+	return 1 unless -e $auto_conf;
+
+	my $content = slurp_file($auto_conf);
+	return $content !~ /^\s*$param\s*=/m;
+}
+
 #
 # Test mandatory options
 command_fails(['pg_createsubscriber'],
@@ -467,6 +478,13 @@ command_ok(
 	],
 	'run pg_createsubscriber on node S');
 
+# Verify that recovery parameters have been reset after pg_createsubscriber
+# We check recovery_target_lsn as a representative parameter - since all
+# recovery parameters are managed as a group, the absence of one indicates
+# that the entire set has been properly cleared from the configuration.
+ok( test_param_absent($node_s, 'recovery_target_lsn'),
+	'recovery_target_lsn parameter was removed');
+
 # Confirm the physical replication slot has been removed
 $result = $node_p->safe_psql($db1,
 	"SELECT count(*) FROM pg_replication_slots WHERE slot_name = '$slotname'"
-- 
2.39.5 (Apple Git-154)