v19-0007-Address-comments-from-Vignesh.patch

From 0be661e54b28025fcbf7e62100d59313f51ce097 Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Date: Tue, 13 Feb 2024 11:57:21 +0000
Subject: [PATCH v19 7/9] Address comments from Vignesh

---
 src/bin/pg_basebackup/.gitignore            |  2 +-
 src/bin/pg_basebackup/pg_createsubscriber.c | 41 ++++++++++++---------
 2 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/src/bin/pg_basebackup/.gitignore b/src/bin/pg_basebackup/.gitignore
index b3a6f5a2fe..14d5de6c01 100644
--- a/src/bin/pg_basebackup/.gitignore
+++ b/src/bin/pg_basebackup/.gitignore
@@ -1,6 +1,6 @@
 /pg_basebackup
+/pg_createsubscriber
 /pg_receivewal
 /pg_recvlogical
-/pg_createsubscriber
 
 /tmp_check/
diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c b/src/bin/pg_basebackup/pg_createsubscriber.c
index c21fd212e1..a20cec8312 100644
--- a/src/bin/pg_basebackup/pg_createsubscriber.c
+++ b/src/bin/pg_basebackup/pg_createsubscriber.c
@@ -10,27 +10,21 @@
  *
  *-------------------------------------------------------------------------
  */
+
 #include "postgres_fe.h"
 
-#include <signal.h>
-#include <sys/stat.h>
 #include <sys/time.h>
-#include <sys/wait.h>
 #include <time.h>
 
-#include "access/xlogdefs.h"
 #include "catalog/pg_authid_d.h"
-#include "catalog/pg_control.h"
 #include "common/connect.h"
 #include "common/controldata_utils.h"
 #include "common/file_perm.h"
-#include "common/file_utils.h"
 #include "common/logging.h"
 #include "common/restricted_token.h"
 #include "fe_utils/recovery_gen.h"
 #include "fe_utils/simple_list.h"
 #include "getopt_long.h"
-#include "utils/pidfile.h"
 
 #define	PGS_OUTPUT_DIR	"pg_createsubscriber_output.d"
 
@@ -114,12 +108,10 @@ static bool success = false;
 static LogicalRepInfo *dbinfo;
 static int	num_dbs = 0;
 
-enum WaitPMResult
+enum PCS_WaitPMResult
 {
-	POSTMASTER_READY,
-	POSTMASTER_STANDBY,
-	POSTMASTER_STILL_STARTING,
-	POSTMASTER_FAILED
+	PCS_READY,
+	PCS_STILL_STARTING,
 };
 
 
@@ -148,8 +140,6 @@ cleanup_objects_atexit(void)
 			if (conn != NULL)
 			{
 				drop_subscription(conn, &dbinfo[i]);
-				if (dbinfo[i].made_publication)
-					drop_publication(conn, &dbinfo[i]);
 				disconnect_database(conn);
 			}
 		}
@@ -181,7 +171,7 @@ usage(void)
 	printf(_(" -P, --publisher-server=CONNSTR      publisher connection string\n"));
 	printf(_(" -S, --subscriber-server=CONNSTR     subscriber connection string\n"));
 	printf(_(" -d, --database=DBNAME               database to create a subscription\n"));
-	printf(_(" -n, --dry-run                       stop before modifying anything\n"));
+	printf(_(" -n, --dry-run                       check clusters only, don't change target server\n"));
 	printf(_(" -t, --recovery-timeout=SECS         seconds to wait for recovery to end\n"));
 	printf(_(" -r, --retain                        retain log file after success\n"));
 	printf(_(" -v, --verbose                       output verbose messages\n"));
@@ -400,6 +390,7 @@ connect_database(const char *conninfo)
 	{
 		pg_log_error("could not clear search_path: %s",
 					 PQresultErrorMessage(res));
+		PQfinish(conn);
 		return NULL;
 	}
 	PQclear(res);
@@ -1045,6 +1036,9 @@ drop_replication_slot(PGconn *conn, LogicalRepInfo *dbinfo,
 						 slot_name, dbinfo->dbname, PQerrorMessage(conn));
 
 		PQclear(res);
+
+		/* Reset a flag accordingly */
+		dbinfo->made_replslot = false;
 	}
 
 	destroyPQExpBuffer(str);
@@ -1168,7 +1162,7 @@ wait_for_end_recovery(const char *conninfo, const char *pg_bin_dir,
 {
 	PGconn	   *conn;
 	PGresult   *res;
-	int			status = POSTMASTER_STILL_STARTING;
+	int			status = PCS_STILL_STARTING;
 	int			timer = 0;
 
 	pg_log_info("waiting the postmaster to reach the consistent state");
@@ -1199,7 +1193,7 @@ wait_for_end_recovery(const char *conninfo, const char *pg_bin_dir,
 		 */
 		if (!in_recovery || dry_run)
 		{
-			status = POSTMASTER_READY;
+			status = PCS_READY;
 			break;
 		}
 
@@ -1218,7 +1212,7 @@ wait_for_end_recovery(const char *conninfo, const char *pg_bin_dir,
 
 	disconnect_database(conn);
 
-	if (status == POSTMASTER_STILL_STARTING)
+	if (status == PCS_STILL_STARTING)
 		pg_fatal("server did not end recovery");
 
 	pg_log_info("postmaster reached the consistent state");
@@ -1336,6 +1330,14 @@ drop_publication(PGconn *conn, LogicalRepInfo *dbinfo)
 						 dbinfo->pubname, dbinfo->dbname, PQerrorMessage(conn));
 
 		PQclear(res);
+
+		/*
+		 * XXX Apart from other dropping functions, we must not reset a flag
+		 * for publication, because the flag indicates the status of both
+		 * nodes. Even if current execution drops a publication on subscriber,
+		 * the primary still has it. This flag must be kept to remember it.
+		 */
+		dbinfo->made_publication = false;
 	}
 
 	destroyPQExpBuffer(str);
@@ -1418,6 +1420,9 @@ drop_subscription(PGconn *conn, LogicalRepInfo *dbinfo)
 						 dbinfo->subname, dbinfo->dbname, PQerrorMessage(conn));
 
 		PQclear(res);
+
+		/* Reset a flag accordingly */
+		dbinfo->made_subscription = false;
 	}
 
 	destroyPQExpBuffer(str);
-- 
2.43.0