From c7423ebb44e1845aa5d5eb480f5fc6b0f24652d1 Mon Sep 17 00:00:00 2001
From: Ian Barwick <ian@2ndquadrant.com>
Date: Thu, 31 Aug 2017 23:54:52 +0900
Subject: [PATCH] Various minor fixes

---
 README.md               | 19 ++++++++++++++-----
 repmgr-action-cluster.c |  2 +-
 repmgr-action-standby.c | 15 ++++++++++++---
 repmgr-client.c         | 17 +++++++----------
 4 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/README.md b/README.md
index bdb4e040..e94b97bf 100644
--- a/README.md
+++ b/README.md
@@ -285,7 +285,6 @@ both servers.
 On the primary server, a PostgreSQL instance must be initialised and running.
 The following replication settings may need to be adjusted:
 
-
     # Enable replication connections; set this figure to at least one more
     # than the number of standbys which will connect to this server
     # (note that repmgr will execute `pg_basebackup` in WAL streaming mode,
@@ -294,7 +293,13 @@ The following replication settings may need to be adjusted:
     max_wal_senders = 10
 
     # Ensure WAL files contain enough information to enable read-only queries
-    # on the standby
+    # on the standby.
+    #
+    #  PostgreSQL 9.5 and earlier: one of 'hot_standby' or 'logical'
+    #  PostgreSQL 9.6 and later: one of 'replica' or 'logical'
+    #    ('hot_standby' will still be accepted as an alias for 'replica')
+    #
+    # See: https://www.postgresql.org/docs/current/static/runtime-config-wal.html#GUC-WAL-LEVEL
 
     wal_level = 'hot_standby'
 
@@ -1085,9 +1090,13 @@ primary; execute:
     repmgr -f /etc./repmgr.conf node service --list --action=start
     repmgr -f /etc./repmgr.conf node service --list --action=restart
 
-If the `pg_ctl` command is used for starting/restarting the server, you *must*
-ensure that logging output is redirected to a file, otherwise the switchover
-will appear to "hang". See note in section `Caveats` below.
+* * *
+
+> *NOTE* on systemd systems we strongly recommend using the appropriate
+> `systemctl` commands (typically run via `sudo`) to ensure systemd is
+> informed about the status of the PostgreSQL service.
+
+* * *
 
 Check that access from applications is minimalized or preferably blocked
 completely, so applications are not unexpectedly interrupted.
diff --git a/repmgr-action-cluster.c b/repmgr-action-cluster.c
index 19377adb..b301e69b 100644
--- a/repmgr-action-cluster.c
+++ b/repmgr-action-cluster.c
@@ -344,7 +344,7 @@ do_cluster_show(void)
 	{
 		if (runtime_options.output_mode == OM_CSV)
 		{
-			int connection_status =	(PQstatus(conn) == CONNECTION_OK) ? 0 : -1;
+			int connection_status =	(cell->node_info->node_status == NODE_STATUS_UP) ? 0 : -1;
 			int recovery_type = RECTYPE_UNKNOWN;
 
 			/*
diff --git a/repmgr-action-standby.c b/repmgr-action-standby.c
index d57077b2..4eebedea 100644
--- a/repmgr-action-standby.c
+++ b/repmgr-action-standby.c
@@ -494,8 +494,8 @@ do_standby_clone(void)
 		}
 
 		log_error(_("unable to take a base backup of the primary server"));
-		log_warning(_("data directory (%s) may need to be cleaned up manually"),
-					local_data_directory);
+		log_hint(_("data directory (\"%s\") may need to be cleaned up manually"),
+				 local_data_directory);
 
 		PQfinish(source_conn);
 		exit(r);
@@ -517,7 +517,16 @@ do_standby_clone(void)
 
 	/* Write the recovery.conf file */
 
-	create_recovery_file(&node_record, &recovery_conninfo, local_data_directory);
+	if (create_recovery_file(&node_record, &recovery_conninfo, local_data_directory) == false)
+	{
+		/* create_recovery_file() will log an error */
+		log_notice(_("unable to create recovery.conf; see preceding error messages"));
+		log_hint(_("data directory (\"%s\") may need to be cleaned up manually"),
+				 local_data_directory);
+
+		PQfinish(source_conn);
+		exit(ERR_BAD_CONFIG);
+	}
 
 	switch(mode)
 	{
diff --git a/repmgr-client.c b/repmgr-client.c
index d944d78b..c42e06f6 100644
--- a/repmgr-client.c
+++ b/repmgr-client.c
@@ -2199,6 +2199,8 @@ create_recovery_file(t_node_info *node_record, t_conninfo_param_list *recovery_c
 	{
 		log_error(_("unable to create recovery.conf file at \"%s\""),
 				  recovery_file_path);
+		log_detail("%s", strerror(errno));
+
 		return false;
 	}
 
@@ -2430,17 +2432,12 @@ remote_command(const char *host, const char *user, const char *command, PQExpBuf
 	}
 	else
 	{
-		/*
-		 * When executed remotely, repmgr commands which execute pg_ctl (particularly
-		 * `repmgr standby follow`) will see the pg_ctl command appear to fail with a
-		 * non-zero return code when the output from the executed pg_ctl command
-		 * has nowhere to go, even though the command actually succeeds. We'll consume an
-		 * arbitrary amount of output and throw it away to work around this.
-		 */
-		int i = 0;
-		while (fgets(output, MAXLEN, fp) != NULL && i < 10)
+		while (fgets(output, MAXLEN, fp) != NULL)
 		{
-			i++;
+			if (!feof(fp))
+			{
+				break;
+			}
 		}
 	}