v1-0001-Fix-incorrect-cleanup-flag-handling-in-pg_creates.patch

From ec934502f2da4e7822b851c040f9b832dd90b0a5 Mon Sep 17 00:00:00 2001
From: Nisha Moond <nisha.moond412@gmail.com>
Date: Mon, 14 Apr 2025 12:10:12 +0530
Subject: [PATCH v1] Fix incorrect cleanup flag handling in pg_createsubscriber

The flags made_publication and made_replslot track whether the tool
created a publication or replication slot on the primary. These are
used during error handling to clean up internal objects on primary.

Previously, these flags were incorrectly reset to false when failures
occurred while dropping objects(publications/replication slots) on the
subscriber. As a result, upon a failure, the cleanup_objects_atexit()
skipped cleanup of these objects on the primary.

This patch fixes the issue by making sure not to set the flags to false
if the failure occurs while dropping objects on the subscriber side.
---
 src/bin/pg_basebackup/pg_createsubscriber.c | 34 +++++++++++++--------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c b/src/bin/pg_basebackup/pg_createsubscriber.c
index f65acc7cb11..d20c19c4ec3 100644
--- a/src/bin/pg_basebackup/pg_createsubscriber.c
+++ b/src/bin/pg_basebackup/pg_createsubscriber.c
@@ -105,7 +105,7 @@ static void drop_failover_replication_slots(struct LogicalRepInfo *dbinfo);
 static char *create_logical_replication_slot(PGconn *conn,
 											 struct LogicalRepInfo *dbinfo);
 static void drop_replication_slot(PGconn *conn, struct LogicalRepInfo *dbinfo,
-								  const char *slot_name);
+								  const char *slot_name, bool in_cleanup);
 static void pg_ctl_status(const char *pg_ctl_cmd, int rc);
 static void start_standby_server(const struct CreateSubscriberOptions *opt,
 								 bool restricted_access,
@@ -115,7 +115,8 @@ static void wait_for_end_recovery(const char *conninfo,
 								  const struct CreateSubscriberOptions *opt);
 static void create_publication(PGconn *conn, struct LogicalRepInfo *dbinfo);
 static void drop_publication(PGconn *conn, const char *pubname,
-							 const char *dbname, bool *made_publication);
+							 const char *dbname, bool *made_publication,
+							 bool in_cleanup);
 static void check_and_drop_publications(PGconn *conn, struct LogicalRepInfo *dbinfo);
 static void create_subscription(PGconn *conn, const struct LogicalRepInfo *dbinfo);
 static void set_replication_progress(PGconn *conn, const struct LogicalRepInfo *dbinfo,
@@ -204,9 +205,9 @@ cleanup_objects_atexit(void)
 			{
 				if (dbinfo->made_publication)
 					drop_publication(conn, dbinfo->pubname, dbinfo->dbname,
-									 &dbinfo->made_publication);
+									 &dbinfo->made_publication, true);
 				if (dbinfo->made_replslot)
-					drop_replication_slot(conn, dbinfo, dbinfo->replslotname);
+					drop_replication_slot(conn, dbinfo, dbinfo->replslotname, true);
 				disconnect_database(conn, false);
 			}
 			else
@@ -1295,7 +1296,7 @@ drop_primary_replication_slot(struct LogicalRepInfo *dbinfo, const char *slotnam
 	conn = connect_database(dbinfo[0].pubconninfo, false);
 	if (conn != NULL)
 	{
-		drop_replication_slot(conn, &dbinfo[0], slotname);
+		drop_replication_slot(conn, &dbinfo[0], slotname, false);
 		disconnect_database(conn, false);
 	}
 	else
@@ -1330,7 +1331,7 @@ drop_failover_replication_slots(struct LogicalRepInfo *dbinfo)
 		{
 			/* Remove failover replication slots from subscriber */
 			for (int i = 0; i < PQntuples(res); i++)
-				drop_replication_slot(conn, &dbinfo[0], PQgetvalue(res, i, 0));
+				drop_replication_slot(conn, &dbinfo[0], PQgetvalue(res, i, 0), false);
 		}
 		else
 		{
@@ -1407,7 +1408,7 @@ create_logical_replication_slot(PGconn *conn, struct LogicalRepInfo *dbinfo)
 
 static void
 drop_replication_slot(PGconn *conn, struct LogicalRepInfo *dbinfo,
-					  const char *slot_name)
+					  const char *slot_name, bool in_cleanup)
 {
 	PQExpBuffer str = createPQExpBuffer();
 	char	   *slot_name_esc;
@@ -1433,7 +1434,9 @@ drop_replication_slot(PGconn *conn, struct LogicalRepInfo *dbinfo,
 		{
 			pg_log_error("could not drop replication slot \"%s\" in database \"%s\": %s",
 						 slot_name, dbinfo->dbname, PQresultErrorMessage(res));
-			dbinfo->made_replslot = false;	/* don't try again. */
+
+			if (in_cleanup)
+				dbinfo->made_replslot = false;	/* don't try again. */
 		}
 
 		PQclear(res);
@@ -1675,7 +1678,7 @@ create_publication(PGconn *conn, struct LogicalRepInfo *dbinfo)
  */
 static void
 drop_publication(PGconn *conn, const char *pubname, const char *dbname,
-				 bool *made_publication)
+				 bool *made_publication, bool in_cleanup)
 {
 	PQExpBuffer str = createPQExpBuffer();
 	PGresult   *res;
@@ -1701,7 +1704,6 @@ drop_publication(PGconn *conn, const char *pubname, const char *dbname,
 		{
 			pg_log_error("could not drop publication \"%s\" in database \"%s\": %s",
 						 pubname, dbname, PQresultErrorMessage(res));
-			*made_publication = false;	/* don't try again. */
 
 			/*
 			 * Don't disconnect and exit here. This routine is used by primary
@@ -1709,7 +1711,15 @@ drop_publication(PGconn *conn, const char *pubname, const char *dbname,
 			 * subscriber (remove the replicated publications). In both cases,
 			 * it can continue and provide instructions for the user to remove
 			 * it later if cleanup fails.
+			 *
+			 * in_cleanup is false when dropping an existing publication on
+			 * the subscriber (e.g., when --remove=publications is specified).
+			 * In this case, the flag 'made_publication' must remain true
+			 * because the publication created on the primary still needs to
+			 * be cleaned up if a failure occurs later.
 			 */
+			if (in_cleanup)
+				*made_publication = false;	/* don't try again. */
 		}
 		PQclear(res);
 	}
@@ -1752,7 +1762,7 @@ check_and_drop_publications(PGconn *conn, struct LogicalRepInfo *dbinfo)
 		/* Drop each publication */
 		for (int i = 0; i < PQntuples(res); i++)
 			drop_publication(conn, PQgetvalue(res, i, 0), dbinfo->dbname,
-							 &dbinfo->made_publication);
+							 &dbinfo->made_publication, false);
 
 		PQclear(res);
 	}
@@ -1763,7 +1773,7 @@ check_and_drop_publications(PGconn *conn, struct LogicalRepInfo *dbinfo)
 	 */
 	if (!drop_all_pubs || dry_run)
 		drop_publication(conn, dbinfo->pubname, dbinfo->dbname,
-						 &dbinfo->made_publication);
+						 &dbinfo->made_publication, false);
 }
 
 /*
-- 
2.34.1