Update HISTORY

Sometimes we're setting node status to active.

FAQ.md

@@ -137,6 +137,7 @@ General
   of events which includes servers removed from the replication cluster
   which no longer have an entry in the `repl_nodes` table.
 
+
 `repmgrd`
 ---------
 
@@ -151,6 +152,9 @@ General
 
   In `repmgr.conf`, set its priority to a value of 0 or less.
 
+  Additionally, if `failover` is set to `manual`, the node will never
+  be considered as a promotion candidate.
+
 - Does `repmgrd` support delayed standbys?
 
   `repmgrd` can monitor delayed standbys - those set up with
@@ -169,3 +173,11 @@ General
 
   Configure your system's `logrotate` service to do this; see example
   in README.md
+
+- I've recloned a failed master as a standby, but `repmgrd` refuses to start?
+
+  Check you registered the standby after recloning. If unregistered the standby
+  cannot be considered as a promotion candidate even if `failover` is set to
+  `automatic`, which is probably not what you want. `repmgrd` will start if
+  `failover` is set to `manual` so the node's replication status can still
+  be monitored, if desired.

HISTORY

@@ -1,4 +1,68 @@
-3.1.4   2016-07-
+3.3     2016-12-27
+        repmgr: always log to STDERR even if log facility defined (Ian)
+        repmgr: add --log-to-file to log repmgr output to the defined
+          log facility (Ian)
+        repmgr: improve handling of command line parameter errors (Ian)
+        repmgr: add option --upstream-conninfo to explicitly set
+          'primary_conninfo' in recovery.conf (Ian)
+        repmgr: enable a standby to be registered which isn't running (Ian)
+        repmgr: enable `standby register --force` to update a node record
+          with cascaded downstream node records (Ian)
+        repmgr: add option `--no-conninfo-password` (Abhijit, Ian)
+        repmgr: add initial support for PostgreSQL 10.0 (Ian)
+        repmgr: escape values in primary_conninfo if needed (Ian)
+
+3.2.1   2016-10-24
+        repmgr: require a valid repmgr cluster name unless -F/--force
+          supplied (Ian)
+        repmgr: check master server is registered with repmgr before
+          cloning (Ian)
+        repmgr: ensure data directory defaults to that of the source node (Ian)
+        repmgr: various fixes to Barman cloning mode (Gianni, Ian)
+        repmgr: fix `repmgr cluster crosscheck` output (Ian)
+
+3.2     2016-10-05
+        repmgr: add support for cloning from a Barman backup (Gianni)
+        repmgr: add commands `standby matrix` and `standby crosscheck` (Gianni)
+        repmgr: suppress connection error display in `repmgr cluster show`
+          unless `--verbose` supplied (Ian)
+        repmgr: add commands `witness register` and `witness unregister` (Ian)
+        repmgr: enable `standby unregister` / `witness unregister` to be
+          executed for a node which is not running (Ian)
+        repmgr: remove deprecated command line options --initdb-no-pwprompt and
+           -l/--local-port (Ian)
+        repmgr: before cloning with pg_basebackup, check that sufficient free
+           walsenders are available (Ian)
+        repmgr: add option `--wait-sync` for `standby register` which causes
+           repmgr to wait for the registered node record to synchronise to
+           the standby (Ian)
+        repmgr: add option `--copy-external-config-files` for files outside
+           of the data directory (Ian)
+        repmgr: only require `wal_keep_segments` to be set in certain corner
+           cases (Ian)
+        repmgr: better support cloning from a node other than the one to
+           stream from (Ian)
+        repmgrd: add configuration options to override the default pg_ctl
+           commands (Jarkko Oranen, Ian)
+        repmgrd: don't start if node is inactive and failover=automatic (Ian)
+        packaging: improve "repmgr-auto" Debian package (Gianni)
+
+
+3.1.5   2016-08-15
+        repmgrd: in a failover situation, prevent endless looping when
+          attempting to establish the status of a node with
+          `failover=manual` (Ian)
+        repmgrd: improve handling of failover events on standbys with
+          `failover=manual`, and create a new event notification
+          for this, `standby_disconnect_manual` (Ian)
+        repmgr: add further event notifications (Gianni)
+        repmgr: when executing `standby switchover`, don't collect remote
+          command output unless required (Gianni, Ian)
+        repmgrd: improve standby monitoring query (Ian, based on suggestion
+          from  Álvaro)
+        repmgr: various command line handling improvements (Ian)
+
+3.1.4   2016-07-12
         repmgr: new configuration option for setting "restore_command"
           in the recovery.conf file generated by repmgr (Martín)
         repmgr: add --csv option to "repmgr cluster show" (Gianni)

Makefile

@@ -5,7 +5,7 @@
 HEADERS = $(wildcard *.h)
 
 repmgrd_OBJS = dbutils.o config.o repmgrd.o log.o strutil.o
-repmgr_OBJS = dbutils.o check_dir.o config.o repmgr.o log.o strutil.o dirmod.o
+repmgr_OBJS = dbutils.o check_dir.o config.o repmgr.o log.o strutil.o dirmod.o compat.o
 
 DATA = repmgr.sql uninstall_repmgr.sql
 
@@ -87,10 +87,12 @@ PG_VERSION = $(shell pg_config --version | cut -d ' ' -f 2 | cut -d '.' -f 1,2)
 REPMGR_VERSION = $(shell grep REPMGR_VERSION version.h | cut -d ' ' -f 3 | cut -d '"' -f 2)
 PKGLIBDIR = $(shell pg_config --pkglibdir)
 SHAREDIR = $(shell pg_config --sharedir)
+PGBINDIR = /usr/lib/postgresql/$(PG_VERSION)/bin
 
 deb: repmgrd repmgr
-	mkdir -p ./debian/usr/bin
+	mkdir -p ./debian/usr/bin ./debian$(PGBINDIR)
-	cp repmgrd repmgr ./debian/usr/bin/
+	cp repmgrd repmgr ./debian$(PGBINDIR)
+	ln -s ../..$(PGBINDIR)/repmgr ./debian/usr/bin/repmgr
 	mkdir -p ./debian$(SHAREDIR)/contrib/
 	cp sql/repmgr_funcs.sql ./debian$(SHAREDIR)/contrib/
 	cp sql/uninstall_repmgr_funcs.sql ./debian$(SHAREDIR)/contrib/

compat.c

@@ -0,0 +1,111 @@
+/*
+ *
+ * compat.c
+ *	  Provide backports of various functions not publicly
+ *    exposed before PostgreSQL 9.6
+ *
+ * Copyright (C) 2ndQuadrant, 2010-2016
+ *
+ * Portions Copyright (c) 1996-2013, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#if (PG_VERSION_NUM < 90600)
+
+#include "repmgr.h"
+#include "compat.h"
+
+/*
+ * Append the given string to the buffer, with suitable quoting for passing
+ * the string as a value, in a keyword/pair value in a libpq connection
+ * string
+ *
+ * This function is copied from src/bin/pg_dump/dumputils.c
+ * as it is only publicly exposed from 9.6
+ */
+void
+appendConnStrVal(PQExpBuffer buf, const char *str)
+{
+	const char *s;
+	bool		needquotes;
+
+	/*
+	 * If the string is one or more plain ASCII characters, no need to quote
+	 * it. This is quite conservative, but better safe than sorry.
+	 */
+	needquotes = true;
+	for (s = str; *s; s++)
+	{
+		if (!((*s >= 'a' && *s <= 'z') || (*s >= 'A' && *s <= 'Z') ||
+			  (*s >= '0' && *s <= '9') || *s == '_' || *s == '.'))
+		{
+			needquotes = true;
+			break;
+		}
+		needquotes = false;
+	}
+
+	if (needquotes)
+	{
+		appendPQExpBufferChar(buf, '\'');
+		while (*str)
+		{
+			/* ' and \ must be escaped by to \' and \\ */
+			if (*str == '\'' || *str == '\\')
+				appendPQExpBufferChar(buf, '\\');
+
+			appendPQExpBufferChar(buf, *str);
+			str++;
+		}
+		appendPQExpBufferChar(buf, '\'');
+	}
+	else
+		appendPQExpBufferStr(buf, str);
+}
+
+/*
+ * Adapted from: src/fe_utils/string_utils.c
+ *
+ * Function not publicly available before PostgreSQL 9.6.
+ */
+void
+appendShellString(PQExpBuffer buf, const char *str)
+{
+	const char *p;
+
+	appendPQExpBufferChar(buf, '\'');
+	for (p = str; *p; p++)
+	{
+		if (*p == '\n' || *p == '\r')
+		{
+			fprintf(stderr,
+					_("shell command argument contains a newline or carriage return: \"%s\"\n"),
+					str);
+			exit(ERR_BAD_CONFIG);
+		}
+
+		if (*p == '\'')
+			appendPQExpBufferStr(buf, "'\"'\"'");
+		else
+			appendPQExpBufferChar(buf, *p);
+	}
+
+	appendPQExpBufferChar(buf, '\'');
+}
+
+
+#endif

compat.h

@@ -0,0 +1,29 @@
+/*
+ * compat.h
+ * Copyright (c) 2ndQuadrant, 2010-2016
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef _COMPAT_H_
+#define _COMPAT_H_
+
+extern void
+appendConnStrVal(PQExpBuffer buf, const char *str);
+
+extern void
+appendShellString(PQExpBuffer buf, const char *str);
+
+#endif

config.c

@@ -1,5 +1,6 @@
 /*
  * config.c - Functions to parse the config file
+ *
  * Copyright (C) 2ndQuadrant, 2010-2016
  *
  * This program is free software: you can redistribute it and/or modify
@@ -9,11 +10,11 @@
  *
  * This program is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ * along with this program.	 If not, see <http://www.gnu.org/licenses/>.
  *
  */
 
@@ -26,7 +27,7 @@
 
 static void parse_event_notifications_list(t_configuration_options *options, const char *arg);
 static void tablespace_list_append(t_configuration_options *options, const char *arg);
-static void exit_with_errors(ErrorList *config_errors);
+static void exit_with_errors(ItemList *config_errors);
 
 const static char *_progname = NULL;
 static char config_file_path[MAXPGPATH];
@@ -54,8 +55,8 @@ progname(void)
  *
  * Returns true if a configuration file could be parsed, otherwise false.
  *
- * Any configuration options changed in this function must also be changed in
+ * Any *repmgrd-specific* configuration options added/changed in this function must also be
- * reload_config()
+ * added/changed in reload_config()
  *
  * NOTE: this function is called before the logger is set up, so we need
  * to handle the verbose option ourselves; also the default log level is NOTICE,
@@ -98,9 +99,9 @@ load_config(const char *config_file, bool verbose, t_configuration_options *opti
 	/*
 	 * If no configuration file was provided, attempt to find a default file
 	 * in this order:
-	 *  - current directory
+	 *	- current directory
-	 *  - /etc/repmgr.conf
+	 *	- /etc/repmgr.conf
-	 *  - default sysconfdir
+	 *	- default sysconfdir
 	 *
 	 * here we just check for the existence of the file; parse_config()
 	 * will handle read errors etc.
@@ -180,6 +181,23 @@ load_config(const char *config_file, bool verbose, t_configuration_options *opti
 }
 
 
+bool
+parse_config(t_configuration_options *options)
+{
+	/* Collate configuration file errors here for friendlier reporting */
+	static ItemList config_errors = { NULL, NULL };
+
+	_parse_config(options, &config_errors);
+
+	if (config_errors.head != NULL)
+	{
+		exit_with_errors(&config_errors);
+	}
+
+	return true;
+}
+
+
 /*
  * Parse configuration file; if any errors are encountered,
  * list them and exit.
@@ -187,8 +205,8 @@ load_config(const char *config_file, bool verbose, t_configuration_options *opti
  * Ensure any default values set here are synced with repmgr.conf.sample
  * and any other documentation.
  */
-bool
+void
-parse_config(t_configuration_options *options)
+_parse_config(t_configuration_options *options, ItemList *error_list)
 {
 	FILE	   *fp;
 	char	   *s,
@@ -200,9 +218,6 @@ parse_config(t_configuration_options *options)
 	PQconninfoOption *conninfo_options;
 	char	   *conninfo_errmsg = NULL;
 
-	/* Collate configuration file errors here for friendlier reporting */
-	static ErrorList config_errors = { NULL, NULL };
-
 	bool		node_found = false;
 
 	/* Initialize configuration options with sensible defaults
@@ -210,15 +225,22 @@ parse_config(t_configuration_options *options)
 	 * to be initialised here
 	 */
 	memset(options->cluster_name, 0, sizeof(options->cluster_name));
-	options->node = -1;
+	options->node = UNKNOWN_NODE_ID;
 	options->upstream_node = NO_UPSTREAM_NODE;
 	options->use_replication_slots = 0;
 	memset(options->conninfo, 0, sizeof(options->conninfo));
+	memset(options->barman_server, 0, sizeof(options->barman_server));
+	memset(options->barman_config, 0, sizeof(options->barman_config));
 	options->failover = MANUAL_FAILOVER;
 	options->priority = DEFAULT_PRIORITY;
 	memset(options->node_name, 0, sizeof(options->node_name));
 	memset(options->promote_command, 0, sizeof(options->promote_command));
 	memset(options->follow_command, 0, sizeof(options->follow_command));
+	memset(options->service_stop_command, 0, sizeof(options->service_stop_command));
+	memset(options->service_start_command, 0, sizeof(options->service_start_command));
+	memset(options->service_restart_command, 0, sizeof(options->service_restart_command));
+	memset(options->service_reload_command, 0, sizeof(options->service_reload_command));
+	memset(options->service_promote_command, 0, sizeof(options->service_promote_command));
 	memset(options->rsync_options, 0, sizeof(options->rsync_options));
 	memset(options->ssh_options, 0, sizeof(options->ssh_options));
 	memset(options->pg_bindir, 0, sizeof(options->pg_bindir));
@@ -254,7 +276,7 @@ parse_config(t_configuration_options *options)
 	{
 		log_verbose(LOG_NOTICE, _("no configuration file provided and no default file found - "
 					 "continuing with default values\n"));
-		return true;
+		return;
 	}
 
 	fp = fopen(config_file_path, "r");
@@ -299,13 +321,17 @@ parse_config(t_configuration_options *options)
 			strncpy(options->cluster_name, value, MAXLEN);
 		else if (strcmp(name, "node") == 0)
 		{
-			options->node = repmgr_atoi(value, "node", &config_errors, false);
+			options->node = repmgr_atoi(value, "node", error_list, false);
 			node_found = true;
 		}
 		else if (strcmp(name, "upstream_node") == 0)
-			options->upstream_node = repmgr_atoi(value, "upstream_node", &config_errors, false);
+			options->upstream_node = repmgr_atoi(value, "upstream_node", error_list, false);
 		else if (strcmp(name, "conninfo") == 0)
 			strncpy(options->conninfo, value, MAXLEN);
+		else if (strcmp(name, "barman_server") == 0)
+			strncpy(options->barman_server, value, MAXLEN);
+		else if (strcmp(name, "barman_config") == 0)
+			strncpy(options->barman_config, value, MAXLEN);
 		else if (strcmp(name, "rsync_options") == 0)
 			strncpy(options->rsync_options, value, QUERY_STR_LEN);
 		else if (strcmp(name, "ssh_options") == 0)
@@ -330,29 +356,39 @@ parse_config(t_configuration_options *options)
 			}
 			else
 			{
-				error_list_append(&config_errors,_("value for 'failover' must be 'automatic' or 'manual'\n"));
+				item_list_append(error_list, _("value for 'failover' must be 'automatic' or 'manual'\n"));
 			}
 		}
 		else if (strcmp(name, "priority") == 0)
-			options->priority = repmgr_atoi(value, "priority", &config_errors, true);
+			options->priority = repmgr_atoi(value, "priority", error_list, true);
 		else if (strcmp(name, "node_name") == 0)
 			strncpy(options->node_name, value, MAXLEN);
 		else if (strcmp(name, "promote_command") == 0)
 			strncpy(options->promote_command, value, MAXLEN);
 		else if (strcmp(name, "follow_command") == 0)
 			strncpy(options->follow_command, value, MAXLEN);
+		else if (strcmp(name, "service_stop_command") == 0)
+			strncpy(options->service_stop_command, value, MAXLEN);
+		else if (strcmp(name, "service_start_command") == 0)
+			strncpy(options->service_start_command, value, MAXLEN);
+		else if (strcmp(name, "service_restart_command") == 0)
+			strncpy(options->service_restart_command, value, MAXLEN);
+		else if (strcmp(name, "service_reload_command") == 0)
+			strncpy(options->service_reload_command, value, MAXLEN);
+		else if (strcmp(name, "service_promote_command") == 0)
+			strncpy(options->service_promote_command, value, MAXLEN);
 		else if (strcmp(name, "master_response_timeout") == 0)
-			options->master_response_timeout = repmgr_atoi(value, "master_response_timeout", &config_errors, false);
+			options->master_response_timeout = repmgr_atoi(value, "master_response_timeout", error_list, false);
 		/*
 		 * 'primary_response_timeout' as synonym for 'master_response_timeout' -
 		 * we'll switch terminology in a future release (3.1?)
 		 */
 		else if (strcmp(name, "primary_response_timeout") == 0)
-			options->master_response_timeout = repmgr_atoi(value, "primary_response_timeout", &config_errors, false);
+			options->master_response_timeout = repmgr_atoi(value, "primary_response_timeout", error_list, false);
 		else if (strcmp(name, "reconnect_attempts") == 0)
-			options->reconnect_attempts = repmgr_atoi(value, "reconnect_attempts", &config_errors, false);
+			options->reconnect_attempts = repmgr_atoi(value, "reconnect_attempts", error_list, false);
 		else if (strcmp(name, "reconnect_interval") == 0)
-			options->reconnect_interval = repmgr_atoi(value, "reconnect_interval", &config_errors, false);
+			options->reconnect_interval = repmgr_atoi(value, "reconnect_interval", error_list, false);
 		else if (strcmp(name, "pg_bindir") == 0)
 			strncpy(options->pg_bindir, value, MAXLEN);
 		else if (strcmp(name, "pg_ctl_options") == 0)
@@ -362,14 +398,14 @@ parse_config(t_configuration_options *options)
 		else if (strcmp(name, "logfile") == 0)
 			strncpy(options->logfile, value, MAXLEN);
 		else if (strcmp(name, "monitor_interval_secs") == 0)
-			options->monitor_interval_secs = repmgr_atoi(value, "monitor_interval_secs", &config_errors, false);
+			options->monitor_interval_secs = repmgr_atoi(value, "monitor_interval_secs", error_list, false);
 		else if (strcmp(name, "retry_promote_interval_secs") == 0)
-			options->retry_promote_interval_secs = repmgr_atoi(value, "retry_promote_interval_secs", &config_errors, false);
+			options->retry_promote_interval_secs = repmgr_atoi(value, "retry_promote_interval_secs", error_list, false);
 		else if (strcmp(name, "witness_repl_nodes_sync_interval_secs") == 0)
-			options->witness_repl_nodes_sync_interval_secs = repmgr_atoi(value, "witness_repl_nodes_sync_interval_secs", &config_errors, false);
+			options->witness_repl_nodes_sync_interval_secs = repmgr_atoi(value, "witness_repl_nodes_sync_interval_secs", error_list, false);
 		else if (strcmp(name, "use_replication_slots") == 0)
 			/* XXX we should have a dedicated boolean argument format */
-			options->use_replication_slots = repmgr_atoi(value, "use_replication_slots", &config_errors, false);
+			options->use_replication_slots = repmgr_atoi(value, "use_replication_slots", error_list, false);
 		else if (strcmp(name, "event_notification_command") == 0)
 			strncpy(options->event_notification_command, value, MAXLEN);
 		else if (strcmp(name, "event_notifications") == 0)
@@ -397,7 +433,7 @@ parse_config(t_configuration_options *options)
 					 _("no value provided for parameter \"%s\""),
 					 name);
 
-			error_list_append(&config_errors, error_message_buf);
+			item_list_append(error_list, error_message_buf);
 		}
 	}
 
@@ -406,11 +442,11 @@ parse_config(t_configuration_options *options)
 
 	if (node_found == false)
 	{
-		error_list_append(&config_errors, _("\"node\": parameter was not found"));
+		item_list_append(error_list, _("\"node\": parameter was not found"));
 	}
 	else if (options->node == 0)
 	{
-		error_list_append(&config_errors, _("\"node\": must be greater than zero"));
+		item_list_append(error_list, _("\"node\": must be greater than zero"));
 	}
 
 	if (strlen(options->conninfo))
@@ -430,18 +466,11 @@ parse_config(t_configuration_options *options)
 					 _("\"conninfo\": %s"),
 					 conninfo_errmsg);
 
-			error_list_append(&config_errors, error_message_buf);
+			item_list_append(error_list, error_message_buf);
 		}
 
 		PQconninfoFree(conninfo_options);
 	}
-
-	if (config_errors.head != NULL)
-	{
-		exit_with_errors(&config_errors);
-	}
-
-	return true;
 }
 
 
@@ -531,70 +560,85 @@ parse_line(char *buf, char *name, char *value)
 	trim(value);
 }
 
+
+/*
+ * reload_config()
+ *
+ * This is only called by repmgrd after receiving a SIGHUP or when a monitoring
+ * loop is started up; it therefore only needs to reload options required
+ * by repmgrd, which are as follows:
+ *
+ * changeable options:
+ * - failover
+ * - follow_command
+ * - logfacility
+ * - logfile
+ * - loglevel
+ * - master_response_timeout
+ * - monitor_interval_secs
+ * - priority
+ * - promote_command
+ * - reconnect_attempts
+ * - reconnect_interval
+ * - retry_promote_interval_secs
+ * - witness_repl_nodes_sync_interval_secs
+ *
+ * non-changeable options:
+ * - cluster_name
+ * - conninfo
+ * - node
+ * - node_name
+ *
+ * extract with something like:
+ *	 grep local_options\\. repmgrd.c | perl -n -e '/local_options\.([\w_]+)/ && print qq|$1\n|;' | sort | uniq
+
+ */
 bool
 reload_config(t_configuration_options *orig_options)
 {
 	PGconn	   *conn;
-	t_configuration_options new_options;
+	t_configuration_options new_options = T_CONFIGURATION_OPTIONS_INITIALIZER;
 	bool	  config_changed = false;
+	bool	  log_config_changed = false;
+
+	static ItemList config_errors = { NULL, NULL };
 
 	/*
 	 * Re-read the configuration file: repmgr.conf
 	 */
-	log_info(_("reloading configuration file and updating repmgr tables\n"));
+	log_info(_("reloading configuration file\n"));
 
-	parse_config(&new_options);
+	_parse_config(&new_options, &config_errors);
-	if (new_options.node == -1)
+
+	if (config_errors.head != NULL)
 	{
+		/* XXX dump errors to log */
 		log_warning(_("unable to parse new configuration, retaining current configuration\n"));
 		return false;
 	}
 
+	/* The following options cannot be changed */
 	if (strcmp(new_options.cluster_name, orig_options->cluster_name) != 0)
 	{
-		log_warning(_("unable to change cluster name, retaining current configuration\n"));
+		log_warning(_("cluster_name cannot be changed, retaining current configuration\n"));
 		return false;
 	}
 
 	if (new_options.node != orig_options->node)
 	{
-		log_warning(_("unable to change node ID, retaining current configuration\n"));
+		log_warning(_("node ID cannot be changed, retaining current configuration\n"));
 		return false;
 	}
 
 	if (strcmp(new_options.node_name, orig_options->node_name) != 0)
 	{
-		log_warning(_("unable to change standby name, keeping current configuration\n"));
+		log_warning(_("node_name cannot be changed, keeping current configuration\n"));
-		return false;
-	}
-
-	if (new_options.failover != MANUAL_FAILOVER && new_options.failover != AUTOMATIC_FAILOVER)
-	{
-		log_warning(_("new value for 'failover' must be 'automatic' or 'manual'\n"));
-		return false;
-	}
-
-	if (new_options.master_response_timeout <= 0)
-	{
-		log_warning(_("new value for 'master_response_timeout' must be greater than zero\n"));
-		return false;
-	}
-
-	if (new_options.reconnect_attempts < 0)
-	{
-		log_warning(_("new value for 'reconnect_attempts' must be zero or greater\n"));
-		return false;
-	}
-
-	if (new_options.reconnect_interval < 0)
-	{
-		log_warning(_("new value for 'reconnect_interval' must be zero or greater\n"));
 		return false;
 	}
 
 	if (strcmp(orig_options->conninfo, new_options.conninfo) != 0)
 	{
-		/* Test conninfo string */
+		/* Test conninfo string works*/
 		conn = establish_db_connection(new_options.conninfo, false);
 		if (!conn || (PQstatus(conn) != CONNECTION_OK))
 		{
@@ -611,27 +655,6 @@ reload_config(t_configuration_options *orig_options)
 	 * to manage them
 	 */
 
-	/* cluster_name */
-	if (strcmp(orig_options->cluster_name, new_options.cluster_name) != 0)
-	{
-		strcpy(orig_options->cluster_name, new_options.cluster_name);
-		config_changed = true;
-	}
-
-	/* conninfo */
-	if (strcmp(orig_options->conninfo, new_options.conninfo) != 0)
-	{
-		strcpy(orig_options->conninfo, new_options.conninfo);
-		config_changed = true;
-	}
-
-	/* node */
-	if (orig_options->node != new_options.node)
-	{
-		orig_options->node = new_options.node;
-		config_changed = true;
-	}
-
 	/* failover */
 	if (orig_options->failover != new_options.failover)
 	{
@@ -639,27 +662,6 @@ reload_config(t_configuration_options *orig_options)
 		config_changed = true;
 	}
 
-	/* priority */
-	if (orig_options->priority != new_options.priority)
-	{
-		orig_options->priority = new_options.priority;
-		config_changed = true;
-	}
-
-	/* node_name */
-	if (strcmp(orig_options->node_name, new_options.node_name) != 0)
-	{
-		strcpy(orig_options->node_name, new_options.node_name);
-		config_changed = true;
-	}
-
-	/* promote_command */
-	if (strcmp(orig_options->promote_command, new_options.promote_command) != 0)
-	{
-		strcpy(orig_options->promote_command, new_options.promote_command);
-		config_changed = true;
-	}
-
 	/* follow_command */
 	if (strcmp(orig_options->follow_command, new_options.follow_command) != 0)
 	{
@@ -667,30 +669,6 @@ reload_config(t_configuration_options *orig_options)
 		config_changed = true;
 	}
 
-	/*
-	 * XXX These ones can change with a simple SIGHUP?
-	 *
-	 * strcpy (orig_options->loglevel, new_options.loglevel); strcpy
-	 * (orig_options->logfacility, new_options.logfacility);
-	 *
-	 * logger_shutdown(); XXX do we have progname here ? logger_init(progname,
-	 * orig_options.loglevel, orig_options.logfacility);
-	 */
-
-	/* rsync_options */
-	if (strcmp(orig_options->rsync_options, new_options.rsync_options) != 0)
-	{
-		strcpy(orig_options->rsync_options, new_options.rsync_options);
-		config_changed = true;
-	}
-
-	/* ssh_options */
-	if (strcmp(orig_options->ssh_options, new_options.ssh_options) != 0)
-	{
-		strcpy(orig_options->ssh_options, new_options.ssh_options);
-		config_changed = true;
-	}
-
 	/* master_response_timeout */
 	if (orig_options->master_response_timeout != new_options.master_response_timeout)
 	{
@@ -698,6 +676,27 @@ reload_config(t_configuration_options *orig_options)
 		config_changed = true;
 	}
 
+	/* monitor_interval_secs */
+	if (orig_options->monitor_interval_secs != new_options.monitor_interval_secs)
+	{
+		orig_options->monitor_interval_secs = new_options.monitor_interval_secs;
+		config_changed = true;
+	}
+
+	/* priority */
+	if (orig_options->priority != new_options.priority)
+	{
+		orig_options->priority = new_options.priority;
+		config_changed = true;
+	}
+
+	/* promote_command */
+	if (strcmp(orig_options->promote_command, new_options.promote_command) != 0)
+	{
+		strcpy(orig_options->promote_command, new_options.promote_command);
+		config_changed = true;
+	}
+
 	/* reconnect_attempts */
 	if (orig_options->reconnect_attempts != new_options.reconnect_attempts)
 	{
@@ -712,27 +711,6 @@ reload_config(t_configuration_options *orig_options)
 		config_changed = true;
 	}
 
-	/* pg_ctl_options */
-	if (strcmp(orig_options->pg_ctl_options, new_options.pg_ctl_options) != 0)
-	{
-		strcpy(orig_options->pg_ctl_options, new_options.pg_ctl_options);
-		config_changed = true;
-	}
-
-	/* pg_basebackup_options */
-	if (strcmp(orig_options->pg_basebackup_options, new_options.pg_basebackup_options) != 0)
-	{
-		strcpy(orig_options->pg_basebackup_options, new_options.pg_basebackup_options);
-		config_changed = true;
-	}
-
-	/* monitor_interval_secs */
-	if (orig_options->monitor_interval_secs != new_options.monitor_interval_secs)
-	{
-		orig_options->monitor_interval_secs = new_options.monitor_interval_secs;
-		config_changed = true;
-	}
-
 	/* retry_promote_interval_secs */
 	if (orig_options->retry_promote_interval_secs != new_options.retry_promote_interval_secs)
 	{
@@ -740,20 +718,54 @@ reload_config(t_configuration_options *orig_options)
 		config_changed = true;
 	}
 
-	/* use_replication_slots */
+
-	if (orig_options->use_replication_slots != new_options.use_replication_slots)
+	/* witness_repl_nodes_sync_interval_secs */
+	if (orig_options->witness_repl_nodes_sync_interval_secs != new_options.witness_repl_nodes_sync_interval_secs)
 	{
-		orig_options->use_replication_slots = new_options.use_replication_slots;
+		orig_options->witness_repl_nodes_sync_interval_secs = new_options.witness_repl_nodes_sync_interval_secs;
 		config_changed = true;
 	}
 
+	/*
+	 * Handle changes to logging configuration
+	 */
+	if (strcmp(orig_options->logfacility, new_options.logfacility) != 0)
+	{
+		strcpy(orig_options->logfacility, new_options.logfacility);
+		log_config_changed = true;
+	}
+
+	if (strcmp(orig_options->logfile, new_options.logfile) != 0)
+	{
+		strcpy(orig_options->logfile, new_options.logfile);
+		log_config_changed = true;
+	}
+
+
+	if (strcmp(orig_options->loglevel, new_options.loglevel) != 0)
+	{
+		strcpy(orig_options->loglevel, new_options.loglevel);
+		log_config_changed = true;
+	}
+
+	if (log_config_changed == true)
+	{
+		log_notice(_("restarting logging with changed parameters\n"));
+		logger_shutdown();
+		logger_init(orig_options, progname());
+	}
+
 	if (config_changed == true)
 	{
-		log_debug(_("reload_config(): configuration has changed\n"));
+		log_notice(_("configuration file reloaded with changed parameters\n"));
 	}
-	else
+	/*
+	 * if logging configuration changed, don't say the configuration didn't
+	 * change, as it clearly has.
+	 */
+	else if (log_config_changed == false)
 	{
-		log_debug(_("reload_config(): configuration has not changed\n"));
+		log_info(_("configuration has not changed\n"));
 	}
 
 	return config_changed;
@@ -761,11 +773,11 @@ reload_config(t_configuration_options *orig_options)
 
 
 void
-error_list_append(ErrorList *error_list, char *error_message)
+item_list_append(ItemList *item_list, char *error_message)
 {
-	ErrorListCell *cell;
+	ItemListCell *cell;
 
-	cell = (ErrorListCell *) pg_malloc0(sizeof(ErrorListCell));
+	cell = (ItemListCell *) pg_malloc0(sizeof(ItemListCell));
 
 	if (cell == NULL)
 	{
@@ -773,19 +785,19 @@ error_list_append(ErrorList *error_list, char *error_message)
 		exit(ERR_BAD_CONFIG);
 	}
 
-	cell->error_message = pg_malloc0(MAXLEN);
+	cell->string = pg_malloc0(MAXLEN);
-	strncpy(cell->error_message, error_message, MAXLEN);
+	strncpy(cell->string, error_message, MAXLEN);
 
-	if (error_list->tail)
+	if (item_list->tail)
 	{
-		error_list->tail->next = cell;
+		item_list->tail->next = cell;
 	}
 	else
 	{
-		error_list->head = cell;
+		item_list->head = cell;
 	}
 
-	error_list->tail = cell;
+	item_list->tail = cell;
 }
 
 
@@ -795,7 +807,7 @@ error_list_append(ErrorList *error_list, char *error_message)
  * otherwise exit
  */
 int
-repmgr_atoi(const char *value, const char *config_item, ErrorList *error_list, bool allow_negative)
+repmgr_atoi(const char *value, const char *config_item, ItemList *error_list, bool allow_negative)
 {
 	char	  *endptr;
 	long	   longval = 0;
@@ -844,7 +856,7 @@ repmgr_atoi(const char *value, const char *config_item, ErrorList *error_list, b
 			exit(ERR_BAD_CONFIG);
 		}
 
-		error_list_append(error_list, error_message_buf);
+		item_list_append(error_list, error_message_buf);
 	}
 
 	return (int32) longval;
@@ -927,7 +939,7 @@ static void
 parse_event_notifications_list(t_configuration_options *options, const char *arg)
 {
 	const char *arg_ptr;
-	char	    event_type_buf[MAXLEN] = "";
+	char		event_type_buf[MAXLEN] = "";
 	char	   *dst_ptr = event_type_buf;
 
 
@@ -986,15 +998,15 @@ parse_event_notifications_list(t_configuration_options *options, const char *arg
 
 
 static void
-exit_with_errors(ErrorList *config_errors)
+exit_with_errors(ItemList *config_errors)
 {
-	ErrorListCell *cell;
+	ItemListCell *cell;
 
 	log_err(_("%s: following errors were found in the configuration file.\n"), progname());
 
 	for (cell = config_errors->head; cell; cell = cell->next)
 	{
-		log_err("%s\n", cell->error_message);
+		log_err("%s\n", cell->string);
 	}
 
 	exit(ERR_BAD_CONFIG);

config.h

@@ -1,5 +1,6 @@
 /*
  * config.h
+ *
  * Copyright (c) 2ndQuadrant, 2010-2016
  *
  * This program is free software: you can redistribute it and/or modify
@@ -57,11 +58,20 @@ typedef struct
 	int			node;
 	int         upstream_node;
 	char		conninfo[MAXLEN];
+	char		barman_server[MAXLEN];
+	char		barman_config[MAXLEN];
 	int			failover;
 	int			priority;
 	char		node_name[MAXLEN];
+	/* commands executed by repmgrd */
 	char		promote_command[MAXLEN];
 	char		follow_command[MAXLEN];
+	/* Overrides for pg_ctl commands */
+	char		service_stop_command[MAXLEN];
+	char		service_start_command[MAXLEN];
+	char		service_restart_command[MAXLEN];
+	char		service_reload_command[MAXLEN];
+	char		service_promote_command[MAXLEN];
 	char		loglevel[MAXLEN];
 	char		logfacility[MAXLEN];
 	char		rsync_options[QUERY_STR_LEN];
@@ -87,32 +97,51 @@ typedef struct
  * The following will initialize the structure with a minimal set of options;
  * actual defaults are set in parse_config() before parsing the configuration file
  */
-#define T_CONFIGURATION_OPTIONS_INITIALIZER { "", -1, NO_UPSTREAM_NODE, "", MANUAL_FAILOVER, -1, "", "", "", "", "", "", "", -1, -1, -1, "", "", "", "", "", 0, 0, 0, 0, "", { NULL, NULL }, { NULL, NULL } }
+#define T_CONFIGURATION_OPTIONS_INITIALIZER { "", UNKNOWN_NODE_ID, NO_UPSTREAM_NODE, "", "", "", MANUAL_FAILOVER, -1, "", "", "", "", "", "", "", "", "", "", "", "", -1, -1, -1, "", "", "", "", "", 0, 0, 0, 0, "", { NULL, NULL }, { NULL, NULL } }
 
-typedef struct ErrorListCell
+typedef struct ItemListCell
 {
-	struct ErrorListCell *next;
+	struct ItemListCell *next;
-	char			     *error_message;
+	char			    *string;
-} ErrorListCell;
+} ItemListCell;
 
-typedef struct ErrorList
+typedef struct ItemList
 {
-	ErrorListCell *head;
+	ItemListCell *head;
-	ErrorListCell *tail;
+	ItemListCell *tail;
-} ErrorList;
+} ItemList;
+
+typedef struct TablespaceDataListCell
+{
+	struct TablespaceDataListCell *next;
+	char	   *name;
+	char	   *oid;
+	char	   *location;
+	/* optional payload */
+	FILE       *f;
+} TablespaceDataListCell;
+
+typedef struct TablespaceDataList
+{
+	TablespaceDataListCell *head;
+	TablespaceDataListCell *tail;
+} TablespaceDataList;
 
 void set_progname(const char *argv0);
 const char * progname(void);
 
 bool		load_config(const char *config_file, bool verbose, t_configuration_options *options, char *argv0);
-bool		reload_config(t_configuration_options *orig_options);
+
+void		_parse_config(t_configuration_options *options, ItemList *error_list);
 bool		parse_config(t_configuration_options *options);
+bool		reload_config(t_configuration_options *orig_options);
+
 void		parse_line(char *buff, char *name, char *value);
 char	   *trim(char *s);
-void		error_list_append(ErrorList *error_list, char *error_message);
+void		item_list_append(ItemList *item_list, char *error_message);
 int			repmgr_atoi(const char *s,
 						const char *config_item,
-						ErrorList *error_list,
+						ItemList *error_list,
 						bool allow_negative);
-
+extern bool		config_file_found;
 #endif

dbutils.c

@@ -1,5 +1,6 @@
 /*
  * dbutils.c - Database connection/management functions
+ *
  * Copyright (C) 2ndQuadrant, 2010-2016
  *
  * This program is free software: you can redistribute it and/or modify
@@ -34,7 +35,7 @@ char repmgr_schema_quoted[MAXLEN] = "";
 static int _get_node_record(PGconn *conn, char *cluster, char *sqlquery, t_node_info *node_info);
 
 PGconn *
-_establish_db_connection(const char *conninfo, const bool exit_on_error, const bool log_notice)
+_establish_db_connection(const char *conninfo, const bool exit_on_error, const bool log_notice, const bool verbose_only)
 {
 	/* Make a connection to the database */
 	PGconn	   *conn = NULL;
@@ -50,15 +51,23 @@ _establish_db_connection(const char *conninfo, const bool exit_on_error, const b
 	/* Check to see that the backend connection was successfully made */
 	if ((PQstatus(conn) != CONNECTION_OK))
 	{
-		if (log_notice)
+		bool emit_log = true;
+
+		if (verbose_only == true && verbose_logging == false)
+			emit_log = false;
+
+		if (emit_log)
 		{
-			log_notice(_("connection to database failed: %s\n"),
+			if (log_notice)
-					PQerrorMessage(conn));
+			{
-		}
+				log_notice(_("connection to database failed: %s\n"),
-		else
+						   PQerrorMessage(conn));
-		{
+			}
-			log_err(_("connection to database failed: %s\n"),
+			else
-					PQerrorMessage(conn));
+			{
+				log_err(_("connection to database failed: %s\n"),
+						PQerrorMessage(conn));
+			}
 		}
 
 		if (exit_on_error)
@@ -71,16 +80,35 @@ _establish_db_connection(const char *conninfo, const bool exit_on_error, const b
 	return conn;
 }
 
+
+/*
+ * Establish a database connection, optionally exit on error
+ */
 PGconn *
 establish_db_connection(const char *conninfo, const bool exit_on_error)
 {
-	return _establish_db_connection(conninfo, exit_on_error, false);
+	return _establish_db_connection(conninfo, exit_on_error, false, false);
 }
 
+/*
+ * Attempt to establish a database connection, never exit on error, only
+ * output error messages if --verbose option used
+ */
 PGconn *
-test_db_connection(const char *conninfo, const bool exit_on_error)
+establish_db_connection_quiet(const char *conninfo)
 {
-	return _establish_db_connection(conninfo, exit_on_error, true);
+	return _establish_db_connection(conninfo, false, false, true);
+}
+
+/*
+ * Attempt to establish a database connection, never exit on error,
+ * output connection error messages as NOTICE (useful when connection
+ * failure is expected)
+ */
+PGconn *
+test_db_connection(const char *conninfo)
+{
+	return _establish_db_connection(conninfo, false, true, false);
 }
 
 
@@ -186,7 +214,7 @@ check_cluster_schema(PGconn *conn)
 	char		sqlquery[QUERY_STR_LEN];
 
 	sqlquery_snprintf(sqlquery,
-					  "SELECT 1 FROM pg_namespace WHERE nspname = '%s'",
+					  "SELECT 1 FROM pg_catalog.pg_namespace WHERE nspname = '%s'",
 					  get_repmgr_schema());
 
 	log_verbose(LOG_DEBUG, "check_cluster_schema(): %s\n", sqlquery);
@@ -251,7 +279,6 @@ is_pgup(PGconn *conn, int timeout)
 	/* Check the connection status twice in case it changes after reset */
 	bool		twice = false;
 
-	/* Check the connection status twice in case it changes after reset */
 	for (;;)
 	{
 		if (PQstatus(conn) != CONNECTION_OK)
@@ -381,7 +408,7 @@ guc_set(PGconn *conn, const char *parameter, const char *op,
 	int			retval = 1;
 
 	sqlquery_snprintf(sqlquery,
-					  "SELECT true FROM pg_settings "
+					  "SELECT true FROM pg_catalog.pg_settings "
 					  " WHERE name = '%s' AND setting %s '%s'",
 					  parameter, op, value);
 
@@ -417,7 +444,7 @@ guc_set_typed(PGconn *conn, const char *parameter, const char *op,
 	int			retval = 1;
 
 	sqlquery_snprintf(sqlquery,
-					  "SELECT true FROM pg_settings "
+					  "SELECT true FROM pg_catalog.pg_settings "
 					  " WHERE name = '%s' AND setting::%s %s '%s'::%s",
 					  parameter, datatype, op, value, datatype);
 
@@ -449,7 +476,7 @@ get_cluster_size(PGconn *conn, char *size)
 
 	sqlquery_snprintf(sqlquery,
 					  "SELECT pg_catalog.pg_size_pretty(SUM(pg_catalog.pg_database_size(oid))::bigint) "
-					  "	 FROM pg_database ");
+					  "	 FROM pg_catalog.pg_database ");
 
 	log_verbose(LOG_DEBUG, "get_cluster_size():\n%s\n", sqlquery);
 
@@ -476,11 +503,11 @@ get_pg_setting(PGconn *conn, const char *setting, char *output)
 	char		sqlquery[QUERY_STR_LEN];
 	PGresult   *res;
 	int			i;
-	bool        success = true;
+	bool        success = false;
 
 	sqlquery_snprintf(sqlquery,
 					  "SELECT name, setting "
-					  " FROM pg_settings WHERE name = '%s'",
+					  "  FROM pg_catalog.pg_settings WHERE name = '%s'",
 					  setting);
 
 	log_verbose(LOG_DEBUG, "get_pg_setting(): %s\n", sqlquery);
@@ -917,7 +944,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;
@@ -936,8 +963,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"),
+			appendPQExpBuffer(error_msg,
-					slot_name);
+							  _("Slot '%s' exists and is not a physical slot\n"),
+							  slot_name);
 			return false;
 		}
 
@@ -949,8 +977,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"),
+		appendPQExpBuffer(error_msg,
-				slot_name);
+						  _("Slot '%s' already exists as an active slot\n"),
+						  slot_name);
 		return false;
 	}
 
@@ -958,25 +987,26 @@ create_replication_slot(PGconn *conn, char *slot_name, int server_version_num)
 	if (server_version_num >= 90600)
 	{
 		sqlquery_snprintf(sqlquery,
-						  "SELECT * FROM pg_create_physical_replication_slot('%s', TRUE)",
+						  "SELECT * FROM pg_catalog.pg_create_physical_replication_slot('%s', TRUE)",
 						  slot_name);
 	}
 	else
 	{
 		sqlquery_snprintf(sqlquery,
-						  "SELECT * FROM pg_create_physical_replication_slot('%s')",
+						  "SELECT * FROM pg_catalog.pg_create_physical_replication_slot('%s')",
 						  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"),
+		appendPQExpBuffer(error_msg,
-				slot_name,
+						  _("unable to create slot '%s' on the master node: %s\n"),
-				PQerrorMessage(conn));
+						  slot_name,
+						  PQerrorMessage(conn));
 		PQclear(res);
 		return false;
 	}
@@ -994,7 +1024,7 @@ get_slot_record(PGconn *conn, char *slot_name, t_replication_slot *record)
 
 	sqlquery_snprintf(sqlquery,
 					  "SELECT slot_name, slot_type, active "
-                      "  FROM pg_replication_slots "
+                      "  FROM pg_catalog.pg_replication_slots "
 					  " WHERE slot_name = '%s' ",
 					  slot_name);
 
@@ -1196,7 +1226,8 @@ witness_copy_node_records(PGconn *masterconn, PGconn *witnessconn, char *cluster
 
 	/* Get current records from primary */
 	sqlquery_snprintf(sqlquery,
-					  "SELECT id, type, upstream_node_id, name, conninfo, priority, slot_name, active FROM %s.repl_nodes",
+					  "SELECT id, type, upstream_node_id, name, conninfo, priority, slot_name, active "
+					  "  FROM %s.repl_nodes",
 					  get_repmgr_schema_quoted(masterconn));
 
 	log_verbose(LOG_DEBUG, "witness_copy_node_records():\n%s\n", sqlquery);
@@ -1310,7 +1341,8 @@ create_node_record(PGconn *conn, char *action, int node, char *type, int upstrea
 	sqlquery_snprintf(sqlquery,
 					  "INSERT INTO %s.repl_nodes "
 					  "       (id, type, upstream_node_id, cluster, "
-					  "        name, conninfo, slot_name, priority, active) "
+					  "        name, conninfo, slot_name, "
+					  "        priority, active) "
 					  "VALUES (%i, '%s', %s, '%s', '%s', '%s', %s, %i, %s) ",
 					  get_repmgr_schema_quoted(conn),
 					  node,
@@ -1404,10 +1436,11 @@ create_event_record(PGconn *conn, t_configuration_options *options, int node_id,
 	bool		success = true;
 	struct tm	ts;
 
-	/* Only attempt to write a record if a connection handle was provided.
+	/*
-	   Also check that the repmgr schema has been properly intialised - if
+	 * Only attempt to write a record if a connection handle was provided.
-	   not it means no configuration file was provided, which can happen with
+	 * Also check that the repmgr schema has been properly initialised - if
-	   e.g. `repmgr standby clone`, and we won't know which schema to write to.
+	 * not it means no configuration file was provided, which can happen with
+	 * e.g. `repmgr standby clone`, and we won't know which schema to write to.
 	 */
 	if (conn != NULL && strcmp(repmgr_schema, DEFAULT_REPMGR_SCHEMA_PREFIX) != 0)
 	{
@@ -1456,7 +1489,6 @@ create_event_record(PGconn *conn, t_configuration_options *options, int node_id,
 						PQerrorMessage(conn));
 
 			success = false;
-
 		}
 		else
 		{
@@ -1597,6 +1629,89 @@ create_event_record(PGconn *conn, t_configuration_options *options, int node_id,
 }
 
 
+bool
+update_node_record(PGconn *conn, char *action, int node, char *type, int upstream_node, char *cluster_name, char *node_name, char *conninfo, int priority, char *slot_name, bool active)
+{
+	char		sqlquery[QUERY_STR_LEN];
+	char		upstream_node_id[MAXLEN];
+	char		slot_name_buf[MAXLEN];
+	PGresult   *res;
+
+	/* XXX this segment copied from create_node_record() */
+	if (upstream_node == NO_UPSTREAM_NODE)
+	{
+		/*
+		 * No explicit upstream node id provided for standby - attempt to
+		 * get primary node id
+		 */
+		if (strcmp(type, "standby") == 0)
+		{
+			int primary_node_id = get_master_node_id(conn, cluster_name);
+			maxlen_snprintf(upstream_node_id, "%i", primary_node_id);
+		}
+		else
+		{
+			maxlen_snprintf(upstream_node_id, "%s", "NULL");
+		}
+	}
+	else
+	{
+		maxlen_snprintf(upstream_node_id, "%i", upstream_node);
+	}
+
+	if (slot_name != NULL && slot_name[0])
+	{
+		maxlen_snprintf(slot_name_buf, "'%s'", slot_name);
+	}
+	else
+	{
+		maxlen_snprintf(slot_name_buf, "%s", "NULL");
+	}
+
+	/* XXX convert to placeholder query */
+	sqlquery_snprintf(sqlquery,
+					  "UPDATE %s.repl_nodes SET "
+					  "       type = '%s', "
+					  "       upstream_node_id = %s, "
+					  "       cluster = '%s', "
+					  "       name = '%s', "
+					  "       conninfo = '%s', "
+					  "       slot_name = %s, "
+					  "       priority = %i, "
+					  "       active = %s "
+					  " WHERE id = %i ",
+					  get_repmgr_schema_quoted(conn),
+					  type,
+					  upstream_node_id,
+					  cluster_name,
+					  node_name,
+					  conninfo,
+					  slot_name_buf,
+					  priority,
+					  active == true ? "TRUE" : "FALSE",
+					  node);
+
+	log_verbose(LOG_DEBUG, "update_node_record(): %s\n", sqlquery);
+
+	if (action != NULL)
+	{
+		log_verbose(LOG_DEBUG, "update_node_record(): action is \"%s\"\n", action);
+	}
+
+	res = PQexec(conn, sqlquery);
+	if (!res || PQresultStatus(res) != PGRES_COMMAND_OK)
+	{
+		log_err(_("Unable to update node record\n%s\n"),
+				PQerrorMessage(conn));
+		PQclear(res);
+		return false;
+	}
+
+	PQclear(res);
+
+	return true;
+}
+
 /*
  * Update node record following change of status
  * (e.g. inactive primary converted to standby)
@@ -1686,7 +1801,8 @@ get_node_record(PGconn *conn, char *cluster, int node_id, t_node_info *node_info
 
 	sqlquery_snprintf(
 		sqlquery,
-		"SELECT id, type, upstream_node_id, name, conninfo, slot_name, priority, active"
+		"SELECT id, type, upstream_node_id, name, conninfo, "
+		"       slot_name, priority, active"
 		"  FROM %s.repl_nodes "
 		" WHERE cluster = '%s' "
 		"   AND id = %i",

dbutils.h

@@ -1,5 +1,6 @@
 /*
  * dbutils.h
+ *
  * Copyright (c) 2ndQuadrant, 2010-2016
  *
  * This program is free software: you can redistribute it and/or modify
@@ -21,6 +22,7 @@
 #define _REPMGR_DBUTILS_H_
 
 #include "access/xlogdefs.h"
+#include "pqexpbuffer.h"
 
 #include "config.h"
 #include "strutil.h"
@@ -77,15 +79,16 @@ typedef struct s_replication_slot
 	bool active;
 }   t_replication_slot;
 
-
+extern char		repmgr_schema[MAXLEN];
 
 PGconn *_establish_db_connection(const char *conninfo,
 								 const bool exit_on_error,
-								 const bool log_notice);
+								 const bool log_notice,
+								 const bool verbose_only);
 PGconn *establish_db_connection(const char *conninfo,
 								const bool exit_on_error);
-PGconn *test_db_connection(const char *conninfo,
+PGconn *establish_db_connection_quiet(const char *conninfo);
-						   const bool exit_on_error);
+PGconn *test_db_connection(const char *conninfo);
 PGconn *establish_db_connection_by_params(const char *keywords[],
 								  const char *values[],
 								  const bool exit_on_error);
@@ -116,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);
@@ -127,6 +130,7 @@ bool		create_node_record(PGconn *conn, char *action, int node, char *type, int u
 bool		delete_node_record(PGconn *conn, int node, char *action);
 int			get_node_record(PGconn *conn, char *cluster, int node_id, t_node_info *node_info);
 int			get_node_record_by_name(PGconn *conn, char *cluster, const char *node_name, t_node_info *node_info);
+bool        update_node_record(PGconn *conn, char *action, int node, char *type, int upstream_node, char *cluster_name, char *node_name, char *conninfo, int priority, char *slot_name, bool active);
 bool        update_node_record_status(PGconn *conn, char *cluster_name, int this_node_id, char *type, int upstream_node_id, bool active);
 bool        update_node_record_set_upstream(PGconn *conn, char *cluster_name, int this_node_id, int new_upstream_node_id);
 bool        create_event_record(PGconn *conn, t_configuration_options *options, int node_id, char *event, bool successful, char *details);

debian/DEBIAN/control

@@ -1,5 +1,5 @@
 Package: repmgr-auto
-Version: 3.1.3
+Version: 3.2dev
 Section: database
 Priority: optional
 Architecture: all

docs/repmgrd-failover-mechanism.md

@@ -0,0 +1,75 @@
+repmgrd's failover algorithm
+============================
+
+When implementing automatic failover, there are two factors which are critical in
+ensuring the desired result is achieved:
+
+  - has the master node genuinely failed?
+  - which is the best node to promote to the new master?
+
+This document outlines repmgrd's decision-making process during automatic failover
+for standbys directly connected to the master node.
+
+
+Master node failure detection
+-----------------------------
+
+If a `repmgrd` instance running on a PostgreSQL standby node is unable to connect to
+the master node, this doesn't neccesarily mean that the master is down and a
+failover is required. Factors such as network connectivity issues could mean that
+even though the standby node is isolated, the replication cluster as a whole
+is functioning correctly, and promoting the standby without further verification
+could result in a "split-brain" situation.
+
+In the event that `repmgrd` is unable to connect to the master node, it will attempt
+to reconnect to the master server several times (as defined by the `reconnect_attempts`
+parameter in `repmgr.conf`), with reconnection attempts  occuring at the interval
+specified by `reconnect_interval`. This happens to verify that the master is definitively
+not accessible (e.g. that connection was not lost due to a brief network glitch).
+
+Appropriate values for these settings will depend very much on the replication
+cluster environment. There will necessarily be a trade-off between the time it
+takes to assume the master is not reachable, and the reliability of that conclusion.
+A standby in a different physical location to the master will probably need a longer
+check interval to rule out possible network issues, whereas one located in the same
+rack with a direct connection between servers could perform the check very quickly.
+
+Note that it's possible the master comes back online after this point is reached,
+but before a new master has been selected; in this case it will be noticed
+during the selection of a new master and no actual failover will take place.
+
+Promotion candidate selection
+-----------------------------
+
+Once `repmgrd` has decided the master is definitively unreachable, following checks
+will be carried out:
+
+* attempts to connect to all other nodes in the cluster (including the witness
+  node, if defined) to establish the state of the cluster, including their
+  current LSN
+
+* If less than half of the nodes are visible (from the viewpoint
+  of this node), `repmgrd` will not take any further action. This is to ensure that
+  e.g. if a replication cluster is spread over multiple data centres, a split-brain
+  situation does not occur if there is a network failure between datacentres. Note
+  that if nodes are split evenly between data centres, a witness server can be
+  used to establish the "majority" daat centre.
+
+* `repmgrd` polls all visible servers and waits for each node to return a valid LSN;
+  it updates the LSN previously  stored for this node if it has increased since
+  the initial check
+
+* once all LSNs have been retrieved, `repmgrd` will check for the highest LSN; if
+  its own node has the highest LSN, it will attempt to promote itself (using the
+  command defined in `promote_command` in `repmgr.conf`. Note that if using
+  `repmgr standby promote` as the promotion command, and the original master becomes available
+  before the promotion takes effect, `repmgr` will return an error and no promotion
+  will take place, and `repmgrd` will resume monitoring as usual.
+
+* if the node is not the promotion candidate, `repmgrd` will execute the
+  `follow_command` defined in `repmgr.conf`. If using `repmgr standby follow` here,
+  `repmgr` will attempt to detect the new master node and attach to that.
+
+
+
+

docs/repmgrd-node-fencing.md

@@ -0,0 +1,152 @@
+Fencing a failed master node with repmgrd and pgbouncer
+=======================================================
+
+With automatic failover, it's essential to ensure that a failed master
+remains inaccessible to your application, even if it comes back online
+again, to avoid a split-brain situation.
+
+By using `pgbouncer` together with `repmgrd`, it's possible to combine
+automatic failover with a process to isolate the failed master from
+your application and ensure that all connections which should go to
+the master are directed there smoothly without having to reconfigure
+your application. (Note that as a connection pooler, `pgbouncer` can
+benefit your application in other ways, but those are beyond the scope
+of this document).
+
+* * *
+
+> *WARNING*: automatic failover is tricky to get right. This document
+> demonstrates one possible implementation method, however you should
+> carefully configure and test any setup to suit the needs of your own
+> replication cluster/application.
+
+* * *
+
+In a failover situation, `repmgrd` promotes a standby to master by executing
+the command defined in `promote_command`. Normally this would be something like:
+
+    repmgr standby promote -f /etc/repmgr.conf
+
+By wrapping this in a custom script which adjusts the `pgbouncer` configuration
+on all nodes, it's possible to fence the failed master and redirect write
+connections to the new master.
+
+The script consists of three sections:
+
+* commands to pause `pgbouncer` on all nodes
+* the promotion command itself
+* commands to reconfigure and restart `pgbouncer` on all nodes
+
+Note that it requires password-less SSH access between all nodes to be able to
+update the `pgbouncer` configuration files.
+
+For the purposes of this demonstration, we'll assume there are 3 nodes (master
+and two standbys), with `pgbouncer` listening on port 6432 handling connections
+to a database called `appdb`.  The `postgres` system user must have write
+access to the `pgbouncer` configuration files on all nodes. We'll assume
+there's a main `pgbouncer` configuration file, `/etc/pgbouncer.ini`, which uses
+the `%include` directive (available from PgBouncer 1.6) to include a separate
+configuration file, `/etc/pgbouncer.database.ini`, which will be modified by
+`repmgr`.
+
+`/etc/pgbouncer.ini` should look something like this:
+
+    [pgbouncer]
+
+    logfile = /var/log/pgbouncer/pgbouncer.log
+    pidfile = /var/run/pgbouncer/pgbouncer.pid
+
+    listen_addr = *
+    listen_port = 6532
+    unix_socket_dir = /tmp
+
+    auth_type = trust
+    auth_file = /etc/pgbouncer.auth
+
+    admin_users = postgres
+    stats_users = postgres
+
+    pool_mode = transaction
+
+    max_client_conn = 100
+    default_pool_size = 20
+    min_pool_size = 5
+    reserve_pool_size = 5
+    reserve_pool_timeout = 3
+
+    log_connections = 1
+    log_disconnections = 1
+    log_pooler_errors = 1
+
+    %include /etc/pgbouncer.database.ini
+
+The actual script is as follows; adjust the configurable items as appropriate:
+
+`/var/lib/postgres/repmgr/promote.sh`
+
+
+    #!/usr/bin/env bash
+    set -u
+    set -e
+
+    # Configurable items
+    PGBOUNCER_HOSTS="node1 node2 node3"
+    PGBOUNCER_DATABASE_INI="/etc/pgbouncer.database.ini"
+    PGBOUNCER_DATABASE="appdb"
+    PGBOUNCER_PORT=6432
+
+    REPMGR_DB="repmgr"
+    REPMGR_USER="repmgr"
+    REPMGR_SCHEMA="repmgr_test"
+
+    # 1. Pause running pgbouncer instances
+    for HOST in $PGBOUNCER_HOSTS
+    do
+        psql -t -c "pause" -h $HOST -p $PGBOUNCER_PORT -U postgres pgbouncer
+    done
+
+    # 2. Promote this node from standby to master
+
+    repmgr standby promote -f /etc/repmgr.conf
+
+    # 3. Reconfigure pgbouncer instances
+
+    PGBOUNCER_DATABASE_INI_NEW="/tmp/pgbouncer.database.ini"
+
+    for HOST in $PGBOUNCER_HOSTS
+    do
+        # Recreate the pgbouncer config file
+        echo -e "[databases]\n" > $PGBOUNCER_DATABASE_INI_NEW
+
+        psql -d $REPMGR_DB -U $REPMGR_USER -t -A \
+          -c "SELECT '${PGBOUNCER_DATABASE}-rw= ' || conninfo || ' application_name=pgbouncer_${HOST}' \
+              FROM ${REPMGR_SCHEMA}.repl_nodes \
+              WHERE active = TRUE AND type='master'" >> $PGBOUNCER_DATABASE_INI_NEW
+
+        psql -d $REPMGR_DB -U $REPMGR_USER -t -A \
+          -c "SELECT '${PGBOUNCER_DATABASE}-ro= ' || conninfo || ' application_name=pgbouncer_${HOST}' \
+              FROM $REPMGR_SCHEMA.repl_nodes \
+              WHERE node_name='${HOST}'" >> $PGBOUNCER_DATABASE_INI_NEW
+
+        rsync $PGBOUNCER_DATABASE_INI_NEW $HOST:$PGBOUNCER_DATABASE_INI
+
+        psql -tc "reload" -h $HOST -p $PGBOUNCER_PORT -U postgres pgbouncer
+        psql -tc "resume" -h $HOST -p $PGBOUNCER_PORT -U postgres pgbouncer
+
+    done
+
+    # Clean up generated file
+    rm $PGBOUNCER_DATABASE_INI_NEW
+
+    echo "Reconfiguration of pgbouncer complete"
+
+Script and template file should be installed on each node where
+`repmgrd` is running.
+
+Finally, set `promote_command` in `repmgr.conf` on each node to
+point to the custom promote script:
+
+    promote_command=/var/lib/postgres/repmgr/promote.sh
+
+and reload/restart any running `repmgrd` instances for the changes to take
+effect.

errcode.h

@@ -29,7 +29,6 @@
 #define ERR_DB_CON 6
 #define ERR_DB_QUERY 7
 #define ERR_PROMOTED 8
-#define ERR_BAD_PASSWORD 9
 #define ERR_STR_OVERFLOW 10
 #define ERR_FAILOVER_FAIL 11
 #define ERR_BAD_SSH 12
@@ -39,5 +38,8 @@
 #define ERR_MONITORING_FAIL 16
 #define ERR_BAD_BACKUP_LABEL 17
 #define ERR_SWITCHOVER_FAIL 18
+#define ERR_BARMAN 19
+#define ERR_REGISTRATION_SYNC 20
+
 
 #endif   /* _ERRCODE_H_ */

log.c

@@ -48,6 +48,11 @@ int			log_level = LOG_NOTICE;
 int			last_log_level = LOG_NOTICE;
 int			verbose_logging = false;
 int			terse_logging = false;
+/*
+ * Global variable to be set by the main application to ensure any log output
+ * emitted before logger_init is called, is output in the correct format
+ */
+int			logger_output_mode = OM_DAEMON;
 
 extern void
 stderr_log_with_level(const char *level_name, int level, const char *fmt, ...)
@@ -62,9 +67,7 @@ stderr_log_with_level(const char *level_name, int level, const char *fmt, ...)
 static void
 _stderr_log_with_level(const char *level_name, int level, const char *fmt, va_list ap)
 {
-	time_t		t;
+	char		buf[100];
-	struct tm  *tm;
-	char		buff[100];
 
 	/*
 	 * Store the requested level so that if there's a subsequent
@@ -74,10 +77,21 @@ _stderr_log_with_level(const char *level_name, int level, const char *fmt, va_li
 
 	if (log_level >= level)
 	{
-		time(&t);
+
-		tm = localtime(&t);
+		/* Format log line prefix with timestamp if in daemon mode */
-		strftime(buff, 100, "[%Y-%m-%d %H:%M:%S]", tm);
+		if (logger_output_mode == OM_DAEMON)
-		fprintf(stderr, "%s [%s] ", buff, level_name);
+		{
+			time_t		t;
+			struct tm  *tm;
+			time(&t);
+			tm = localtime(&t);
+			strftime(buf, 100, "[%Y-%m-%d %H:%M:%S]", tm);
+			fprintf(stderr, "%s [%s] ", buf, level_name);
+		}
+		else
+		{
+			fprintf(stderr, "%s: ", level_name);
+		}
 
 		vfprintf(stderr, fmt, ap);
 
@@ -142,7 +156,7 @@ log_verbose(int level, const char *fmt, ...)
 
 
 bool
-logger_init(t_configuration_options * opts, const char *ident)
+logger_init(t_configuration_options *opts, const char *ident)
 {
 	char	   *level = opts->loglevel;
 	char	   *facility = opts->logfacility;
@@ -176,6 +190,13 @@ logger_init(t_configuration_options * opts, const char *ident)
 			stderr_log_warning(_("Invalid log level \"%s\" (available values: DEBUG, INFO, NOTICE, WARNING, ERR, ALERT, CRIT or EMERG)\n"), level);
 	}
 
+	/*
+	 * STDERR only logging requested - finish here without setting up any further
+	 * logging facility.
+	 */
+	if (logger_output_mode == OM_COMMAND_LINE)
+		return true;
+
 	if (facility && *facility)
 	{
 
@@ -236,9 +257,10 @@ logger_init(t_configuration_options * opts, const char *ident)
 		stderr_log_notice(_("Redirecting logging output to '%s'\n"), opts->logfile);
 		fd = freopen(opts->logfile, "a", stderr);
 
-		/* It's possible freopen() may still fail due to e.g. a race condition;
+		/*
-		   as it's not feasible to restore stderr after a failed freopen(),
+		 * It's possible freopen() may still fail due to e.g. a race condition;
-		   we'll write to stdout as a last resort.
+		 * as it's not feasible to restore stderr after a failed freopen(),
+		 * we'll write to stdout as a last resort.
 		 */
 		if (fd == NULL)
 		{

log.h

@@ -25,6 +25,9 @@
 #define REPMGR_SYSLOG 1
 #define REPMGR_STDERR 2
 
+#define OM_COMMAND_LINE 1
+#define OM_DAEMON       2
+
 extern void
 stderr_log_with_level(const char *level_name, int level, const char *fmt,...)
 __attribute__((format(PG_PRINTF_ATTRIBUTE, 3, 4)));
@@ -130,5 +133,8 @@ __attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 3)));
 
 extern int	log_type;
 extern int	log_level;
+extern int	verbose_logging;
+extern int	terse_logging;
+extern int	logger_output_mode;
 
-#endif
+#endif /* _REPMGR_LOG_H_ */

repmgr.conf.sample

@@ -31,7 +31,7 @@
 #
 #   https://www.postgresql.org/docs/current/static/libpq-connect.html#LIBPQ-CONNSTRING
 #
-#conninfo='host=192.168.204.104 dbname=repmgr_db user=repmgr_usr'
+#conninfo='host=192.168.204.104 dbname=repmgr user=repmgr'
 #
 # If repmgrd is in use, consider explicitly setting `connect_timeout` in the
 # conninfo string to determine the length of time which elapses before
@@ -66,6 +66,12 @@
 # (default: NOTICE)
 #loglevel=NOTICE
 
+# Note that logging facility settings will only apply to `repmgrd` by default;
+# `repmgr` will always write to STDERR unless the switch `--log-to-file` is
+# supplied, in which case it will log to the same destination as `repmgrd`.
+# This is mainly intended for those cases when `repmgr` is executed directly
+# by `repmgrd`.
+
 # Logging facility: possible values are STDERR or - for Syslog integration - one of LOCAL0, LOCAL1, ..., LOCAL7, USER
 # (default: STDERR)
 #logfacility=STDERR
@@ -74,6 +80,12 @@
 #
 #logfile='/var/log/repmgr/repmgr.log'
 
+# By default only repmgrd log output will be written to a file,
+# if defined in "logfile"
+# enable this to restore old behaviour where output from the repmgr
+# client will be written to the logfile too
+#log_repmgr_to_file = 0
+
 # event notifications can be passed to an arbitrary external program
 # together with the following parameters:
 #
@@ -100,6 +112,34 @@
 # path to PostgreSQL binary directory (location of pg_ctl, pg_basebackup etc.)
 # (if not provided, defaults to system $PATH)
 #pg_bindir=/usr/bin/
+#
+# Debian/Ubuntu users: you will probably need to set this to the directory
+# where `pg_ctl` is located, e.g. /usr/lib/postgresql/9.5/bin/
+
+# service control commands
+#
+# repmgr provides options to override the default pg_ctl commands
+# used to stop, start, restart, reload and promote the PostgreSQL cluster
+#
+# NOTE: These commands must be runnable on remote nodes as well for switchover
+# to function correctly.
+#
+# If you use sudo, the user repmgr runs as (usually 'postgres')  must have
+# passwordless sudo access to execute the command
+#
+# For example, to use systemd, you may use the following configuration:
+#
+#    # this is required when running sudo over ssh without -t:
+#    Defaults:postgres !requiretty
+#    postgres ALL = NOPASSWD: /usr/bin/systemctl stop postgresql-9.5, \
+#       /usr/bin/systemctl start postgresql-9.5, \
+#       /usr/bin/systemctl restart postgresql-9.5
+#
+# service_start_command = systemctl start postgresql-9.5
+# service_stop_command = systemctl stop postgresql-9.5
+# service_restart_command = systemctl restart postgresql-9.5
+# service_reload_command = pg_ctlcluster 9.5 main reload
+# service_promote_command = pg_ctlcluster 9.5 main promote
 
 # external command options
 
@@ -132,9 +172,11 @@
 # These settings are only applied when repmgrd is running. Values shown
 # are defaults.
 
-# Number of seconds to wait for a response from the primary server before
+# monitoring interval in seconds; default is 2
-# deciding it has failed.
+#monitor_interval_secs=2
 
+# Maximum number of seconds to wait for a response from the primary server
+# before deciding it has failed.
 #master_response_timeout=60
 
 # Number of attempts at what interval (in seconds) to try and
@@ -144,16 +186,21 @@
 #reconnect_interval=10
 
 # Autofailover options
-#failover=manual    # one of 'automatic', 'manual'
+#failover=manual    # one of 'automatic', 'manual' (default: manual)
-                    # (default: manual)
+                    # defines the action to take in the event of upstream failure
-#priority=100       # a value of zero or less prevents the node being promoted to primary
+                    #
+                    # 'automatic': repmgrd will automatically attempt to promote the
+                    #    node or follow the new upstream node
+                    # 'manual': repmgrd will take no action and the mode will require
+                    #    manual attention to reattach it to replication
+
+#priority=100       # indicate a preferred priorty for promoting nodes
+                    # a value of zero or less prevents the node being promoted to primary
                     # (default: 100)
+
 #promote_command='repmgr standby promote -f /path/to/repmgr.conf'
 #follow_command='repmgr standby follow -f /path/to/repmgr.conf -W'
 
-# monitoring interval in seconds; default is 2
-#monitor_interval_secs=2
-
 # change wait time for primary; before we bail out and exit when the primary
 # disappears, we wait 'reconnect_attempts' * 'retry_promote_interval_secs'
 # seconds; by default this would be half an hour, as 'retry_promote_interval_secs'

repmgr.h

@@ -23,6 +23,7 @@
 #include <libpq-fe.h>
 #include <postgres_fe.h>
 #include <getopt_long.h>
+#include "pqexpbuffer.h"
 
 #include "strutil.h"
 #include "dbutils.h"
@@ -35,7 +36,7 @@
 
 #define ERRBUFF_SIZE	512
 
-#define DEFAULT_WAL_KEEP_SEGMENTS	"5000"
+#define DEFAULT_WAL_KEEP_SEGMENTS	"0"
 #define DEFAULT_DEST_DIR		"."
 #define DEFAULT_REPMGR_SCHEMA_PREFIX	"repmgr_"
 #define DEFAULT_PRIORITY		100
@@ -47,56 +48,133 @@
 #define NO_UPSTREAM_NODE	-1
 #define UNKNOWN_NODE_ID     -1
 
+/* command line options without short versions */
+#define OPT_HELP                         1
+#define OPT_CHECK_UPSTREAM_CONFIG        2
+#define OPT_RECOVERY_MIN_APPLY_DELAY     3
+#define OPT_COPY_EXTERNAL_CONFIG_FILES   4
+#define OPT_CONFIG_ARCHIVE_DIR           5
+#define OPT_PG_REWIND                    6
+#define OPT_PWPROMPT                     7
+#define OPT_CSV                          8
+#define OPT_NODE                         9
+#define OPT_WITHOUT_BARMAN               10
+#define OPT_NO_UPSTREAM_CONNECTION       11
+#define OPT_REGISTER_WAIT                12
+#define OPT_CLUSTER                      13
+#define OPT_LOG_TO_FILE                  14
+#define OPT_UPSTREAM_CONNINFO            15
+#define OPT_NO_CONNINFO_PASSWORD         16
+#define OPT_REPLICATION_USER             17
+
+/* deprecated command line options */
+#define OPT_INITDB_NO_PWPROMPT           998
+#define OPT_IGNORE_EXTERNAL_CONFIG_FILES 999
+
+/* values for --copy-external-config-files */
+#define CONFIG_FILE_SAMEPATH 1
+#define CONFIG_FILE_PGDATA 2
 
 
 /* Run time options type */
 typedef struct
 {
+	/* general repmgr options */
+	char		config_file[MAXPGPATH];
+	bool		verbose;
+	bool		terse;
+	bool		force;
+	char		pg_bindir[MAXLEN]; /* overrides setting in repmgr.conf */
 
+	/* logging parameters */
+	char		loglevel[MAXLEN];  /* overrides setting in repmgr.conf */
+	bool		log_to_file;
+
+	/* connection parameters */
 	char		dbname[MAXLEN];
 	char		host[MAXLEN];
 	char		username[MAXLEN];
 	char		dest_dir[MAXPGPATH];
-	char		config_file[MAXPGPATH];
 	char		remote_user[MAXLEN];
 	char		superuser[MAXLEN];
+	char		masterport[MAXLEN];
+	bool		conninfo_provided;
+	bool		connection_param_provided;
+	bool		host_param_provided;
+
+	/* standby clone parameters */
+	bool		wal_keep_segments_used;
 	char		wal_keep_segments[MAXLEN];
-	bool		verbose;
-	bool		terse;
-	bool		force;
-	bool		wait_for_master;
 	bool		ignore_rsync_warn;
-	bool		witness_pwprompt;
 	bool		rsync_only;
 	bool		fast_checkpoint;
-	bool		ignore_external_config_files;
+	bool		without_barman;
-	bool		csv_mode;
+	bool		no_upstream_connection;
-	char		masterport[MAXLEN];
+	bool		no_conninfo_password;
-	/*
+	bool		copy_external_config_files;
-	 * configuration file parameters which can be overridden on the
+	int			copy_external_config_files_destination;
-	 * command line
+	char		upstream_conninfo[MAXLEN];
-	 */
+	char		replication_user[MAXLEN];
-	char		loglevel[MAXLEN];
-
-	/* parameter used by STANDBY SWITCHOVER */
-	char		remote_config_file[MAXLEN];
-	char		pg_rewind[MAXPGPATH];
-	char		pg_ctl_mode[MAXLEN];
-	/* parameter used by STANDBY {ARCHIVE_CONFIG | RESTORE_CONFIG} */
-	char		config_archive_dir[MAXLEN];
-	/* parameter used by CLUSTER CLEANUP */
-	int			keep_history;
-
-	char		pg_bindir[MAXLEN];
 
 	char		recovery_min_apply_delay[MAXLEN];
 
-	/* deprecated command line options */
+	/* standby register paarameters */
-	char		localport[MAXLEN];
+	bool		wait_register_sync;
-	bool		initdb_no_pwprompt;
+	int			wait_register_sync_seconds;
+
+	/* witness create parameters */
+	bool		witness_pwprompt;
+
+	/* standby follow parameters */
+	bool		wait_for_master;
+
+	/* cluster {show|matrix|crosscheck} parameters */
+	bool		csv_mode;
+
+	/* cluster cleanup parameters */
+	int			keep_history;
+
+	/* standby switchover parameters */
+	char		remote_config_file[MAXLEN];
+	bool		pg_rewind_supplied;
+	char		pg_rewind[MAXPGPATH];
+	char		pg_ctl_mode[MAXLEN];
+
+	/* standby {archive_config | restore_config} parameters  */
+	char		config_archive_dir[MAXLEN];
+
+	/* {standby|witness} unregister parameters */
+	int			node;
+
 }	t_runtime_options;
 
-#define T_RUNTIME_OPTIONS_INITIALIZER { "", "", "", "", "", "", "", DEFAULT_WAL_KEEP_SEGMENTS, false, false, false, false, false, false, false, false, false, false, "", "", "", "", "fast", "", 0, "", "", "", false }
+#define T_RUNTIME_OPTIONS_INITIALIZER { \
+		/* general repmgr options */	\
+		"", false, false, false, "",	\
+		/* logging parameters */ \
+		"", false,                      \
+		/* connection parameters */		\
+		"", "", "", "", "", "", "", 	\
+		false, false, false,		    \
+		/* standby clone parameters */  \
+		false, DEFAULT_WAL_KEEP_SEGMENTS, false, false, false, false, false, false, \
+		false, CONFIG_FILE_SAMEPATH, "", "", "", \
+		/* standby register paarameters */ \
+	    false, 0,							 \
+		/* witness create parameters */ \
+		false,                          \
+		/* standby follow parameters */ \
+		false,                          \
+		/* cluster {show|matrix|crosscheck} parameters */ \
+		false,                          \
+		/* cluster cleanup parameters */ \
+		0,                              \
+		/* standby switchover parameters */ \
+		"", false, "", "fast",          \
+		/* standby {archive_config | restore_config} parameters  */ \
+		"",                             \
+		/* {standby|witness} unregister parameters */ \
+		UNKNOWN_NODE_ID }
 
 struct BackupLabel
 {
@@ -110,7 +188,60 @@ struct BackupLabel
 	XLogRecPtr min_failover_slot_lsn;
 };
 
-extern char		repmgr_schema[MAXLEN];
+
-extern bool		config_file_found;
+typedef struct
+{
+	char		slot[MAXLEN];
+	char		xlog_method[MAXLEN];
+} t_basebackup_options;
+
+#define T_BASEBACKUP_OPTIONS_INITIALIZER { "", "" }
+
+typedef struct
+{
+	int    size;
+	char **keywords;
+	char **values;
+} t_conninfo_param_list;
+
+typedef struct
+{
+	char filepath[MAXPGPATH];
+	char filename[MAXPGPATH];
+	bool in_data_directory;
+} t_configfile_info;
+
+
+typedef struct
+{
+	int    size;
+	int    entries;
+	t_configfile_info **files;
+} t_configfile_list;
+
+#define T_CONFIGFILE_LIST_INITIALIZER { 0, 0, NULL }
+
+
+typedef struct
+{
+	int node_id;
+	int node_status;
+} t_node_status_rec;
+
+typedef struct
+{
+	int node_id;
+	char node_name[MAXLEN];
+	t_node_status_rec **node_status_list;
+} t_node_matrix_rec;
+
+typedef struct
+{
+	int node_id;
+	char node_name[MAXLEN];
+	t_node_matrix_rec **matrix_list_rec;
+} t_node_status_cube;
+
+
 
 #endif

repmgr.sql

@@ -64,7 +64,7 @@ CREATE INDEX idx_repl_status_sort ON repl_monitor(last_monitor_time, standby_nod
  * This view shows the list of nodes with the information of which one is the upstream
  * in each case (when appliable)
  */
-CREATE VIEW repl_show_nodes AS 
+CREATE VIEW repl_show_nodes AS
 SELECT rn.id, rn.conninfo, rn.type, rn.name, rn.cluster,
 	rn.priority, rn.active, sq.name AS upstream_node_name
 FROM repl_nodes as rn LEFT JOIN repl_nodes AS sq ON sq.id=rn.upstream_node_id;

repmgrd.c

@@ -1,5 +1,6 @@
 /*
  * repmgrd.c - Replication manager daemon
+ *
  * Copyright (C) 2ndQuadrant, 2010-2016
  *
  * This module connects to the nodes of a replication cluster and monitors
@@ -41,7 +42,10 @@
 #include "access/xlogdefs.h"
 #include "pqexpbuffer.h"
 
+/* Message strings passed in repmgrSharedState->location */
 
+#define PASSIVE_NODE "PASSIVE_NODE"
+#define LSN_QUERY_ERROR "LSN_QUERY_ERROR"
 
 /* Local info */
 t_configuration_options local_options = T_CONFIGURATION_OPTIONS_INITIALIZER;
@@ -59,6 +63,13 @@ t_node_info node_info;
 
 bool		failover_done = false;
 
+/*
+ * when `failover=manual`, and the upstream server has gone away,
+ * this flag is set to indicate we should connect to whatever the
+ * current master is to update monitoring information
+ */
+bool		manual_mode_upstream_disconnected = false;
+
 char	   *pid_file = NULL;
 
 static void help(void);
@@ -100,17 +111,15 @@ static void check_and_create_pid_file(const char *pid_file);
 static void
 close_connections()
 {
-	if (master_conn != NULL && PQisBusy(master_conn) == 1)
+	if (PQstatus(master_conn) == CONNECTION_OK && PQisBusy(master_conn) == 1)
 		cancel_query(master_conn, local_options.master_response_timeout);
 
-	if (my_local_conn != NULL)
+
+	if (PQstatus(my_local_conn) == CONNECTION_OK)
 		PQfinish(my_local_conn);
 
-	if (master_conn != NULL && master_conn != my_local_conn)
+	if (PQstatus(master_conn) == CONNECTION_OK)
 		PQfinish(master_conn);
-
-	master_conn = NULL;
-	my_local_conn = NULL;
 }
 
 
@@ -124,7 +133,7 @@ main(int argc, char **argv)
 		{"monitoring-history", no_argument, NULL, 'm'},
 		{"daemonize", no_argument, NULL, 'd'},
 		{"pid-file", required_argument, NULL, 'p'},
-		{"help", no_argument, NULL, '?'},
+		{"help", no_argument, NULL, OPT_HELP},
 		{"version", no_argument, NULL, 'V'},
 		{NULL, 0, NULL, 0}
 	};
@@ -158,6 +167,23 @@ main(int argc, char **argv)
 	{
 		switch (c)
 		{
+			case '?':
+				/* Actual help option given */
+				if (strcmp(argv[optind - 1], "-?") == 0)
+				{
+					help();
+					exit(SUCCESS);
+				}
+				/* unknown option reported by getopt */
+				else
+					goto unknown_option;
+				break;
+			case OPT_HELP:
+				help();
+				exit(SUCCESS);
+			case 'V':
+				printf("%s %s (PostgreSQL %s)\n", progname(), REPMGR_VERSION, PG_VERSION);
+				exit(SUCCESS);
 			case 'f':
 				config_file = optarg;
 				break;
@@ -173,18 +199,21 @@ main(int argc, char **argv)
 			case 'p':
 				pid_file = optarg;
 				break;
-			case '?':
+
-				help();
-				exit(SUCCESS);
-			case 'V':
-				printf("%s %s (PostgreSQL %s)\n", progname(), REPMGR_VERSION, PG_VERSION);
-				exit(SUCCESS);
 			default:
+		unknown_option:
 				usage();
 				exit(ERR_BAD_CONFIG);
 		}
 	}
 
+
+	/*
+	 * Tell the logger we're a daemon - this will ensure any output logged
+	 * before the logger is initialized will be formatted correctly
+	 */
+	logger_output_mode = OM_DAEMON;
+
 	/*
 	 * Parse the configuration file, if provided. If no configuration file
 	 * was provided, or one was but was incomplete, parse_config() will
@@ -225,6 +254,7 @@ main(int argc, char **argv)
 	}
 
 	logger_init(&local_options, progname());
+
 	if (verbose)
 		logger_set_verbose();
 
@@ -289,10 +319,46 @@ main(int argc, char **argv)
 
 	log_debug("node id is %i, upstream is %i\n", node_info.node_id, node_info.upstream_node_id);
 
+    /*
+     * Check if node record is active - if not, and `failover=automatic`, the node
+     * won't be considered as a promotion candidate; this often happens when
+     * a failed primary is recloned and the node was not re-registered, giving
+     * the impression failover capability is there when it's not. In this case
+     * abort with an error and a hint about registering.
+     *
+     * If `failover=manual`, repmgrd can continue to passively monitor the node, but
+     * we should nevertheless issue a warning and the same hint.
+     */
+
+    if (node_info.active == false)
+    {
+        char *hint = "Check that 'repmgr (master|standby) register' was executed for this node";
+
+        switch (local_options.failover)
+        {
+            case AUTOMATIC_FAILOVER:
+                log_err(_("This node is marked as inactive and cannot be used for failover\n"));
+                log_hint(_("%s\n"), hint);
+                terminate(ERR_BAD_CONFIG);
+
+            case MANUAL_FAILOVER:
+                log_warning(_("This node is marked as inactive and will be passively monitored only\n"));
+                log_hint(_("%s\n"), hint);
+                break;
+
+            default:
+                /* This should never happen */
+                log_err(_("Unknown failover mode %i\n"), local_options.failover);
+                terminate(ERR_BAD_CONFIG);
+        }
+
+    }
+
 	/*
 	 * MAIN LOOP This loops cycles at startup and once per failover and
-	 * Requisites: - my_local_conn needs to be already setted with an active
+	 * Requisites:
-	 * connection - no master connection
+	 *  - my_local_conn must have an active connection to the monitored node
+	 *  - master_conn must not be open
 	 */
 	do
 	{
@@ -404,7 +470,7 @@ main(int argc, char **argv)
 													local_options.cluster_name,
 													&master_options.node, NULL);
 
-				if (master_conn == NULL)
+				if (PQstatus(master_conn) != CONNECTION_OK)
 				{
 					PQExpBufferData errmsg;
 					initPQExpBuffer(&errmsg);
@@ -433,6 +499,7 @@ main(int argc, char **argv)
 					my_local_conn = establish_db_connection(local_options.conninfo, true);
 					update_registration();
 				}
+
 				/* Log startup event */
 				if (startup_event_logged == false)
 				{
@@ -588,15 +655,15 @@ witness_monitor(void)
 			}
 			else
 			{
-				log_debug(_("new master found with node ID: %i\n"), master_options.node);
+				log_info(_("new master found with node ID: %i\n"), master_options.node);
 				connection_ok = true;
 
 				/*
 				 * Update the repl_nodes table from the new master to reflect the changed
 				 * node configuration
 				 *
-				 * XXX it would be neat to be able to handle this with e.g. table-based
+				 * It would be neat to be able to handle this with e.g. table-based
-				 * logical replication
+				 * logical replication if available in core
 				 */
 				witness_copy_node_records(master_conn, my_local_conn, local_options.cluster_name);
 
@@ -639,7 +706,7 @@ witness_monitor(void)
 								 local_options.master_response_timeout) != 1)
 		return;
 
-	/* Get local xlog info */
+	/* Get timestamp for monitoring update */
 	sqlquery_snprintf(sqlquery, "SELECT CURRENT_TIMESTAMP");
 
 	res = PQexec(my_local_conn, sqlquery);
@@ -692,13 +759,14 @@ static void
 standby_monitor(void)
 {
 	PGresult   *res;
+	char		sqlquery[QUERY_STR_LEN];
+
 	char		monitor_standby_timestamp[MAXLEN];
 	char		last_wal_primary_location[MAXLEN];
 	char		last_xlog_receive_location[MAXLEN];
 	char		last_xlog_replay_location[MAXLEN];
 	char		last_xact_replay_timestamp[MAXLEN];
-	bool		last_xlog_receive_location_gte_replayed;
+	bool		receiving_streamed_wal = true;
-	char		sqlquery[QUERY_STR_LEN];
 
 	XLogRecPtr	lsn_master_current_xlog_location;
 	XLogRecPtr	lsn_last_xlog_receive_location;
@@ -714,12 +782,12 @@ standby_monitor(void)
 	PGconn	   *upstream_conn;
 	char		upstream_conninfo[MAXCONNINFO];
 	int			upstream_node_id;
-	t_node_info upstream_node;
 
 	int			active_master_id;
 	const char *upstream_node_type = NULL;
 
-	bool		receiving_streamed_wal = true;
+
+
 	/*
 	 * Verify that the local node is still available - if not there's
 	 * no point in doing much else anyway
@@ -741,52 +809,91 @@ standby_monitor(void)
 		goto continue_monitoring_standby;
 	}
 
-	upstream_conn = get_upstream_connection(my_local_conn,
+	/*
-											local_options.cluster_name,
+	 * Standby has `failover` set to manual and is disconnected from
-											local_options.node,
+	 * replication following a prior upstream node failure - we'll
-											&upstream_node_id,
+	 * find the master to be able to write monitoring information, if
-											upstream_conninfo);
+	 * required
+	 */
+	if (manual_mode_upstream_disconnected == true)
+	{
+		upstream_conn = get_master_connection(my_local_conn,
+												local_options.cluster_name,
+												&upstream_node_id,
+												upstream_conninfo);
+		upstream_node_type = "master";
+	}
+	else
+	{
+		upstream_conn = get_upstream_connection(my_local_conn,
+												local_options.cluster_name,
+												local_options.node,
+												&upstream_node_id,
+												upstream_conninfo);
 
-	upstream_node_type = (upstream_node_id == master_options.node)
+		upstream_node_type = (upstream_node_id == master_options.node)
-		? "master"
+			? "master"
-		: "upstream";
+			: "upstream";
+	}
 
 	/*
 	 * Check that the upstream node is still available
 	 * If not, initiate failover process
-	 */
+	 *
-
-	check_connection(&upstream_conn, upstream_node_type, upstream_conninfo);
-	/*
 	 * This takes up to local_options.reconnect_attempts *
 	 * local_options.reconnect_interval seconds
 	 */
 
+	check_connection(&upstream_conn, upstream_node_type, upstream_conninfo);
+
 	if (PQstatus(upstream_conn) != CONNECTION_OK)
 	{
+		int previous_master_node_id = master_options.node;
+
 		PQfinish(upstream_conn);
 		upstream_conn = NULL;
 
+		/*
+		 * When `failover=manual`, no actual failover will be performed, instead
+		 * the following happens:
+		 *  - find the new master
+		 *  - create an event notification `standby_disconnect_manual`
+		 *  - set a flag to indicate we're disconnected from replication,
+		 */
 		if (local_options.failover == MANUAL_FAILOVER)
 		{
 			log_err(_("Unable to reconnect to %s. Now checking if another node has been promoted.\n"), upstream_node_type);
 
+			/*
+			 * Set the location string in shared memory to indicate to other
+			 * repmgrd instances that we're *not* a promotion candidate and
+			 * that other repmgrd instance should not expect location updates
+			 * from us
+			 */
+
+			update_shared_memory(PASSIVE_NODE);
+
 			for (connection_retries = 0; connection_retries < local_options.reconnect_attempts; connection_retries++)
 			{
 				master_conn = get_master_connection(my_local_conn,
 					local_options.cluster_name, &master_options.node, NULL);
+
 				if (PQstatus(master_conn) == CONNECTION_OK)
 				{
 					/*
 					 * Connected, we can continue the process so break the
 					 * loop
 					 */
-					log_err(_("connected to node %d, continuing monitoring.\n"),
+					log_notice(_("connected to node %d, continuing monitoring.\n"),
 							master_options.node);
 					break;
 				}
 				else
 				{
+					/*
+					 * XXX this is the only place where `retry_promote_interval_secs`
+					 * is used - this parameter should be renamed or possibly be replaced
+					 */
 					log_err(
 					    _("no new master found, waiting %i seconds before retry...\n"),
 					    local_options.retry_promote_interval_secs
@@ -816,6 +923,36 @@ standby_monitor(void)
 
 				terminate(ERR_DB_CON);
 			}
+
+			/*
+			 * connected to a master - is it the same as the former upstream?
+			 * if not:
+			 *  - create event standby_disconnect
+			 *  - set global "disconnected_manual_standby"
+			 */
+
+			if (previous_master_node_id != master_options.node)
+			{
+				PQExpBufferData errmsg;
+				initPQExpBuffer(&errmsg);
+
+				appendPQExpBuffer(&errmsg,
+								  _("node %i is in manual failover mode and is now disconnected from replication"),
+								  local_options.node);
+
+				log_verbose(LOG_DEBUG, "old master: %i; current: %i\n", previous_master_node_id, master_options.node);
+
+				manual_mode_upstream_disconnected = true;
+
+				create_event_record(master_conn,
+									&local_options,
+									local_options.node,
+									"standby_disconnect_manual",
+									/* here "true" indicates the action has occurred as expected */
+									true,
+									errmsg.data);
+
+			}
 		}
 		else if (local_options.failover == AUTOMATIC_FAILOVER)
 		{
@@ -826,6 +963,8 @@ standby_monitor(void)
 			 * Failover handling is handled differently depending on whether
 			 * the failed node is the master or a cascading standby
 			 */
+			t_node_info upstream_node;
+
 			upstream_node = get_node_info(my_local_conn, local_options.cluster_name, upstream_node_id);
 
 			if (upstream_node.type == MASTER)
@@ -883,8 +1022,8 @@ standby_monitor(void)
 				 *
 				 * We should log a message so the user knows of the situation at hand.
 				 *
-				 * XXX check if the original master is still active and display a
+				 * XXX check if the original master is still active and display a warning
-				 * warning
+				 * XXX add event notification
 				 */
 				log_err(_("It seems this server was promoted manually (not by repmgr) so you might by in the presence of a split-brain.\n"));
 				log_err(_("Check your cluster and manually fix any anomaly.\n"));
@@ -916,8 +1055,8 @@ standby_monitor(void)
 		 * the stream. If we set the local standby node as failed and it's now running
 		 * and receiving replication data, we should activate it again.
 		 */
-	        set_local_node_status();
+		set_local_node_status();
-	        log_info(_("standby connection recovered!\n"));
+		log_info(_("standby connection recovered!\n"));
 	}
 
 	/* Fast path for the case where no history is requested */
@@ -929,8 +1068,6 @@ standby_monitor(void)
 	 * from the upstream node to write monitoring information
 	 */
 
-	upstream_node = get_node_info(my_local_conn, local_options.cluster_name, upstream_node_id);
-
 	sprintf(sqlquery,
 			"SELECT id "
 			"  FROM %s.repl_nodes "
@@ -982,13 +1119,28 @@ standby_monitor(void)
 	if (wait_connection_availability(master_conn, local_options.master_response_timeout) != 1)
 		return;
 
-	/* Get local xlog info */
+	/* Get local xlog info
+	 *
+	 * If receive_location is NULL, we're in archive recovery and not streaming WAL
+	 * If receive_location is less than replay location, we were streaming WAL but are
+	 *   somehow disconnected and evidently in archive recovery
+	 */
 	sqlquery_snprintf(sqlquery,
-					  "SELECT CURRENT_TIMESTAMP, "
+					  " SELECT ts, "
-					  "pg_catalog.pg_last_xlog_receive_location(), "
+					  "        CASE WHEN (receive_location IS NULL OR receive_location < replay_location) "
-					  "pg_catalog.pg_last_xlog_replay_location(), "
+					  "          THEN replay_location "
-					  "pg_catalog.pg_last_xact_replay_timestamp(), "
+					  "          ELSE receive_location"
-					  "pg_catalog.pg_last_xlog_receive_location() >= pg_catalog.pg_last_xlog_replay_location()");
+					  "        END AS receive_location,"
+					  "        replay_location, "
+					  "        replay_timestamp, "
+					  "        COALESCE(receive_location, '0/0') >= replay_location AS receiving_streamed_wal "
+					  "   FROM (SELECT CURRENT_TIMESTAMP AS ts, "
+					  "         pg_catalog.pg_last_xlog_receive_location() AS receive_location, "
+					  "         pg_catalog.pg_last_xlog_replay_location()  AS replay_location, "
+					  "         pg_catalog.pg_last_xact_replay_timestamp() AS replay_timestamp "
+					  "        ) q ");
+
+
 
 	res = PQexec(my_local_conn, sqlquery);
 	if (PQresultStatus(res) != PGRES_TUPLES_OK)
@@ -1004,40 +1156,17 @@ standby_monitor(void)
 	strncpy(last_xlog_replay_location, PQgetvalue(res, 0, 2), MAXLEN);
 	strncpy(last_xact_replay_timestamp, PQgetvalue(res, 0, 3), MAXLEN);
 
-	last_xlog_receive_location_gte_replayed = (strcmp(PQgetvalue(res, 0, 4), "t") == 0)
+	receiving_streamed_wal = (strcmp(PQgetvalue(res, 0, 4), "t") == 0)
 		? true
 		: false;
 
-	/*
+	if (receiving_streamed_wal == false)
-	 * If pg_last_xlog_receive_location is NULL, this means we're in archive
-	 * recovery and will need to calculate lag based on pg_last_xlog_replay_location
-	 */
-
-	/*
-	 * Replayed WAL is greater than received streamed WAL
-	 */
-	if (PQgetisnull(res, 0, 1))
 	{
-		receiving_streamed_wal = false;
+		log_verbose(LOG_DEBUG, _("standby %i not connected to streaming replication"), local_options.node);
 	}
 
 	PQclear(res);
 
-	/*
-	 * In the unusual event of a standby becoming disconnected from the primary,
-	 * while this repmgrd remains connected to the primary,  subtracting
-	 * "last_xlog_replay_location" from "lsn_last_xlog_receive_location" and coercing to
-	 * (long long unsigned int) will result in a meaningless, very large
-	 * value which will overflow a BIGINT column and spew error messages into the
-	 * PostgreSQL log. In the absence of a better strategy, skip attempting
-	 * to insert a monitoring record.
-	 */
-	if (receiving_streamed_wal == true && last_xlog_receive_location_gte_replayed == false)
-	{
-		log_verbose(LOG_WARNING,
-					"Replayed WAL newer than received WAL - is this standby connected to its upstream?\n");
-	}
-
 	/*
 	 * Get master xlog position
 	 *
@@ -1059,23 +1188,9 @@ standby_monitor(void)
 
 	lsn_master_current_xlog_location = lsn_to_xlogrecptr(last_wal_primary_location, NULL);
 	lsn_last_xlog_replay_location = lsn_to_xlogrecptr(last_xlog_replay_location, NULL);
+	lsn_last_xlog_receive_location = lsn_to_xlogrecptr(last_xlog_receive_location, NULL);
 
-	/* Calculate apply lag */
+	apply_lag = (long long unsigned int)lsn_last_xlog_receive_location - lsn_last_xlog_replay_location;
-	if (last_xlog_receive_location_gte_replayed == false)
-	{
-		/*
-		 * We're not receiving streaming WAL - in this case the receive location
-		 * equals the last replayed location
-		 */
-		apply_lag = 0;
-		strncpy(last_xlog_receive_location, last_xlog_replay_location, MAXLEN);
-		lsn_last_xlog_receive_location = lsn_to_xlogrecptr(last_xlog_replay_location, NULL);
-	}
-	else
-	{
-		apply_lag = (long long unsigned int)lsn_last_xlog_receive_location - lsn_last_xlog_replay_location;
-		lsn_last_xlog_receive_location = lsn_to_xlogrecptr(last_xlog_receive_location, NULL);
-	}
 
 	/* Calculate replication lag */
 	if (lsn_master_current_xlog_location >= lsn_last_xlog_receive_location)
@@ -1146,7 +1261,7 @@ do_master_failover(void)
 	PGresult   *res;
 	char		sqlquery[QUERY_STR_LEN];
 
-	int			total_nodes = 0;
+	int			total_active_nodes = 0;
 	int			visible_nodes = 0;
 	int			ready_nodes = 0;
 
@@ -1158,8 +1273,6 @@ do_master_failover(void)
 	XLogRecPtr	xlog_recptr;
 	bool		lsn_format_ok;
 
-	char		last_xlog_replay_location[MAXLEN];
-
 	PGconn	   *node_conn = NULL;
 
 	/*
@@ -1179,7 +1292,7 @@ do_master_failover(void)
 			"SELECT id, conninfo, type, upstream_node_id "
 			"  FROM %s.repl_nodes "
 			" WHERE cluster = '%s' "
-		        "   AND active IS TRUE "
+			"   AND active IS TRUE "
 			"   AND priority > 0 "
 			" ORDER BY priority DESC, id "
 			" LIMIT %i ",
@@ -1195,32 +1308,25 @@ do_master_failover(void)
 		terminate(ERR_DB_QUERY);
 	}
 
-	/*
+	total_active_nodes = PQntuples(res);
-	 * total nodes that are registered
+	log_debug(_("%d active nodes registered\n"), total_active_nodes);
-	 */
-	total_nodes = PQntuples(res);
-	log_debug(_("%d active nodes registered\n"), total_nodes);
 
 	/*
 	 * Build an array with the nodes and indicate which ones are visible and
 	 * ready
 	 */
-	for (i = 0; i < total_nodes; i++)
+	for (i = 0; i < total_active_nodes; i++)
 	{
+		char node_type[MAXLEN];
+
+		nodes[i] = (t_node_info) T_NODE_INFO_INITIALIZER;
+
 		nodes[i].node_id = atoi(PQgetvalue(res, i, 0));
 
 		strncpy(nodes[i].conninfo_str, PQgetvalue(res, i, 1), MAXCONNINFO);
+		strncpy(node_type, PQgetvalue(res, i, 2), MAXLEN);
 
-		nodes[i].type = parse_node_type(PQgetvalue(res, i, 2));
+		nodes[i].type = parse_node_type(node_type);
-
-		/* Copy details of the failed node */
-		/* XXX only node_id is actually used later */
-		if (nodes[i].type == MASTER)
-		{
-			failed_master.node_id = nodes[i].node_id;
-			failed_master.xlog_location = nodes[i].xlog_location;
-			failed_master.is_ready = nodes[i].is_ready;
-		}
 
 		nodes[i].upstream_node_id = atoi(PQgetvalue(res, i, 3));
 
@@ -1231,11 +1337,42 @@ do_master_failover(void)
 		nodes[i].is_visible = false;
 		nodes[i].is_ready = false;
 
-		nodes[i].xlog_location = InvalidXLogRecPtr;
+		log_debug(_("node=%i conninfo=\"%s\" type=%s\n"),
+				  nodes[i].node_id,
+				  nodes[i].conninfo_str,
+				  node_type);
 
-		log_debug(_("node=%d conninfo=\"%s\" type=%s\n"),
+		/* Copy details of the failed master node */
-				  nodes[i].node_id, nodes[i].conninfo_str,
+		if (nodes[i].type == MASTER)
-				  PQgetvalue(res, i, 2));
+		{
+			/* XXX only node_id is currently used */
+			failed_master.node_id = nodes[i].node_id;
+
+			/*
+			 * XXX experimental
+			 *
+			 * Currently an attempt is made to connect to the master,
+			 * which is very likely to be a waste of time at this point, as we'll
+			 * have spent the last however many seconds trying to do just that
+			 * in check_connection() before deciding it's gone away.
+			 *
+			 * If the master did come back at this point, the voting algorithm should decide
+			 * it's the "best candidate" anyway and no standby will promote itself or
+			 * attempt to follow* another server.
+			 *
+			 * If we don't try and connect to the master here (and the code generally
+			 * assumes it's failed anyway) but it does come back any time from here
+			 * onwards, promotion will fail and the promotion candidate will
+			 * notice the reappearance.
+			 *
+			 * TLDR version: by skipping the master connection attempt (and the chances
+			 * the master would reappear between the last attempt in check_connection()
+			 * and now are minimal) we can remove useless cycles during the failover process;
+			 * if the master does reappear it will be caught before later anyway.
+			 */
+
+			continue;
+		}
 
 		node_conn = establish_db_connection(nodes[i].conninfo_str, false);
 
@@ -1256,13 +1393,13 @@ do_master_failover(void)
 	PQclear(res);
 
 	log_debug(_("total nodes counted: registered=%d, visible=%d\n"),
-			  total_nodes, visible_nodes);
+			  total_active_nodes, visible_nodes);
 
 	/*
 	 * Am I on the group that should keep alive? If I see less than half of
-	 * total_nodes then I should do nothing
+	 * total_active_nodes then I should do nothing
 	 */
-	if (visible_nodes < (total_nodes / 2.0))
+	if (visible_nodes < (total_active_nodes / 2.0))
 	{
 		log_err(_("Unable to reach most of the nodes.\n"
 				  "Let the other standby servers decide which one will be the master.\n"
@@ -1271,7 +1408,7 @@ do_master_failover(void)
 	}
 
 	/* Query all available nodes to determine readiness and LSN */
-	for (i = 0; i < total_nodes; i++)
+	for (i = 0; i < total_active_nodes; i++)
 	{
 		log_debug("checking node %i...\n", nodes[i].node_id);
 
@@ -1340,8 +1477,8 @@ do_master_failover(void)
 				  " considered as new master and exit.\n"),
 				PQerrorMessage(my_local_conn));
 		PQclear(res);
-		sprintf(last_xlog_replay_location, "'%X/%X'", 0, 0);
+
-		update_shared_memory(last_xlog_replay_location);
+		update_shared_memory(LSN_QUERY_ERROR);
 		terminate(ERR_DB_QUERY);
 	}
 	/* write last location in shared memory */
@@ -1349,7 +1486,7 @@ do_master_failover(void)
 	PQclear(res);
 
 	/* Wait for each node to come up and report a valid LSN */
-	for (i = 0; i < total_nodes; i++)
+	for (i = 0; i < total_active_nodes; i++)
 	{
 		/*
 		 * ensure witness server is marked as ready, and skip
@@ -1382,7 +1519,6 @@ do_master_failover(void)
 		 */
 		if (PQstatus(node_conn) != CONNECTION_OK)
 		{
-			/* XXX */
 			log_info(_("At this point, it could be some race conditions "
 					   "that are acceptable, assume the node is restarting "
 					   "and starting failover procedure\n"));
@@ -1391,6 +1527,7 @@ do_master_failover(void)
 
 		while (!nodes[i].is_ready)
 		{
+			char location_value[MAXLEN];
 
 			sqlquery_snprintf(sqlquery,
 							  "SELECT %s.repmgr_get_last_standby_location()",
@@ -1406,7 +1543,11 @@ do_master_failover(void)
 				terminate(ERR_DB_QUERY);
 			}
 
-			xlog_recptr = lsn_to_xlogrecptr(PQgetvalue(res, 0, 0), &lsn_format_ok);
+			/* Copy the returned value as we'll need to reference it a few times */
+			strncpy(location_value, PQgetvalue(res, 0, 0), MAXLEN);
+			PQclear(res);
+
+			xlog_recptr = lsn_to_xlogrecptr(location_value, &lsn_format_ok);
 
 			/* If position reported as "invalid", check for format error or
 			 * empty string; otherwise position is 0/0 and we need to continue
@@ -1414,10 +1555,36 @@ do_master_failover(void)
 			 */
 			if (xlog_recptr == InvalidXLogRecPtr)
 			{
+				bool continue_loop = true;
+
 				if (lsn_format_ok == false)
 				{
+
+					/*
+					 * The node is indicating it is not a promotion candidate -
+					 * in this case we can store its invalid LSN to ensure it
+					 * can't be a promotion candidate when comparing locations
+					 */
+					if (strcmp(location_value, PASSIVE_NODE) == 0)
+					{
+						log_debug("node %i is passive mode\n", nodes[i].node_id);
+						log_info(_("node %i will not be considered for promotion\n"), nodes[i].node_id);
+						nodes[i].xlog_location = InvalidXLogRecPtr;
+						continue_loop = false;
+					}
+					/*
+					 * This should probably never happen but if it does, rule the
+					 * node out as a promotion candidate
+					 */
+					else if (strcmp(location_value, LSN_QUERY_ERROR) == 0)
+					{
+						log_warning(_("node %i is unable to update its shared memory and will not be considered for promotion\n"), nodes[i].node_id);
+						nodes[i].xlog_location = InvalidXLogRecPtr;
+						continue_loop = false;
+					}
+
 					/* Unable to parse value returned by `repmgr_get_last_standby_location()` */
-					if (*PQgetvalue(res, 0, 0) == '\0')
+					else if (*location_value == '\0')
 					{
 						log_crit(
 							_("unable to obtain LSN from node %i"), nodes[i].node_id
@@ -1426,8 +1593,8 @@ do_master_failover(void)
 							_("please check that 'shared_preload_libraries=repmgr_funcs' is set in postgresql.conf\n")
 							);
 
-						PQclear(res);
 						PQfinish(node_conn);
+						/* XXX shouldn't we just ignore this node? */
 						exit(ERR_BAD_CONFIG);
 					}
 
@@ -1435,25 +1602,29 @@ do_master_failover(void)
 					 * Very unlikely to happen; in the absence of any better
 					 * strategy keep checking
 					 */
-					log_warning(_("unable to parse LSN \"%s\"\n"),
+					else {
-								PQgetvalue(res, 0, 0));
+						log_warning(_("unable to parse LSN \"%s\"\n"),
+									location_value);
+					}
 				}
 				else
 				{
 					log_debug(
 						_("invalid LSN returned from node %i: '%s'\n"),
 						nodes[i].node_id,
-						PQgetvalue(res, 0, 0)
+						location_value);
-						);
 				}
 
-				PQclear(res);
+				/*
-
+				 * If the node is still reporting an InvalidXLogRecPtr, it means
-				/* If position is 0/0, keep checking */
+				 * its repmgrd hasn't yet had time to update it (either with a valid
-				/* XXX we should add a timeout here to prevent infinite looping
+				 * XLogRecPtr or a message) so we continue looping.
+				 *
+				 * XXX we should add a timeout here to prevent infinite looping
 				 * if the other node's repmgrd is not up
 				 */
-				continue;
+				if (continue_loop == true)
+					continue;
 			}
 
 			if (nodes[i].xlog_location < xlog_recptr)
@@ -1461,8 +1632,7 @@ do_master_failover(void)
 				nodes[i].xlog_location = xlog_recptr;
 			}
 
-			log_debug(_("LSN of node %i is: %s\n"), nodes[i].node_id, PQgetvalue(res, 0, 0));
+			log_debug(_("LSN of node %i is: %s\n"), nodes[i].node_id, location_value);
-			PQclear(res);
 
 			ready_nodes++;
 			nodes[i].is_ready = true;
@@ -1475,7 +1645,7 @@ do_master_failover(void)
 	/*
 	 * determine which one is the best candidate to promote to master
 	 */
-	for (i = 0; i < total_nodes; i++)
+	for (i = 0; i < total_active_nodes; i++)
 	{
 		/* witness server can never be a candidate */
 		if (nodes[i].type == WITNESS)
@@ -1564,6 +1734,8 @@ do_master_failover(void)
 				{
 					log_notice(_("Original master reappeared before this standby was promoted - no action taken\n"));
 
+					/* XXX log an event here?  */
+
 					PQfinish(master_conn);
 					master_conn = NULL;
 
@@ -1700,8 +1872,10 @@ do_master_failover(void)
 		termPQExpBuffer(&event_details);
 	}
 
-	/* to force it to re-calculate mode and master node */
+	/*
-	// ^ ZZZ check that behaviour ^
+	 * setting "failover_done" to true will cause the node's monitoring loop
+	 * to restart in the appropriate mode for the node's (possibly new) role
+	 */
 	failover_done = true;
 }
 
@@ -1877,7 +2051,7 @@ check_connection(PGconn **conn, const char *type, const char *conninfo)
 		{
 			if (conninfo == NULL)
 			{
-				log_err("INTERNAL ERROR: *conn == NULL && conninfo == NULL");
+				log_err("INTERNAL ERROR: *conn == NULL && conninfo == NULL\n");
 				terminate(ERR_INTERNAL);
 			}
 			*conn = establish_db_connection(conninfo, false);
@@ -1917,18 +2091,21 @@ check_connection(PGconn **conn, const char *type, const char *conninfo)
 /*
  * set_local_node_status()
  *
- * If failure of the local node is detected, attempt to connect
+ * Attempt to connect to the current master server (as stored in the global
- * to the current master server (as stored in the global variable
+ * variable `master_conn`) and set the local node's status to the result
- * `master_conn`) and update its record to failed.
+ * of `is_standby(my_local_conn)`. Normally this will be used to mark
+ * a node as failed, but in some circumstances we may be marking it
+ * as recovered.
  */
 
 static bool
 set_local_node_status(void)
 {
-	PGresult       *res;
+	PGresult   *res;
 	char		sqlquery[QUERY_STR_LEN];
-	int		active_master_node_id = NODE_NOT_FOUND;
+	int			active_master_node_id = NODE_NOT_FOUND;
 	char		master_conninfo[MAXLEN];
+	bool		local_node_status;
 
 	if (!check_connection(&master_conn, "master", NULL))
 	{
@@ -1987,24 +2164,29 @@ set_local_node_status(void)
 
 	/*
 	 * Attempt to set the active record to the correct value.
-	 * First
 	 */
 
+	local_node_status = (is_standby(my_local_conn) == 1);
+
 	if (!update_node_record_status(master_conn,
 					    local_options.cluster_name,
 					    node_info.node_id,
 					    "standby",
 					    node_info.upstream_node_id,
-					    is_standby(my_local_conn)==1))
+					    local_node_status))
 	{
-		log_err(_("unable to set local node %i as inactive on master: %s\n"),
+		log_err(_("unable to set local node %i as %s on master: %s\n"),
 				node_info.node_id,
+				local_node_status == false ? "inactive" : "active",
 				PQerrorMessage(master_conn));
 
 		return false;
 	}
 
-	log_notice(_("marking this node (%i) as inactive on master\n"), node_info.node_id);
+	log_notice(_("marking this node (%i) as %s on master\n"),
+			   node_info.node_id,
+			   local_node_status == false ? "inactive" : "active");
+
 	return true;
 }
 
@@ -2138,7 +2320,7 @@ lsn_to_xlogrecptr(char *lsn, bool *format_ok)
 	{
 		if (format_ok != NULL)
 			*format_ok = false;
-		log_err(_("incorrect log location format: %s\n"), lsn);
+		log_warning(_("incorrect log location format: %s\n"), lsn);
 		return 0;
 	}
 

strutil.c

@@ -87,3 +87,21 @@ maxlen_snprintf(char *str, const char *format,...)
 
 	return retval;
 }
+
+
+/*
+ * Escape a string for use as a parameter in recovery.conf
+ * Caller must free returned value
+ */
+char *
+escape_recovery_conf_value(const char *src)
+{
+	char	   *result = escape_single_quotes_ascii(src);
+
+	if (!result)
+	{
+		fprintf(stderr, _("%s: out of memory\n"), progname());
+		exit(ERR_INTERNAL);
+	}
+	return result;
+}

strutil.h

@@ -22,6 +22,7 @@
 #define _STRUTIL_H_
 
 #include <stdlib.h>
+#include "pqexpbuffer.h"
 #include "errcode.h"
 
 
@@ -48,4 +49,6 @@ extern int
 maxlen_snprintf(char *str, const char *format,...)
 __attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 3)));
 
+extern char *
+escape_recovery_conf_value(const char *src);
 #endif   /* _STRUTIL_H_ */

version.h

@@ -1,6 +1,6 @@
 #ifndef _VERSION_H_
 #define _VERSION_H_
 
-#define REPMGR_VERSION "3.2dev"
+#define REPMGR_VERSION "3.3"
 
 #endif