From 9523894808ce83b16fba2455c786f56ecc6ae6f7 Mon Sep 17 00:00:00 2001
From: Ian Barwick <ian@2ndquadrant.com>
Date: Fri, 30 Sep 2016 15:14:35 +0900
Subject: [PATCH 01/23] Bump dev version number

3.3dev
---
 version.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/version.h b/version.h
index 78de4b4f..c49452f3 100644
--- a/version.h
+++ b/version.h
@@ -1,6 +1,6 @@
 #ifndef _VERSION_H_
 #define _VERSION_H_
 
-#define REPMGR_VERSION "3.2dev"
+#define REPMGR_VERSION "3.3dev"
 
 #endif

From 84595fe71172e11dfa5c1d43febdf77b38dbc08d Mon Sep 17 00:00:00 2001
From: Ian Barwick <ian@2ndquadrant.com>
Date: Mon, 3 Oct 2016 15:57:33 +0900
Subject: [PATCH 02/23] Tweak repmgr.conf.sample

Put `monitor_interval_secs` at the start of the `repmgrd` section, as it's
a very fundamental configuration item.
---
 repmgr.conf.sample | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/repmgr.conf.sample b/repmgr.conf.sample
index b3dc3d06..20215b3d 100644
--- a/repmgr.conf.sample
+++ b/repmgr.conf.sample
@@ -160,6 +160,9 @@
 # These settings are only applied when repmgrd is running. Values shown
 # are defaults.
 
+# monitoring interval in seconds; default is 2
+#monitor_interval_secs=2
+
 # Number of seconds to wait for a response from the primary server before
 # deciding it has failed.
 
@@ -187,9 +190,6 @@
 #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'

From ba89758366ad0460ad817287866606c1c628b972 Mon Sep 17 00:00:00 2001
From: Ian Barwick <ian@2ndquadrant.com>
Date: Mon, 3 Oct 2016 15:59:03 +0900
Subject: [PATCH 03/23] Update `barman-wal-restore` documentation

Barman 2.0 provides this in a separate, more convenient `barman-cli` package;
document this and add note about previous `barman-wal-restore.py` script.
---
 README.md | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/README.md b/README.md
index 69478a50..31225880 100644
--- a/README.md
+++ b/README.md
@@ -599,8 +599,8 @@ ensure that:
 - the `barman_server` setting in `repmgr.conf` is set to the SSH
   hostname of the Barman server;
 - the `restore_command` setting in `repmgr.conf` is configured to
-  use a copy of the `barman-wal-restore.py` script shipped with Barman
-  (see below);
+  use a copy of the `barman-wal-restore` script shipped with the
+  `barman-cli package` (see below);
 - the Barman catalogue includes at least one valid backup for this
   server.
 
@@ -616,26 +616,24 @@ ensure that:
 > corresponding to the value of `barman_server` in `repmgr.conf`. See
 > the "Host" section in `man 5 ssh_config` for more details.
 
-`barman-wal-restore.py` is a Python script provided by the Barman
-development team, which must be copied in a location accessible to
-`repmgr`, and marked as executable; `restore_command` must then be
-set in `repmgr.conf` as follows:
+`barman-wal-restore` is a Python script provided by the Barman
+development team as part of the `barman-cli` package (Barman 2.0
+and later; for Barman 1.x the script is provided separately as
+`barman-wal-restore.py`).
+
+`restore_command` must then be set in `repmgr.conf` as follows:
 
     <script> <Barman hostname> <cluster_name> %f %p
 
 For instance, suppose that we have installed Barman on the `barmansrv`
-host, and that we have placed a copy of `barman-wal-restore.py` into
-the `/usr/local/bin` directory. First, we ensure that the script is
-executable:
-
-    sudo chmod +x /usr/local/bin/barman-wal-restore.py
-
-Then we check that `repmgr.conf` includes the following lines:
+host, and that `barman-wal-restore` is located as an executable at
+`/usr/bin/barman-wal-restore`;  `repmgr.conf` should include the following
+lines:
 
     barman_server=barmansrv
-    restore_command=/usr/local/bin/barman-wal-restore.py barmansrv test %f %p
+    restore_command=/usr/bin/barman-wal-restore barmansrv test %f %p
 
-To use a non-default Barman configuration file on the Barman server,
+NOTE: to use a non-default Barman configuration file on the Barman server,
 specify this in `repmgr.conf` with `barman_config`:
 
     barman_config=/path/to/barman.conf
@@ -688,7 +686,7 @@ and destination server as the contents of files existing on both servers need
 to be compared, meaning this method is not necessarily faster than making a
 fresh clone with `pg_basebackup`.
 
-> *NOTE*: `barman-wal-restore.py` supports command line switches to
+> *NOTE*: `barman-wal-restore` supports command line switches to
 > control parallelism (`--parallel=N`) and compression (`--bzip2`,
 > `--gzip`).
 

From eb90f864c9e8e593fe1dfea33ecb0d47b295a745 Mon Sep 17 00:00:00 2001
From: Ian Barwick <ian@2ndquadrant.com>
Date: Wed, 5 Oct 2016 10:31:25 +0900
Subject: [PATCH 04/23] repmgr: consistent error message style

---
 repmgr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/repmgr.c b/repmgr.c
index e16e0e1d..85a83861 100644
--- a/repmgr.c
+++ b/repmgr.c
@@ -6119,7 +6119,7 @@ do_witness_unregister(void)
 										NULL, NULL);
 	if (PQstatus(master_conn) != CONNECTION_OK)
 	{
-		log_err(_("Unable to connect to master server\n"));
+		log_err(_("unable to connect to master server\n"));
 		exit(ERR_BAD_CONFIG);
 	}
 

From 2fae788bc4baaeeaef36aaa03528a4ca509eb5ad Mon Sep 17 00:00:00 2001
From: Ian Barwick <ian@2ndquadrant.com>
Date: Wed, 5 Oct 2016 11:25:36 +0900
Subject: [PATCH 05/23] Add documentation for repmgrd failover process and
 failed node fencing

Addresses GitHub #200.
---
 README.md                          |  21 +++-
 docs/repmgrd-failover-mechanism.md |  75 +++++++++++++++
 docs/repmgrd-node-fencing.md       | 150 +++++++++++++++++++++++++++++
 3 files changed, 243 insertions(+), 3 deletions(-)
 create mode 100644 docs/repmgrd-failover-mechanism.md
 create mode 100644 docs/repmgrd-node-fencing.md

diff --git a/README.md b/README.md
index 31225880..8c236c8a 100644
--- a/README.md
+++ b/README.md
@@ -580,13 +580,13 @@ base backups and WAL files.
 
 Barman support provides the following advantages:
 
-- the primary node does not need to perform a new backup every time a
+- the master node does not need to perform a new backup every time a
   new standby is cloned;
 - a standby node can be disconnected for longer periods without losing
   the ability to catch up, and without causing accumulation of WAL
-  files on the primary node;
+  files on the master node;
 - therefore, `repmgr` does not need to use replication slots, and the
-  primary node does not need to set `wal_keep_segments`.
+  master node does not need to set `wal_keep_segments`.
 
 > *NOTE*: In view of the above, Barman support is incompatible with
 > the `use_replication_slots` setting in `repmgr.conf`.
@@ -1743,6 +1743,21 @@ which contains connection details for the local database.
     the current working directory; no additional arguments are required.
 
 
+### Further documentation
+
+As well as this README, the `repmgr` source contains following additional
+documentation files:
+
+* FAQ.md - frequently asked questions
+* CONTRIBUTING.md - how to contribute to `repmgr`
+* PACKAGES.md - details on building packages
+* SSH-RSYNC.md - how to set up passwordless SSH between nodes
+* docs/repmgrd-failover-mechanism.md - how repmgrd picks which node to promote
+* docs/repmgrd-node-fencing.md - how to "fence" a failed master node
+
+
+
+
 ### Error codes
 
 `repmgr` or `repmgrd` will return one of the following error codes on program
diff --git a/docs/repmgrd-failover-mechanism.md b/docs/repmgrd-failover-mechanism.md
new file mode 100644
index 00000000..b6d013ae
--- /dev/null
+++ b/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.
+
+
+
+
diff --git a/docs/repmgrd-node-fencing.md b/docs/repmgrd-node-fencing.md
new file mode 100644
index 00000000..ecfd13c4
--- /dev/null
+++ b/docs/repmgrd-node-fencing.md
@@ -0,0 +1,150 @@
+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
+file on all nodes, assumed to be at `/etc/pgbouncer.ini`.
+
+The script also requires a template file containing global `pgbouncer`
+configuration, which should looks something like this (adjust
+settings appropriately for your environment):
+
+`/var/lib/postgres/repmgr/pgbouncer.ini.template`
+
+    [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
+
+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"
+    REPMGR_DB="repmgr"
+    REPMGR_USER="repmgr"
+    REPMGR_SCHEMA="repmgr_test"
+    PGBOUNCER_CONFIG="/etc/pgbouncer.ini"
+    PGBOUNCER_INI_TEMPLATE="/var/lib/postgres/repmgr/pgbouncer.ini.template"
+    PGBOUNCER_DATABASE="appdb"
+
+    # 1. Pause running pgbouncer instances
+    for HOST in $PGBOUNCER_HOSTS
+    do
+        psql -t -c "pause" -h $HOST -p $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_INI_NEW="/tmp/pgbouncer.ini.new"
+
+    for HOST in $PGBOUNCER_HOSTS
+    do
+        # Recreate the pgbouncer config file
+        echo -e "[databases]\n" > $PGBOUNCER_INI_NEW
+
+        psql -d $REPMGR_DB -U $REPMGR_USER -t -A \
+          -c "SELECT '$PGBOUNCER_DATABASE= ' || conninfo || ' application_name=pgbouncer_$HOST' \
+              FROM $REPMGR_SCHEMA.repl_nodes \
+              WHERE active = TRUE AND type='master'" >> $PGBOUNCER_INI_NEW
+
+        cat $PGBOUNCER_INI_TEMPLATE >> $PGBOUNCER_INI_NEW
+
+        rsync $PGBOUNCER_INI_NEW $HOST:$PGBOUNCER_CONFIG
+
+        psql -tc "reload" -h $HOST -U postgres pgbouncer
+        psql -tc "resume" -h $HOST -U postgres pgbouncer
+
+    done
+
+    # Clean up generated file
+    rm $PGBOUNCER_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.

From 09ac6cd145907d25161845400d7ea65614b5374b Mon Sep 17 00:00:00 2001
From: Ian Barwick <ian@2ndquadrant.com>
Date: Wed, 5 Oct 2016 13:57:10 +0900
Subject: [PATCH 06/23] Update history

---
 HISTORY | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/HISTORY b/HISTORY
index 1aa4a530..cf975f44 100644
--- a/HISTORY
+++ b/HISTORY
@@ -1,4 +1,4 @@
-3.2     2016-
+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`
@@ -16,9 +16,15 @@
         repmgr: add option `--copy-external-config-files` for files outside
            of the data directory (Ian)
         repmgr: add configuration options to override the default pg_ctl
-           commands (Jarkko Oranen)
+           commands (Jarkko Oranen, 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: 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

From 1150bf272a63aa548c22dc7511f2b5bb681d87e6 Mon Sep 17 00:00:00 2001
From: Ian Barwick <ian@2ndquadrant.com>
Date: Wed, 5 Oct 2016 15:09:07 +0900
Subject: [PATCH 07/23] Update README

`--ignore-external-config-files` deprecated
---
 README.md | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/README.md b/README.md
index 8c236c8a..4ba6ed04 100644
--- a/README.md
+++ b/README.md
@@ -7,7 +7,7 @@ replication capabilities with utilities to set up standby servers, monitor
 replication, and perform administrative tasks such as failover or switchover
 operations.
 
-The current `repmgr` version, 3.1.5, supports all PostgreSQL versions from
+The current `repmgr` version, 3.2, supports all PostgreSQL versions from
 9.3, including the upcoming 9.6.
 
 Overview
@@ -690,20 +690,6 @@ fresh clone with `pg_basebackup`.
 > control parallelism (`--parallel=N`) and compression (`--bzip2`,
 > `--gzip`).
 
-### Dealing with PostgreSQL configuration files
-
-By default, `repmgr` will attempt to copy the standard configuration files
-(`postgresql.conf`, `pg_hba.conf` and `pg_ident.conf`) even if they are located
-outside of the data directory (though currently they will be copied
-into the standby's data directory). To prevent this happening, when executing
-`repmgr standby clone` provide the `--ignore-external-config-files` option.
-
-If using `rsync` to clone a standby, additional control over which files
-not to transfer is possible by configuring `rsync_options` in `repmgr.conf`,
-which enables any valid `rsync` options to be passed to that command, e.g.:
-
-    rsync_options='--exclude=postgresql.local.conf'
-
 ### Controlling `primary_conninfo` in `recovery.conf`
 
 The `primary_conninfo` setting in `recovery.conf` generated by `repmgr`

From b509ce63829ecdb6b06d535574bf87c996819b8c Mon Sep 17 00:00:00 2001
From: Ian Barwick <ian@2ndquadrant.com>
Date: Wed, 5 Oct 2016 16:47:37 +0900
Subject: [PATCH 08/23] Minor README fix

---
 README.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/README.md b/README.md
index 4ba6ed04..6618c5d8 100644
--- a/README.md
+++ b/README.md
@@ -1012,7 +1012,7 @@ should have been updated to reflect this:
   at a two-server master/standby replication cluster and currently does
   not support additional standbys.
 - `repmgr standby switchover` is designed to use the `pg_rewind` utility,
-  standard in 9.5 and later and available for separately in 9.3 and 9.4
+  standard in 9.5 and later and available separately in 9.3 and 9.4
   (see note below)
 - `pg_rewind` *requires* that either `wal_log_hints` is enabled, or that
    data checksums were enabled when the cluster was initialized. See the

From 2dcb75f889e390888eeabc60984f4e1a3b697632 Mon Sep 17 00:00:00 2001
From: Ian Barwick <ian@2ndquadrant.com>
Date: Thu, 6 Oct 2016 07:38:45 +0900
Subject: [PATCH 09/23] Add 'cluster crosscheck' to help output detail

Per GitHub #243.
---
 repmgr.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/repmgr.c b/repmgr.c
index 85a83861..e3dd9707 100644
--- a/repmgr.c
+++ b/repmgr.c
@@ -6254,7 +6254,10 @@ do_help(void)
 	printf(_(" witness register      - registers a witness server\n"));
 	printf(_(" witness unregister    - unregisters a witness server\n"));
 	printf(_(" cluster show          - displays information about cluster nodes\n"));
-	printf(_(" cluster matrix        - displays the cluster's connection matrix\n"));
+	printf(_(" cluster matrix        - displays the cluster's connection matrix\n" \
+	         "                           as seen from the current node\n"));
+	printf(_(" cluster crosscheck    - displays the cluster's connection matrix\n" \
+	         "                           as seen from all nodes\n"));
 	printf(_(" cluster cleanup       - prunes or truncates monitoring history\n" \
 			 "                         (monitoring history creation requires repmgrd\n" \
 			 "                         with --monitoring-history option)\n"));

From a217b4d0a9d101f72fa26286e6f4eb5f22f6db3a Mon Sep 17 00:00:00 2001
From: Ian Barwick <barwick@gmail.com>
Date: Fri, 7 Oct 2016 07:42:15 +0900
Subject: [PATCH 10/23] repmgr: standardize SSH-related error messages

---
 repmgr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/repmgr.c b/repmgr.c
index e3dd9707..a473a6f6 100644
--- a/repmgr.c
+++ b/repmgr.c
@@ -2512,7 +2512,7 @@ do_standby_clone(void)
 		r = test_ssh_connection(runtime_options.host, runtime_options.remote_user);
 		if (r != 0)
 		{
-			log_err(_("aborting, remote host %s is not reachable.\n"),
+			log_err(_("aborting, remote host %s is not reachable via SSH.\n"),
 					runtime_options.host);
 			exit(ERR_BAD_SSH);
 		}
@@ -4610,7 +4610,7 @@ do_standby_switchover(void)
 
 	if (r != 0)
 	{
-		log_err(_("unable to connect via ssh to host %s, user %s\n"), remote_host, runtime_options.remote_user);
+		log_err(_("unable to connect via SSH to host %s, user %s\n"), remote_host, runtime_options.remote_user);
 	}
 
 	if (get_pg_setting(remote_conn, "data_directory", remote_data_directory) == false)

From f435abb3ecd3b166c469204d42f65d8b6b99e745 Mon Sep 17 00:00:00 2001
From: Ian Barwick <ian@2ndquadrant.com>
Date: Mon, 10 Oct 2016 15:10:06 +0900
Subject: [PATCH 11/23] Update README

Also reorder HISTORY entries.
---
 HISTORY   | 4 ++--
 README.md | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/HISTORY b/HISTORY
index cf975f44..7eae480c 100644
--- a/HISTORY
+++ b/HISTORY
@@ -15,12 +15,12 @@
            the standby (Ian)
         repmgr: add option `--copy-external-config-files` for files outside
            of the data directory (Ian)
-        repmgr: add configuration options to override the default pg_ctl
-           commands (Jarkko Oranen, 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)
 
diff --git a/README.md b/README.md
index 6618c5d8..31149909 100644
--- a/README.md
+++ b/README.md
@@ -8,7 +8,7 @@ replication, and perform administrative tasks such as failover or switchover
 operations.
 
 The current `repmgr` version, 3.2, supports all PostgreSQL versions from
-9.3, including the upcoming 9.6.
+9.3 to 9.6.
 
 Overview
 --------
@@ -600,7 +600,7 @@ ensure that:
   hostname of the Barman server;
 - the `restore_command` setting in `repmgr.conf` is configured to
   use a copy of the `barman-wal-restore` script shipped with the
-  `barman-cli package` (see below);
+  `barman-cli` package (see below);
 - the Barman catalogue includes at least one valid backup for this
   server.
 

From 871ec47ff574068fbe41ca381f01537a782746e8 Mon Sep 17 00:00:00 2001
From: Ian Barwick <ian@2ndquadrant.com>
Date: Mon, 10 Oct 2016 16:20:17 +0900
Subject: [PATCH 12/23] Fix `repmgr cluster crosscheck` output

Show actual node ID rather than incremental number.

Fixes GitHub #244.
---
 repmgr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/repmgr.c b/repmgr.c
index a473a6f6..13481412 100644
--- a/repmgr.c
+++ b/repmgr.c
@@ -1646,7 +1646,7 @@ do_cluster_crosscheck(void)
 
 	printf("%*s | Id ", name_length, node_header);
 	for (i = 0; i < n; i++)
-		printf("| %2d ", i+1);
+		printf("| %2d ", cube[i]->node_id);
 	printf("\n");
 
 	for (i = 0; i < name_length; i++)

From 502c0567534c80fed27b4ef213c60bfcb720499c Mon Sep 17 00:00:00 2001
From: Gianni Ciolli <gianni.ciolli@2ndQuadrant.it>
Date: Fri, 7 Oct 2016 19:31:02 +0200
Subject: [PATCH 13/23] When restoring from Barman, create pg_logical
 subdirectories

Nodes cloned from Barman backups were missing these subdirectories,
and so they were unable to start when promoted.
---
 repmgr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/repmgr.c b/repmgr.c
index 13481412..eb08fbc6 100644
--- a/repmgr.c
+++ b/repmgr.c
@@ -3315,14 +3315,14 @@ do_standby_clone(void)
 					/* Only from 9.5 */
 					"pg_commit_ts",
 					/* Only from 9.4 */
-					"pg_dynshmem", "pg_logical", "pg_replslot",
+					"pg_dynshmem", "pg_logical", "pg_logical/snapshot", "pg_logical/mappings", "pg_replslot",
 					/* Already in 9.3 */
 					"pg_serial", "pg_snapshots", "pg_stat", "pg_stat_tmp", "pg_tblspc",
 					"pg_twophase", "pg_xlog", 0
 				};
 				const int vers[] = {
 					90500,
-					90400, 90400, 90400,
+					90400, 90400, 90400, 90400, 90400,
 					0, 0, 0, 0, 0,
 					0, 0, 0
 				};

From e4cb6d71302da5c2f895169d43ac4a86370c36be Mon Sep 17 00:00:00 2001
From: Ian Barwick <barwick@gmail.com>
Date: Mon, 10 Oct 2016 22:56:49 +0900
Subject: [PATCH 14/23] repmgr: simplify LSN parsing

Also silences a compiler warning.
---
 repmgr.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/repmgr.c b/repmgr.c
index 13481412..83a01eca 100644
--- a/repmgr.c
+++ b/repmgr.c
@@ -3900,29 +3900,31 @@ stop_backup:
 	exit(retval);
 }
 
-static bool
+static void
 parse_lsn(XLogRecPtr *ptr, const char *str)
 {
 	uint32 high, low;
 
 	if (sscanf(str, "%x/%x", &high, &low) != 2)
-		return false;
+		return;
 
 	*ptr = (((XLogRecPtr)high) << 32) + (XLogRecPtr)low;
 
-	return true;
+	return;
 }
 
 static XLogRecPtr
 parse_label_lsn(const char *label_key, const char *label_value)
 {
-	XLogRecPtr ptr;
+	XLogRecPtr ptr = InvalidXLogRecPtr;
 
-	if (!parse_lsn(&ptr, label_value))
+	parse_lsn(&ptr, label_value);
+
+	/* parse_lsn() will not modify ptr if it can't parse the label value */
+	if (ptr == InvalidXLogRecPtr)
 	{
 		log_err(_("Couldn't parse backup label entry \"%s: %s\" as lsn"),
 				label_key, label_value);
-		return InvalidXLogRecPtr;
 	}
 
 	return ptr;

From a279c42df9c943b03e5ef67f12c77a55343b61ab Mon Sep 17 00:00:00 2001
From: Ian Barwick <ian@2ndquadrant.com>
Date: Tue, 11 Oct 2016 09:32:55 +0900
Subject: [PATCH 15/23] "pg_logical/snapshot" -> "pg_logical/snapshots"

---
 repmgr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/repmgr.c b/repmgr.c
index cd75d17b..2e94c4a5 100644
--- a/repmgr.c
+++ b/repmgr.c
@@ -3315,7 +3315,7 @@ do_standby_clone(void)
 					/* Only from 9.5 */
 					"pg_commit_ts",
 					/* Only from 9.4 */
-					"pg_dynshmem", "pg_logical", "pg_logical/snapshot", "pg_logical/mappings", "pg_replslot",
+					"pg_dynshmem", "pg_logical", "pg_logical/snapshots", "pg_logical/mappings", "pg_replslot",
 					/* Already in 9.3 */
 					"pg_serial", "pg_snapshots", "pg_stat", "pg_stat_tmp", "pg_tblspc",
 					"pg_twophase", "pg_xlog", 0

From 50119056a5481bf9ec339fd2a923455b90ff137a Mon Sep 17 00:00:00 2001
From: Ian Barwick <ian@2ndquadrant.com>
Date: Tue, 11 Oct 2016 12:54:56 +0900
Subject: [PATCH 16/23] repmgr: clean up runtime options structure

Place elements in a sensible order and split the associated initializer
macro over multiple lines for easier editing.

Also move a few related global variables into to the structure to keep
everything in the same place.
---
 repmgr.c |  54 +++++++++++++-----------------
 repmgr.h | 100 +++++++++++++++++++++++++++++++++++++++----------------
 2 files changed, 95 insertions(+), 59 deletions(-)

diff --git a/repmgr.c b/repmgr.c
index 2e94c4a5..1b83a639 100644
--- a/repmgr.c
+++ b/repmgr.c
@@ -178,12 +178,6 @@ static bool	config_file_required = true;
 t_runtime_options runtime_options = T_RUNTIME_OPTIONS_INITIALIZER;
 t_configuration_options options = T_CONFIGURATION_OPTIONS_INITIALIZER;
 
-static bool  wal_keep_segments_used = false;
-static bool  conninfo_provided = false;
-static bool  connection_param_provided = false;
-static bool  host_param_provided = false;
-static bool  pg_rewind_supplied = false;
-
 static char *server_mode = NULL;
 static char *server_cmd = NULL;
 
@@ -365,13 +359,13 @@ main(int argc, char **argv)
 			case 'd':
 				strncpy(runtime_options.dbname, optarg, MAXLEN);
 				/* we'll set the dbname parameter below  if we detect it's not a conninfo string */
-				connection_param_provided = true;
+				runtime_options.connection_param_provided = true;
 				break;
 			case 'h':
 				strncpy(runtime_options.host, optarg, MAXLEN);
 				param_set(&source_conninfo, "host", optarg);
-				connection_param_provided = true;
-				host_param_provided = true;
+				runtime_options.connection_param_provided = true;
+				runtime_options.host_param_provided = true;
 				break;
 			case 'p':
 				repmgr_atoi(optarg, "-p/--port", &cli_errors, false);
@@ -379,12 +373,12 @@ main(int argc, char **argv)
 				strncpy(runtime_options.masterport,
 						optarg,
 						MAXLEN);
-				connection_param_provided = true;
+				runtime_options.connection_param_provided = true;
 				break;
 			case 'U':
 				strncpy(runtime_options.username, optarg, MAXLEN);
 				param_set(&source_conninfo, "user", optarg);
-				connection_param_provided = true;
+				runtime_options.connection_param_provided = true;
 				break;
 			case 'S':
 				strncpy(runtime_options.superuser, optarg, MAXLEN);
@@ -403,7 +397,7 @@ main(int argc, char **argv)
 				strncpy(runtime_options.wal_keep_segments,
 						optarg,
 						MAXLEN);
-				wal_keep_segments_used = true;
+				runtime_options.wal_keep_segments_used = true;
 				break;
 			case 'k':
 				runtime_options.keep_history = repmgr_atoi(optarg, "-k/--keep-history", &cli_errors, false);
@@ -519,7 +513,7 @@ main(int argc, char **argv)
 				{
 					strncpy(runtime_options.pg_rewind, optarg, MAXPGPATH);
 				}
-				pg_rewind_supplied = true;
+				runtime_options.pg_rewind_supplied = true;
 				break;
 			case OPT_PWPROMPT:
 				runtime_options.witness_pwprompt = true;
@@ -586,7 +580,7 @@ main(int argc, char **argv)
 		{
 			char	   *errmsg = NULL;
 
-			conninfo_provided = true;
+			runtime_options.conninfo_provided = true;
 
 			opts = PQconninfoParse(runtime_options.dbname, &errmsg);
 
@@ -620,13 +614,13 @@ main(int argc, char **argv)
 						(opt->val != NULL && opt->val[0] != '\0'))
 					{
 						strncpy(runtime_options.host, opt->val, MAXLEN);
-						host_param_provided = true;
+						runtime_options.host_param_provided = true;
 					}
 					if (strcmp(opt->keyword, "hostaddr") == 0 &&
 						(opt->val != NULL && opt->val[0] != '\0'))
 					{
 						strncpy(runtime_options.host, opt->val, MAXLEN);
-						host_param_provided = true;
+						runtime_options.host_param_provided = true;
 					}
 					else if (strcmp(opt->keyword, "port") == 0 &&
 							 (opt->val != NULL && opt->val[0] != '\0'))
@@ -762,7 +756,7 @@ main(int argc, char **argv)
 				initPQExpBuffer(&additional_host_arg);
 				appendPQExpBuffer(&additional_host_arg,
 								  _("Conflicting parameters:  you can't use %s while providing a node separately."),
-								  conninfo_provided == true ? "host=" : "-h/--host");
+								  runtime_options.conninfo_provided == true ? "host=" : "-h/--host");
 				item_list_append(&cli_errors, additional_host_arg.data);
 			}
 			else
@@ -884,7 +878,7 @@ main(int argc, char **argv)
 	 * the version check for 9.4 or later is done in check_upstream_config()
 	 */
 
-	if (options.use_replication_slots && wal_keep_segments_used)
+	if (options.use_replication_slots && runtime_options.wal_keep_segments_used)
 	{
 		log_warning(_("-w/--wal-keep-segments has no effect when replication slots in use\n"));
 	}
@@ -4268,7 +4262,7 @@ do_standby_follow(void)
 	 * to determine primary, and carry out some other checks while we're
 	 * at it.
 	 */
-	if (host_param_provided == false)
+	if (runtime_options.host_param_provided == false)
 	{
 		/* We need to connect to check configuration */
 		log_info(_("connecting to standby database\n"));
@@ -4562,7 +4556,7 @@ do_standby_switchover(void)
 	 * Add a friendly notice if --pg_rewind supplied for 9.5 and later - we'll
 	 * be ignoring it anyway
 	 */
-	if (pg_rewind_supplied == true && server_version_num >= 90500)
+	if (runtime_options.pg_rewind_supplied == true && server_version_num >= 90500)
 	{
 		log_notice(_("--pg_rewind not required for PostgreSQL 9.5 and later\n"));
 	}
@@ -4677,7 +4671,7 @@ do_standby_switchover(void)
 	else
 	{
 		/* 9.3/9.4 - user can use separately-compiled pg_rewind */
-		if (pg_rewind_supplied == true)
+		if (runtime_options.pg_rewind_supplied == true)
 		{
 			use_pg_rewind = true;
 
@@ -6574,7 +6568,7 @@ run_basebackup(const char *data_dir, int server_version)
 	 * consistency with other applications accepts a conninfo string
 	 * under -d/--dbname)
 	 */
-	if (conninfo_provided == true)
+	if (runtime_options.conninfo_provided == true)
 	{
 		appendPQExpBuffer(&params, " -d '%s'", runtime_options.dbname);
 	}
@@ -6695,7 +6689,7 @@ check_parameters_for_action(const int action)
 			 * parameters are at least useless and could be confusing so
 			 * reject them
 			 */
-			if (connection_param_provided)
+			if (runtime_options.connection_param_provided)
 			{
 				item_list_append(&cli_warnings, _("master connection parameters not required when executing MASTER REGISTER"));
 			}
@@ -6712,7 +6706,7 @@ check_parameters_for_action(const int action)
 			 * need connection parameters to the master because we can detect
 			 * the master in repl_nodes
 			 */
-			if (connection_param_provided)
+			if (runtime_options.connection_param_provided)
 			{
 				item_list_append(&cli_warnings, _("master connection parameters not required when executing STANDBY REGISTER"));
 			}
@@ -6729,7 +6723,7 @@ check_parameters_for_action(const int action)
 			 * need connection parameters to the master because we can detect
 			 * the master in repl_nodes
 			 */
-			if (connection_param_provided)
+			if (runtime_options.connection_param_provided)
 			{
 				item_list_append(&cli_warnings, _("master connection parameters not required when executing STANDBY UNREGISTER"));
 			}
@@ -6747,7 +6741,7 @@ check_parameters_for_action(const int action)
 			 * detect the master in repl_nodes if we can't find it then the
 			 * promote action will be cancelled
 			 */
-			if (connection_param_provided)
+			if (runtime_options.connection_param_provided)
 			{
 				item_list_append(&cli_warnings, _("master connection parameters not required when executing STANDBY PROMOTE"));
 			}
@@ -6773,7 +6767,7 @@ check_parameters_for_action(const int action)
 					item_list_append(&cli_errors, _("master hostname (-h/--host) required when executing STANDBY FOLLOW with -D/--data-dir option"));
 				}
 
-				if (host_param_provided && !runtime_options.dest_dir[0])
+				if (runtime_options.host_param_provided && !runtime_options.dest_dir[0])
 				{
 					item_list_append(&cli_errors, _("local data directory (-D/--data-dir) required when executing STANDBY FOLLOW with -h/--host option"));
 				}
@@ -6883,7 +6877,7 @@ check_parameters_for_action(const int action)
 			item_list_append(&cli_warnings, _("-r/--rsync-only can only be used when executing STANDBY CLONE"));
 		}
 
-		if (wal_keep_segments_used)
+		if (runtime_options.wal_keep_segments_used)
 		{
 			item_list_append(&cli_warnings, _("-w/--wal-keep-segments can only be used when executing STANDBY CLONE"));
 		}
@@ -6906,7 +6900,7 @@ check_parameters_for_action(const int action)
 	/* Warn about parameters which apply to STANDBY SWITCHOVER only */
 	if (action != STANDBY_SWITCHOVER)
 	{
-		if (pg_rewind_supplied == true)
+		if (runtime_options.pg_rewind_supplied == true)
 		{
 			item_list_append(&cli_warnings, _("--pg_rewind can only be used when executing STANDBY SWITCHOVER"));
 		}
@@ -7455,7 +7449,7 @@ check_upstream_config(PGconn *conn, int server_version_num, bool exit_on_error)
 		/*
 		 * -w/--wal-keep-segments was supplied - check against that value
 		 */
-		if (wal_keep_segments_used == true)
+		if (runtime_options.wal_keep_segments_used == true)
 		{
 			check_wal_keep_segments = true;
 			strncpy(min_wal_keep_segments, runtime_options.wal_keep_segments, MAXLEN);
diff --git a/repmgr.h b/repmgr.h
index d2ba3c0b..764f20a7 100644
--- a/repmgr.h
+++ b/repmgr.h
@@ -64,9 +64,10 @@
 #define OPT_CLUSTER                      13
 
 /* deprecated command line options */
-#define OPT_INITDB_NO_PWPROMPT           999
-#define OPT_IGNORE_EXTERNAL_CONFIG_FILES 998
+#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
 
@@ -74,53 +75,94 @@
 /* Run time options type */
 typedef struct
 {
+	/* general repmgr options */
+	char		config_file[MAXPGPATH];
+	bool		verbose;
+	bool		terse;
+	bool		force;
+
+	/* options which override setting in repmgr.conf */
+	char		loglevel[MAXLEN];
+	char		pg_bindir[MAXLEN];
+
+	/* 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		csv_mode;
 	bool		without_barman;
 	bool		no_upstream_connection;
 	bool		copy_external_config_files;
 	int			copy_external_config_files_destination;
 	bool		wait_register_sync;
 	int			wait_register_sync_seconds;
-	char		masterport[MAXLEN];
-	/*
-	 * configuration file parameters which can be overridden on the
-	 * command line
-	 */
-	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;
-	/* parameter used by {STANDBY|WITNESS} UNREGISTER */
-	int			node;
-
-	char		pg_bindir[MAXLEN];
 
 	char		recovery_min_apply_delay[MAXLEN];
+
+	/* 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, false, false, CONFIG_FILE_SAMEPATH, false, 0, "", "", "", "", "fast", "", 0, UNKNOWN_NODE_ID, "", ""}
+#define T_RUNTIME_OPTIONS_INITIALIZER { \
+		/* general repmgr options */	\
+		"", false, false, false,		\
+		/* options which override setting in repmgr.conf */ \
+		"", "",                         \
+		/* connection parameters */		\
+		"", "", "", "", "", "", "", 	\
+		false, false, false,		    \
+		/* standby clone parameters */  \
+		false, DEFAULT_WAL_KEEP_SEGMENTS, false, false, false, false, false, false, \
+		CONFIG_FILE_SAMEPATH, 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
 {

From 96c14adfdb5b91820fb32b3529ed34680ed55ff1 Mon Sep 17 00:00:00 2001
From: Ian Barwick <ian@2ndquadrant.com>
Date: Mon, 17 Oct 2016 15:38:12 +0900
Subject: [PATCH 17/23] repmgr: ensure data directory defaults to that of the
 source node

As long as -D/--pgdata is not supplied, and barman mode is not used
for cloning.
---
 repmgr.c | 71 ++++++++++++++++++++++++++++++++------------------------
 1 file changed, 41 insertions(+), 30 deletions(-)

diff --git a/repmgr.c b/repmgr.c
index 1b83a639..c84c6a8d 100644
--- a/repmgr.c
+++ b/repmgr.c
@@ -2516,9 +2516,14 @@ do_standby_clone(void)
 	/*
 	 * If dest_dir (-D/--pgdata) was provided, this will become the new data
 	 * directory (otherwise repmgr will default to using the same directory
-	 * path as on the source host)
+	 * path as on the source host).
+	 *
+	 * Note that barman mode requires -D/--pgdata.
+	 *
+	 * If -D/--pgdata is not supplied, and we're not cloning from barman,
+	 * the source host's data directory will be fetched later, after
+	 * we've connected to it.
 	 */
-
 	if (runtime_options.dest_dir[0])
 	{
 		target_directory_provided = true;
@@ -2541,26 +2546,7 @@ do_standby_clone(void)
 	{
 		strncpy(local_data_directory, runtime_options.dest_dir, MAXPGPATH);
 	}
-	/*
-	 * Otherwise use the same data directory as on the remote host
-	 */
-	else
-	{
-		strncpy(local_data_directory, master_data_directory, MAXPGPATH);
 
-		log_notice(_("setting data directory to: %s\n"), local_data_directory);
-		log_hint(_("use -D/--data-dir to explicitly specify a data directory\n"));
-	}
-
-	/* Check the local data directory can be used */
-
-	if (!create_pg_dir(local_data_directory, runtime_options.force))
-	{
-		log_err(_("unable to use directory %s ...\n"),
-				local_data_directory);
-		log_hint(_("use -F/--force option to force this directory to be overwritten\n"));
-		exit(ERR_BAD_CONFIG);
-	}
 
 	/*
 	 * Initialise list of conninfo parameters which will later be used
@@ -2633,7 +2619,7 @@ do_standby_clone(void)
 
 		if (!create_pg_dir(local_repmgr_directory, runtime_options.force))
 		{
-			log_err(_("unable to use directory %s ...\n"),
+			log_err(_("unable to use directory \"%s\" ...\n"),
 					local_repmgr_directory);
 			log_hint(_("use -F/--force option to force this directory to be overwritten\n"));
 
@@ -2732,6 +2718,28 @@ do_standby_clone(void)
 				primary_conn = source_conn;
 			}
 
+
+			/* Fetch the source's data directory */
+			if (get_pg_setting(source_conn, "data_directory", master_data_directory) == false)
+			{
+				log_err(_("Unable to retrieve upstream node's data directory\n"));
+				log_hint(_("STANDBY CLONE must be run as a database superuser"));
+				PQfinish(source_conn);
+				exit(ERR_BAD_CONFIG);
+			}
+
+			/*
+			 * If no target directory was explicitly provided, we'll default to
+			 * the same directory as on the source host.
+			 */
+			if (target_directory_provided == false)
+			{
+				strncpy(local_data_directory, master_data_directory, MAXPGPATH);
+
+				log_notice(_("setting data directory to: \"%s\"\n"), local_data_directory);
+				log_hint(_("use -D/--data-dir to explicitly specify a data directory\n"));
+			}
+
 			/*
 			 * Copy the source connection so that we have some default values,
 			 * particularly stuff like passwords extracted from PGPASSFILE;
@@ -2892,6 +2900,17 @@ do_standby_clone(void)
 		}
 	}
 
+
+	/* Check the destination data directory can be used */
+
+	if (!create_pg_dir(local_data_directory, runtime_options.force))
+	{
+		log_err(_("unable to use directory %s ...\n"),
+				local_data_directory);
+		log_hint(_("use -F/--force option to force this directory to be overwritten\n"));
+		exit(ERR_BAD_CONFIG);
+	}
+
 	if (mode != barman)
 	{
 		/*
@@ -2943,14 +2962,6 @@ do_standby_clone(void)
 			}
 		}
 
-
-		if (get_pg_setting(source_conn, "data_directory", master_data_directory) == false)
-		{
-			log_err(_("Unable to retrieve upstream node's data directory\n"));
-			log_hint(_("STANDBY CLONE must be run as a database superuser"));
-			exit(ERR_BAD_CONFIG);
-		}
-
 		/*
 		 * Obtain configuration file locations
 		 * We'll check to see whether the configuration files are in the data

From f0588334510bbb41f2f5e3d1601805159f820577 Mon Sep 17 00:00:00 2001
From: Ian Barwick <ian@2ndquadrant.com>
Date: Wed, 19 Oct 2016 10:00:50 +0900
Subject: [PATCH 18/23] repmgr: in Barman clone mode, don't try and create the
 data directory twice

In Barman mode the data directory is created early containing a temporary
directory needed to hold temporary files while cloning from the Barman
server. In other modes we might not know the data directory location until
connecting to the source server, so its creation happens later. In Barman
mode ensure that step is skipped.
---
 repmgr.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/repmgr.c b/repmgr.c
index c84c6a8d..11a4c1d9 100644
--- a/repmgr.c
+++ b/repmgr.c
@@ -2901,18 +2901,23 @@ do_standby_clone(void)
 	}
 
 
-	/* Check the destination data directory can be used */
-
-	if (!create_pg_dir(local_data_directory, runtime_options.force))
-	{
-		log_err(_("unable to use directory %s ...\n"),
-				local_data_directory);
-		log_hint(_("use -F/--force option to force this directory to be overwritten\n"));
-		exit(ERR_BAD_CONFIG);
-	}
 
 	if (mode != barman)
 	{
+
+		/*
+		 * Check the destination data directory can be used
+		 * (in Barman mode, this directory will already have been created)
+		 */
+
+		if (!create_pg_dir(local_data_directory, runtime_options.force))
+		{
+			log_err(_("unable to use directory %s ...\n"),
+					local_data_directory);
+			log_hint(_("use -F/--force option to force this directory to be overwritten\n"));
+			exit(ERR_BAD_CONFIG);
+		}
+
 		/*
 		 * Check that tablespaces named in any `tablespace_mapping` configuration
 		 * file parameters exist.

From e19c6433891eda6c9c34f40f84cdb1b7b359105c Mon Sep 17 00:00:00 2001
From: Ian Barwick <ian@2ndquadrant.com>
Date: Wed, 19 Oct 2016 15:06:21 +0900
Subject: [PATCH 19/23] repmgr: refactor check_upstream_config() for clarity

Also ensure the check for sufficient walsenders is only carried out
when using pg_basebackup.
---
 repmgr.c | 37 ++++++++++++++++++++++++++++---------
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/repmgr.c b/repmgr.c
index 11a4c1d9..7ec8e034 100644
--- a/repmgr.c
+++ b/repmgr.c
@@ -7327,10 +7327,10 @@ check_master_standby_version_match(PGconn *conn, PGconn *master_conn)
 /*
  * check_upstream_config()
  *
- * Perform sanity check on upstream server configuration
+ * Perform sanity check on upstream server configuration before starting cloning
+ * process
  *
  * TODO:
- *  - check replication connection is possble
  *  - check user is qualified to perform base backup
  */
 
@@ -7343,14 +7343,33 @@ check_upstream_config(PGconn *conn, int server_version_num, bool exit_on_error)
 	t_basebackup_options  backup_options = T_BASEBACKUP_OPTIONS_INITIALIZER;
 	bool		xlog_stream = true;
 
+	enum {
+		barman,
+		rsync,
+		pg_basebackup
+	}			mode;
+
+
+	/*
+	 * Detecting the intended cloning mode
+	 */
+	if (runtime_options.rsync_only)
+		mode = rsync;
+	else if (strcmp(options.barman_server, "") != 0 && ! runtime_options.without_barman)
+		mode = barman;
+	else
+		mode = pg_basebackup;
+
 	/*
 	 * Parse `pg_basebackup_options`, if set, to detect whether --xlog-method
 	 * has been set to something other than `stream` (i.e. `fetch`), as
 	 * this will influence some checks
 	 */
+
 	parse_pg_basebackup_options(options.pg_basebackup_options, &backup_options);
 	if (strlen(backup_options.xlog_method) && strcmp(backup_options.xlog_method, "stream") != 0)
 		xlog_stream = false;
+
 	/* Check that WAL level is set correctly */
 	if (server_version_num < 90400)
 	{
@@ -7457,7 +7476,7 @@ check_upstream_config(PGconn *conn, int server_version_num, bool exit_on_error)
 	 * physical replication slots not available or not requested - check if
 	 * there are any circumstances where `wal_keep_segments` should be set
 	 */
-	else if (! runtime_options.without_barman && strcmp(options.barman_server, "") == 0)
+	else if (mode != barman)
 	{
 		bool check_wal_keep_segments = false;
 		char min_wal_keep_segments[MAXLEN] = "1";
@@ -7588,7 +7607,12 @@ check_upstream_config(PGconn *conn, int server_version_num, bool exit_on_error)
 		config_ok = false;
 	}
 
-	if (!runtime_options.rsync_only)
+	/*
+	 * If using pg_basebackup, ensure sufficient replication connections can be made.
+	 * There's no guarantee they'll still be available by the time pg_basebackup
+	 * is executed, but there's nothing we can do about that.
+	 */
+	if (mode == pg_basebackup)
 	{
 
 		PGconn	  **connections;
@@ -7610,11 +7634,6 @@ check_upstream_config(PGconn *conn, int server_version_num, bool exit_on_error)
 		 * work out how many replication connections are required (1 or 2)
 		 */
 
-		/*
-		 * --xlog-method set by user to something other than "stream" (should be "fetch",
-		 * but we're just checking it's not "stream")
-		 */
-
 		if (xlog_stream == true)
 			min_replication_connections += 1;
 

From a63baf7fcb937ab3db0f2225d9d019c2f3d0a0b4 Mon Sep 17 00:00:00 2001
From: Ian Barwick <ian@2ndquadrant.com>
Date: Wed, 19 Oct 2016 15:30:02 +0900
Subject: [PATCH 20/23] repmgr: rename local variable `config_file_found` to
 `remote_config_file_found`

We also have a global `config_file_found` - avoid confusion with that.
---
 repmgr.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/repmgr.c b/repmgr.c
index 7ec8e034..fdf9e94c 100644
--- a/repmgr.c
+++ b/repmgr.c
@@ -3666,7 +3666,7 @@ do_standby_clone(void)
 									  file->filepath, dest_path, false, server_version_num);
 				if (r != 0)
 				{
-					log_err(_("standby clone: unable to copying config file '%s'\n"),
+					log_err(_("standby clone: unable to copy config file '%s'\n"),
 							file->filename);
 				}
 			}
@@ -4836,7 +4836,7 @@ do_standby_switchover(void)
 	else
 	{
 		int		    i;
-		bool		config_file_found = false;
+		bool		remote_config_file_found = false;
 
 		const char *config_paths[] = {
 			runtime_options.config_file,
@@ -4846,7 +4846,7 @@ do_standby_switchover(void)
 
 		log_verbose(LOG_INFO, _("no remote configuration file provided - checking default locations\n"));
 
-		for (i = 0; config_paths[i] && config_file_found == false; ++i)
+		for (i = 0; config_paths[i] && remote_config_file_found == false; ++i)
 		{
 			/*
 			 * Don't attempt to check for an empty filename - this might be the case
@@ -4878,13 +4878,13 @@ do_standby_switchover(void)
 				strncpy(runtime_options.remote_config_file, config_paths[i], MAXLEN);
 				log_verbose(LOG_INFO, _("configuration file \"%s\" found on remote server\n"),
 							runtime_options.remote_config_file);
-				config_file_found = true;
+				remote_config_file_found = true;
 			}
 
 			termPQExpBuffer(&command_output);
 		}
 
-		if (config_file_found == false)
+		if (remote_config_file_found == false)
 		{
 			log_err(_("no remote configuration file supplied or found in a default location - terminating\n"));
 			log_hint(_("specify the remote configuration file with -C/--remote-config-file\n"));

From 3f7c30b84dd49df111db2c83ec8dad3c1781e98b Mon Sep 17 00:00:00 2001
From: Ian Barwick <ian@2ndquadrant.com>
Date: Wed, 19 Oct 2016 16:16:12 +0900
Subject: [PATCH 21/23] repmgr: require a valid repmgr cluster name unless
 -F/--force supplied

Addresses issue mentioned in GitHub #242.
---
 repmgr.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/repmgr.c b/repmgr.c
index fdf9e94c..8a76a1e2 100644
--- a/repmgr.c
+++ b/repmgr.c
@@ -898,6 +898,7 @@ main(int argc, char **argv)
 		}
 
 	/* Initialise the repmgr schema name */
+
 	if (strlen(repmgr_cluster))
 		/* --cluster parameter provided */
 		maxlen_snprintf(repmgr_schema, "%s%s", DEFAULT_REPMGR_SCHEMA_PREFIX,
@@ -906,6 +907,16 @@ main(int argc, char **argv)
 		maxlen_snprintf(repmgr_schema, "%s%s", DEFAULT_REPMGR_SCHEMA_PREFIX,
 						options.cluster_name);
 
+	/*
+	 * If no value for the repmgr_schema provided, continue only under duress.
+	 */
+	if (strcmp(repmgr_schema, DEFAULT_REPMGR_SCHEMA_PREFIX) == 0 && !runtime_options.force)
+	{
+		log_err(_("unable to determine cluster name - please provide a valid configuration file with -c/--config-file\n"));
+		log_hint(_("Use -F/--force to continue anyway\n"));
+		exit(ERR_BAD_CONFIG);
+	}
+
 	/*
 	 * Initialise slot name, if required (9.4 and later)
 	 *

From 20d66df0efd180baa7c640db64273d91631a6589 Mon Sep 17 00:00:00 2001
From: Ian Barwick <ian@2ndquadrant.com>
Date: Wed, 19 Oct 2016 20:26:55 +0900
Subject: [PATCH 22/23] repmgr: prevent involuntary cloning where no repmgr
 schema present on master

---
 repmgr.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/repmgr.c b/repmgr.c
index 8a76a1e2..f3c2ab79 100644
--- a/repmgr.c
+++ b/repmgr.c
@@ -2730,6 +2730,27 @@ do_standby_clone(void)
 			}
 
 
+
+			/*
+			 * Sanity-check that the master node has a repmgr schema - if not
+			 * present, fail with an error (unless -F/--force is used)
+			 */
+
+			if (check_cluster_schema(primary_conn) == false)
+			{
+				if (!runtime_options.force)
+				{
+					/* schema doesn't exist */
+					log_err(_("expected repmgr schema '%s' not found on master server\n"), get_repmgr_schema());
+					log_hint(_("check that the master server was correctly registered\n"));
+					PQfinish(source_conn);
+					exit(ERR_BAD_CONFIG);
+				}
+
+				log_warning(_("expected repmgr schema '%s' not found on master server\n"), get_repmgr_schema());
+			}
+
+
 			/* Fetch the source's data directory */
 			if (get_pg_setting(source_conn, "data_directory", master_data_directory) == false)
 			{
@@ -2751,6 +2772,8 @@ do_standby_clone(void)
 				log_hint(_("use -D/--data-dir to explicitly specify a data directory\n"));
 			}
 
+
+
 			/*
 			 * Copy the source connection so that we have some default values,
 			 * particularly stuff like passwords extracted from PGPASSFILE;
@@ -6106,6 +6129,7 @@ do_witness_register(PGconn *masterconn)
 	log_notice(_("configuration has been successfully copied to the witness\n"));
 }
 
+
 static void
 do_witness_unregister(void)
 {

From 0cde0068ddab502cd110eb00289151755b429071 Mon Sep 17 00:00:00 2001
From: Ian Barwick <ian@2ndquadrant.com>
Date: Thu, 20 Oct 2016 10:56:16 +0900
Subject: [PATCH 23/23] repmgrd: improve witness failover logging

Log the new master node as INFO rather than DEBUG.
---
 repmgrd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/repmgrd.c b/repmgrd.c
index 7ffe1b86..3011254e 100644
--- a/repmgrd.c
+++ b/repmgrd.c
@@ -647,7 +647,7 @@ 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;
 
 				/*