From 3e51a85e075623596e57162236655c74503bc1de Mon Sep 17 00:00:00 2001
From: Ian Barwick <barwick@gmail.com>
Date: Tue, 20 Sep 2016 09:16:33 +0900
Subject: [PATCH] repmgr: consolidate error messages during replication slot
 generation

Return error messages to the called so they can be logged as events;
prevent log message duplication in one case.
---
 dbutils.c | 21 ++++++++++++---------
 dbutils.h |  3 ++-
 repmgr.c  | 31 +++++++++++++++++++++----------
 3 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/dbutils.c b/dbutils.c
index 83f6bccf..fd1080b8 100644
--- a/dbutils.c
+++ b/dbutils.c
@@ -945,7 +945,7 @@ get_repmgr_schema_quoted(PGconn *conn)
 
 
 bool
-create_replication_slot(PGconn *conn, char *slot_name, int server_version_num)
+create_replication_slot(PGconn *conn, char *slot_name, int server_version_num, PQExpBufferData *error_msg)
 {
 	char				sqlquery[QUERY_STR_LEN];
 	int					query_res;
@@ -964,8 +964,9 @@ create_replication_slot(PGconn *conn, char *slot_name, int server_version_num)
 	{
 		if (strcmp(slot_info.slot_type, "physical") != 0)
 		{
-			log_err(_("Slot '%s' exists and is not a physical slot\n"),
-					slot_name);
+			appendPQExpBuffer(error_msg,
+							  _("Slot '%s' exists and is not a physical slot\n"),
+							  slot_name);
 			return false;
 		}
 
@@ -977,8 +978,9 @@ create_replication_slot(PGconn *conn, char *slot_name, int server_version_num)
 			return true;
 		}
 
-		log_err(_("Slot '%s' already exists as an active slot\n"),
-				slot_name);
+		appendPQExpBuffer(error_msg,
+						  _("Slot '%s' already exists as an active slot\n"),
+						  slot_name);
 		return false;
 	}
 
@@ -996,15 +998,16 @@ create_replication_slot(PGconn *conn, char *slot_name, int server_version_num)
 						  slot_name);
 	}
 
-	log_debug(_("create_replication_slot(): Creating slot '%s' on primary\n"), slot_name);
+	log_debug(_("create_replication_slot(): Creating slot '%s' on master\n"), slot_name);
 	log_verbose(LOG_DEBUG, "create_replication_slot():\n%s\n", sqlquery);
 
 	res = PQexec(conn, sqlquery);
 	if (!res || PQresultStatus(res) != PGRES_TUPLES_OK)
 	{
-		log_err(_("unable to create slot '%s' on the primary node: %s\n"),
-				slot_name,
-				PQerrorMessage(conn));
+		appendPQExpBuffer(error_msg,
+						  _("unable to create slot '%s' on the master node: %s\n"),
+						  slot_name,
+						  PQerrorMessage(conn));
 		PQclear(res);
 		return false;
 	}
diff --git a/dbutils.h b/dbutils.h
index ce9135bc..e3aebd79 100644
--- a/dbutils.h
+++ b/dbutils.h
@@ -22,6 +22,7 @@
 #define _REPMGR_DBUTILS_H_
 
 #include "access/xlogdefs.h"
+#include "pqexpbuffer.h"
 
 #include "config.h"
 #include "strutil.h"
@@ -118,7 +119,7 @@ int			wait_connection_availability(PGconn *conn, long long timeout);
 bool		cancel_query(PGconn *conn, int timeout);
 char       *get_repmgr_schema(void);
 char       *get_repmgr_schema_quoted(PGconn *conn);
-bool		create_replication_slot(PGconn *conn, char *slot_name, int server_version_num);
+bool		create_replication_slot(PGconn *conn, char *slot_name, int server_version_num, PQExpBufferData *error_msg);
 int			get_slot_record(PGconn *conn, char *slot_name, t_replication_slot *record);
 bool		drop_replication_slot(PGconn *conn, char *slot_name);
 bool		start_backup(PGconn *conn, char *first_wal_segment, bool fast_checkpoint);
diff --git a/repmgr.c b/repmgr.c
index 09971b81..d81ca150 100644
--- a/repmgr.c
+++ b/repmgr.c
@@ -2309,11 +2309,25 @@ do_standby_clone(void)
 	 */
 	if (mode != barman && options.use_replication_slots)
 	{
-		if (create_replication_slot(source_conn, repmgr_slot_name, server_version_num) == false)
+		PQExpBufferData event_details;
+		initPQExpBuffer(&event_details);
+
+		if (create_replication_slot(source_conn, repmgr_slot_name, server_version_num, &event_details) == false)
 		{
+			log_err("%s\n", event_details.data);
+
+			create_event_record(primary_conn,
+								&options,
+								options.node,
+								"standby_clone",
+								false,
+								event_details.data);
+
 			PQfinish(source_conn);
 			exit(ERR_DB_QUERY);
 		}
+
+		termPQExpBuffer(&event_details);
 	}
 
 	if (mode == rsync)
@@ -3576,16 +3590,11 @@ do_standby_follow(void)
 	{
  		int	server_version_num = get_server_version(master_conn, NULL);
 
-		if (create_replication_slot(master_conn, repmgr_slot_name, server_version_num) == false)
+		PQExpBufferData event_details;
+		initPQExpBuffer(&event_details);
+
+		if (create_replication_slot(master_conn, repmgr_slot_name, server_version_num, &event_details) == false)
 		{
-			PQExpBufferData event_details;
-			initPQExpBuffer(&event_details);
-
-			appendPQExpBuffer(&event_details,
-							  _("Unable to create slot '%s' on the master node: %s"),
-							  repmgr_slot_name,
-							  PQerrorMessage(master_conn));
-
 			log_err("%s\n", event_details.data);
 
 			create_event_record(master_conn,
@@ -3598,6 +3607,8 @@ do_standby_follow(void)
 			PQfinish(master_conn);
 			exit(ERR_DB_QUERY);
 		}
+
+		termPQExpBuffer(&event_details);
 	}
 
 	/* XXX add more detail! */