v9-0001-pg_createsubscriber-use-include_if_exists-for-recove.patch

text/x-patch

Filename: v9-0001-pg_createsubscriber-use-include_if_exists-for-recove.patch
Type: text/x-patch
Part: 0
Message: Re: Resetting recovery target parameters in pg_createsubscriber
From fc1a4bdf8ee44f4b7a2da81bd6a1164eef6855f4 Mon Sep 17 00:00:00 2001
From: Alena Vinter <dlaaren8@gmail.com>
Date: Tue, 30 Dec 2025 13:56:18 +0700
Subject: [PATCH 1/2] pg_createsubscriber: use include_if_exists for recovery
 config and disable on exit

Write temporary recovery parameters to pg_createsubscriber.conf and
include it from postgresql.auto.conf via include_if_exists.  Upon
completion or failure, rename the file to
pg_createsubscriber.conf.disabled so it is ignored on subsequent
restarts.
---
 src/bin/pg_basebackup/pg_createsubscriber.c   | 77 ++++++++++++++-----
 .../t/040_pg_createsubscriber.pl              | 34 ++++++++
 2 files changed, 90 insertions(+), 21 deletions(-)

diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c b/src/bin/pg_basebackup/pg_createsubscriber.c
index dab4dfb3a52..81ab4641a31 100644
--- a/src/bin/pg_basebackup/pg_createsubscriber.c
+++ b/src/bin/pg_basebackup/pg_createsubscriber.c
@@ -29,9 +29,13 @@
 #include "fe_utils/string_utils.h"
 #include "fe_utils/version.h"
 #include "getopt_long.h"
+#include "common/file_utils.h"

 #define	DEFAULT_SUB_PORT	"50432"
 #define	OBJECTTYPE_PUBLICATIONS  0x0001
+#define PG_AUTOCONF_FILENAME	"postgresql.auto.conf"
+#define INCLUDED_CONF_FILE		"pg_createsubscriber.conf"
+#define INCLUDED_CONF_FILE_DISABLED		"pg_createsubscriber.conf.disabled"

 /* Command-line options */
 struct CreateSubscriberOptions
@@ -156,22 +160,43 @@ static char *subscriber_dir = NULL;

 static bool recovery_ended = false;
 static bool standby_running = false;
-
+static bool recovery_params_set = false;

 /*
- * Cleanup objects that were created by pg_createsubscriber if there is an
- * error.
+ * Clean up objects created by pg_createsubscriber.
  *
- * Publications and replication slots are created on primary. Depending on the
- * step it failed, it should remove the already created objects if it is
- * possible (sometimes it won't work due to a connection issue).
- * There is no cleanup on the target server. The steps on the target server are
- * executed *after* promotion, hence, at this point, a failure means recreate
- * the physical replica and start again.
+ * Publications and replication slots are created on the primary. Depending on
+ * the step where it failed, already created objects should be removed if
+ * possible (sometimes this won't work due to a connection issue).
+ * There is no cleanup on the target server *after* its promotion, because any
+ * failure at this point means recreating the physical replica and starting
+ * again.
  */
 static void
 cleanup_objects_atexit(void)
 {
+	/*
+	 * Disable the recovery configuration by renaming the
+	 * "pg_createsubscriber.conf" file. Since "postgresql.auto.conf" uses
+	 * `include_if_exists`, postgres will silently ignore the missing file.
+	 * The renamed file is preserved fo debugging.
+	 */
+	if (recovery_params_set)
+	{
+		char conf_filename[MAXPGPATH];
+		char conf_filename_disabled[MAXPGPATH];
+		int err;
+
+		snprintf(conf_filename, MAXPGPATH, "%s/%s", subscriber_dir, INCLUDED_CONF_FILE);
+		snprintf(conf_filename_disabled, MAXPGPATH, "%s/%s", subscriber_dir, INCLUDED_CONF_FILE_DISABLED);
+
+		if ((err = durable_rename(conf_filename, conf_filename_disabled)) != 0)
+		{
+			pg_log_warning("recovery parameters were set but not properly reset on the target");
+			pg_log_warning_hint("manual removal of recovery parameters is required from");
+		}
+	}
+
 	if (success)
 		return;

@@ -1322,23 +1347,33 @@ setup_recovery(const struct LogicalRepInfo *dbinfo, const char *datadir, const c
 	appendPQExpBufferStr(recoveryconfcontents, "recovery_target_name = ''\n");
 	appendPQExpBufferStr(recoveryconfcontents, "recovery_target_time = ''\n");
 	appendPQExpBufferStr(recoveryconfcontents, "recovery_target_xid = ''\n");
+	appendPQExpBuffer(recoveryconfcontents, "recovery_target_lsn = '%s'\n", lsn);

-	if (dry_run)
-	{
-		appendPQExpBufferStr(recoveryconfcontents, "# dry run mode\n");
-		appendPQExpBuffer(recoveryconfcontents,
-						  "recovery_target_lsn = '%X/%08X'\n",
-						  LSN_FORMAT_ARGS((XLogRecPtr) InvalidXLogRecPtr));
-	}
-	else
+	pg_log_debug("recovery parameters:\n%s", recoveryconfcontents->data);
+
+	if (!dry_run)
 	{
-		appendPQExpBuffer(recoveryconfcontents, "recovery_target_lsn = '%s'\n",
-						  lsn);
+		char  conf_filename[MAXPGPATH];
+		FILE *fd;
+
+		recovery_params_set = true;
+
+		snprintf(conf_filename, MAXPGPATH, "%s/%s", datadir, INCLUDED_CONF_FILE);
+
+		fd = fopen(conf_filename, "w");
+		if (fd == NULL)
+			pg_fatal("could not open file \"%s\": %m", conf_filename);
+
+		if (fwrite(recoveryconfcontents->data, recoveryconfcontents->len, 1, fd) != 1)
+			pg_fatal("could not write to file \"%s\": %m", conf_filename);
+
+		fclose(fd);
+
+		resetPQExpBuffer(recoveryconfcontents);
+		appendPQExpBufferStr(recoveryconfcontents, "include_if_exists '" INCLUDED_CONF_FILE "'");
 		WriteRecoveryConfig(conn, datadir, recoveryconfcontents);
 	}
 	disconnect_database(conn, false);
-
-	pg_log_debug("recovery parameters:\n%s", recoveryconfcontents->data);
 }

 /*
diff --git a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
index 4657172c9ac..8a9200aead7 100644
--- a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
+++ b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
@@ -160,6 +160,7 @@ primary_slot_name = '$slotname'
 primary_conninfo = '$pconnstr dbname=postgres'
 hot_standby_feedback = on
 ]);
+my $sconnstr = $node_s->connstr;
 $node_s->set_standby_mode();
 $node_s->start;

@@ -579,10 +580,43 @@ is($result, qq($db1|{test_pub3}
 $db2|{pub2}),
 	"subscriptions use the correct publications");

+
+# Create a physical standby from the promoted subscriber
+$node_s->safe_psql('postgres',
+    "SELECT pg_create_physical_replication_slot('$slotname');");
+
+# Create backup from promoted subscriber
+$node_s->backup('backup_3');
+
+# Initialize new physical standby
+my $node_k = PostgreSQL::Test::Cluster->new('node_k');
+$node_k->init_from_backup($node_s, 'backup_3',
+    has_streaming => 1);
+
+# Configure the new standby
+$node_k->append_conf('postgresql.conf', qq[
+primary_slot_name = '$slotname'
+primary_conninfo = '$sconnstr dbname=postgres'
+hot_standby_feedback = on
+]);
+
+$node_k->set_standby_mode();
+my $node_k_name = $node_s->name;
+command_ok(
+	[
+		'pg_ctl', '--wait',
+		'--pgdata' => $node_k->data_dir,
+		'--log' => $node_k->logfile,
+		'--options' => "--cluster-name=$node_k_name",
+		'start'
+	],
+	"node_k has started");
+
 # clean up
 $node_p->teardown_node;
 $node_s->teardown_node;
 $node_t->teardown_node;
 $node_f->teardown_node;
+$node_k->teardown_node;

 done_testing();
--
2.52.0