README: clarify that the repmgr metadatabase must be part of the replication cluster

Bump version to 3.2.1

HISTORY

@@ -1,17 +1,3 @@
-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)

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 compat.o
+repmgr_OBJS = dbutils.o check_dir.o config.o repmgr.o log.o strutil.o dirmod.o
 
 DATA = repmgr.sql uninstall_repmgr.sql
 

README.md

@@ -230,29 +230,15 @@ The configuration file will be searched for in the following locations:
 
 Note that if a file is explicitly specified with `-f/--config-file`, an error will
 be raised if it is not found or not readable and no attempt will be made to check
-default locations; this is to prevent `repmgr` unexpectedly reading the wrong file.
+default locations; this is to prevent `repmgr` reading the wrong file.
 
 For a full list of annotated configuration items, see the file `repmgr.conf.sample`.
 
 The following parameters in the configuration file can be overridden with
 command line options:
 
-- `log_level` with `-L/--log-level`
+- `-L/--log-level`
-- `pg_bindir` with `-b/--pg_bindir`
+- `-b/--pg_bindir`
-
-
-### Logging
-
-By default `repmgr` and `repmgrd` will log directly to `STDERR`. For `repmgrd`
-we recommend capturing output in a logfile or using your system's log facility;
-see `repmgr.conf.sample` for details.
-
-As a command line utility, `repmgr` will log directly to the console by default
-(this is a change in behaviour from versions before 3.3, where it would always
-log to the same location as `repmgrd`). However in some circumstances, such as
-when `repmgr` is executed by `repmgrd` during a failover event, it makes sense to
-capture `repmgr`'s log output - this can be done by supplying the command-line
-option `--log-to-file` to `repmgr`.
 
 
 ### Command line options and environment variables
@@ -289,14 +275,14 @@ Setting up a simple replication cluster with repmgr
 The following section will describe how to set up a basic replication cluster
 with a master and a standby server using the `repmgr` command line tool.
 It is assumed PostgreSQL is installed on both servers in the cluster,
-`rsync` is available and passwordless SSH connections are possible between
+`rsync` is available and password-less SSH connections are possible between
 both servers.
 
 * * *
 
 > *TIP*: for testing `repmgr`, it's possible to use multiple PostgreSQL
 > instances running on different ports on the same computer, with
-> passwordless SSH access to `localhost` enabled.
+> password-less SSH access to `localhost` enabled.
 
 * * *
 
@@ -427,7 +413,7 @@ be registered with `repmgr`, which creates the `repmgr` database and adds
 a metadata record for the server:
 
     $ repmgr -f repmgr.conf master register
-    NOTICE: master node correctly registered for cluster test with id 1 (conninfo: host=repmgr_node1 user=repmgr dbname=repmgr)
+    [2016-01-07 16:56:46] [NOTICE] master node correctly registered for cluster test with id 1 (conninfo: host=repmgr_node1 user=repmgr dbname=repmgr)
 
 The metadata record looks like this:
 
@@ -454,13 +440,13 @@ the values `node`, `node_name` and `conninfo` adjusted accordingly, e.g.:
 Clone the standby with:
 
     $ repmgr -h repmgr_node1 -U repmgr -d repmgr -D /path/to/node2/data/ -f /etc/repmgr.conf standby clone
-    NOTICE: destination directory '/path/to/node2/data/' provided
+    [2016-01-07 17:21:26] [NOTICE] destination directory '/path/to/node2/data/' provided
-    NOTICE: starting backup...
+    [2016-01-07 17:21:26] [NOTICE] starting backup...
-    HINT: this may take some time; consider using the -c/--fast-checkpoint option
+    [2016-01-07 17:21:26] [HINT] this may take some time; consider using the -c/--fast-checkpoint option
     NOTICE:  pg_stop_backup complete, all required WAL segments have been archived
-    NOTICE: standby clone (using pg_basebackup) complete
+    [2016-01-07 17:21:28] [NOTICE] standby clone (using pg_basebackup) complete
-    NOTICE: you can now start your PostgreSQL server
+    [2016-01-07 17:21:28] [NOTICE] you can now start your PostgreSQL server
-    HINT: for example : pg_ctl -D /path/to/node2/data/ start
+    [2016-01-07 17:21:28] [HINT] for example : pg_ctl -D /path/to/node2/data/ start
 
 This will clone the PostgreSQL data directory files from the master at `repmgr_node1`
 using PostgreSQL's `pg_basebackup` utility. A `recovery.conf` file containing the
@@ -555,8 +541,8 @@ Connect to the master server and execute:
 
 Register the standby server with:
 
-    $ repmgr -f /etc/repmgr.conf standby register
+    repmgr -f /etc/repmgr.conf standby register
-    NOTICE: standby node correctly registered for cluster test with id 2 (conninfo: host=repmgr_node2 user=repmgr dbname=repmgr)
+    [2016-01-08 11:13:16] [NOTICE] standby node correctly registered for cluster test with id 2 (conninfo: host=repmgr_node2 user=repmgr dbname=repmgr)
 
 Connect to the standby server's `repmgr` database and check the `repl_nodes`
 table:
@@ -587,21 +573,6 @@ to effectively manage cascading replication (see below).
 
 * * *
 
-Under some circumstances you may wish to register a standby which is not
-yet running; this can be the case when using provisioning tools to create
-a complex replication cluster. In this case, by using the `-F/--force`
-option and providing the connection parameters to the master server,
-the standby can be registered.
-
-Similarly, with cascading replication it may be necessary to register
-a standby whose upstream node has not yet been registered - in this case,
-using `-F/--force` will result in the creation of an inactive placeholder
-record for the upstream node, which will however later need to be registered
-with the `-F/--force` option too.
-
-When used with `standby register`, care should be taken that use of the
-`-F/--force` option does not result in an incorrectly configured cluster.
-
 ### Using Barman to clone a standby
 
 `repmgr standby clone` also supports Barman, the Backup and
@@ -670,13 +641,13 @@ specify this in `repmgr.conf` with `barman_config`:
 
 Now we can clone a standby using the Barman server:
 
-    $ repmgr -h node1 -d repmgr -D 9.5/main -f /etc/repmgr.conf standby clone
+    $ repmgr -h node1 -D 9.5/main -f /etc/repmgr.conf standby clone
-    NOTICE: destination directory '9.5/main' provided
+    [2016-06-12 20:08:35] [NOTICE] destination directory '9.5/main' provided
-    NOTICE: getting backup from Barman...
+    [2016-06-12 20:08:35] [NOTICE] getting backup from Barman...
-    NOTICE: standby clone (from Barman) complete
+    [2016-06-12 20:08:36] [NOTICE] standby clone (from Barman) complete
-    NOTICE: you can now start your PostgreSQL server
+    [2016-06-12 20:08:36] [NOTICE] you can now start your PostgreSQL server
-    HINT: for example : pg_ctl -D 9.5/data start
+    [2016-06-12 20:08:36] [HINT] for example : pg_ctl -D 9.5/data start
-    HINT: After starting the server, you need to register this standby with "repmgr standby register"
+    [2016-06-12 20:08:36] [HINT] After starting the server, you need to register this standby with "repmgr standby register"
 
 
 
@@ -736,22 +707,13 @@ string passed to `repmgr` with `-d/--dbname` (see above for details), and/or set
 appropriate environment variables.
 
 Note that PostgreSQL will always set explicit defaults for `sslmode` and
-`sslcompression` (and from PostgreSQL 10.0 also `target_session_attrs`).
+`sslcompression`.
 
 If `application_name` is set in the standby's `conninfo` parameter in
 `repmgr.conf`, this value will be appended to `primary_conninfo`, otherwise
 `repmgr` will set `application_name` to the same value as the `node_name`
 parameter.
 
-By default `repmgr` assumes the user who owns the `repmgr` metadatabase will
-also be the replication user; a different replication user can be specified
-with `--replication-user`.
-
-If the upstream server requires a password, and this was provided via
-`PGPASSWORD`, `.pgpass` etc., by default `repmgr` will include this in
-`primary_conninfo`. Use the command line option `--no-conninfo-password` to
-suppress this.
-
 
 Setting up cascading replication with repmgr
 --------------------------------------------
@@ -785,15 +747,15 @@ created standby. Clone this standby (using the connection parameters
 for the existing standby) and register it:
 
     $ repmgr -h repmgr_node2 -U repmgr -d repmgr -D /path/to/node3/data/ -f /etc/repmgr.conf standby clone
-    NOTICE: destination directory 'node_3/data/' provided
+    [2016-01-08 13:44:52] [NOTICE] destination directory 'node_3/data/' provided
-    NOTICE: starting backup (using pg_basebackup)...
+    [2016-01-08 13:44:52] [NOTICE] starting backup (using pg_basebackup)...
-    HINT: this may take some time; consider using the -c/--fast-checkpoint option
+    [2016-01-08 13:44:52] [HINT] this may take some time; consider using the -c/--fast-checkpoint option
-    NOTICE: standby clone (using pg_basebackup) complete
+    [2016-01-08 13:44:52] [NOTICE] standby clone (using pg_basebackup) complete
-    NOTICE: you can now start your PostgreSQL server
+    [2016-01-08 13:44:52] [NOTICE] you can now start your PostgreSQL server
-    HINT: for example : pg_ctl -D /path/to/node_3/data start
+    [2016-01-08 13:44:52] [HINT] for example : pg_ctl -D /path/to/node_3/data start
 
     $ repmgr -f /etc/repmgr.conf standby register
-    NOTICE: standby node correctly registered for cluster test with id 3 (conninfo: host=repmgr_node3 dbname=repmgr user=repmgr)
+    [2016-01-08 14:04:32] [NOTICE] standby node correctly registered for cluster test with id 3 (conninfo: host=repmgr_node3 dbname=repmgr user=repmgr)
 
 After starting the standby, the `repl_nodes` table will look like this:
 
@@ -805,15 +767,6 @@ After starting the standby, the `repl_nodes` table will look like this:
       3 | standby |                2 | test    | node3 | host=repmgr_node3 dbname=repmgr user=repmgr |           |      100 | t
     (3 rows)
 
-* * *
-
-> *TIP*: under some circumstances when setting up a cascading replication
-> cluster, you may wish to clone a downstream standby whose upstream node
-> does not yet exist. In this case you can clone from the master (or
-> another upstream node) and provide the parameter `--upstream-conninfo`
-> to explictly set the upstream's `primary_conninfo` string in `recovery.conf`.
-
-* * *
 
 Using replication slots with repmgr
 -----------------------------------
@@ -899,19 +852,19 @@ Promote the first standby with:
 
 This will produce output similar to the following:
 
-    ERROR: connection to database failed: could not connect to server: Connection refused
+    [2016-01-08 16:07:31] [ERROR] connection to database failed: could not connect to server: Connection refused
             Is the server running on host "repmgr_node1" (192.161.2.1) and accepting
             TCP/IP connections on port 5432?
     could not connect to server: Connection refused
             Is the server running on host "repmgr_node1" (192.161.2.1) and accepting
             TCP/IP connections on port 5432?
 
-    NOTICE: promoting standby
+    [2016-01-08 16:07:31] [NOTICE] promoting standby
-    NOTICE: promoting server using '/usr/bin/postgres/pg_ctl -D /path/to/node_2/data promote'
+    [2016-01-08 16:07:31] [NOTICE] promoting server using '/usr/bin/postgres/pg_ctl -D /path/to/node_2/data promote'
     server promoting
-    NOTICE: STANDBY PROMOTE successful
+    [2016-01-08 16:07:33] [NOTICE] STANDBY PROMOTE successful
 
-Note: the first `ERROR` is `repmgr` attempting to connect to the current
+Note: the first `[ERROR]` is `repmgr` attempting to connect to the current
 master to verify that it has failed. If a valid master is found, `repmgr`
 will refuse to promote a standby.
 
@@ -943,7 +896,7 @@ end of the preceding section ("Promoting a standby server with repmgr"),
 execute this:
 
     $ repmgr -f /etc/repmgr.conf -D /path/to/node_3/data/ -h repmgr_node2 -U repmgr -d repmgr standby follow
-    NOTICE: restarting server using '/usr/bin/postgres/pg_ctl -D /path/to/node_3/data/ -w -m fast restart'
+    [2016-01-08 16:57:06] [NOTICE] restarting server using '/usr/bin/postgres/pg_ctl -D /path/to/node_3/data/ -w -m fast restart'
     waiting for server to shut down.... done
     server stopped
     waiting for server to start.... done
@@ -1015,26 +968,26 @@ local server, as well as the normal default locations. `repmgr` will check
 this file can be found before performing any further actions.
 
     $ repmgr -f /etc/repmgr.conf -C /etc/repmgr.conf standby switchover -v
-    NOTICE: using configuration file "/etc/repmgr.conf"
+    [2016-01-27 16:38:33] [NOTICE] using configuration file "/etc/repmgr.conf"
-    NOTICE: switching current node 2 to master server and demoting current master to standby...
+    [2016-01-27 16:38:33] [NOTICE] switching current node 2 to master server and demoting current master to standby...
-    NOTICE: 5 files copied to /tmp/repmgr-node1-archive
+    [2016-01-27 16:38:34] [NOTICE] 5 files copied to /tmp/repmgr-node1-archive
-    NOTICE: connection to database failed: FATAL:  the database system is shutting down
+    [2016-01-27 16:38:34] [NOTICE] connection to database failed: FATAL:  the database system is shutting down
 
-    NOTICE: current master has been stopped
+    [2016-01-27 16:38:34] [NOTICE] current master has been stopped
-    ERROR: connection to database failed: FATAL:  the database system is shutting down
+    [2016-01-27 16:38:34] [ERROR] connection to database failed: FATAL:  the database system is shutting down
 
-    NOTICE: promoting standby
+    [2016-01-27 16:38:34] [NOTICE] promoting standby
-    NOTICE: promoting server using '/usr/local/bin/pg_ctl -D /var/lib/postgresql/9.5/node_2/data promote'
+    [2016-01-27 16:38:34] [NOTICE] promoting server using '/usr/local/bin/pg_ctl -D /var/lib/postgresql/9.5/node_2/data promote'
     server promoting
-    NOTICE: STANDBY PROMOTE successful
+    [2016-01-27 16:38:36] [NOTICE] STANDBY PROMOTE successful
-    NOTICE: Executing pg_rewind on old master server
+    [2016-01-27 16:38:36] [NOTICE] Executing pg_rewind on old master server
-    NOTICE: 5 files copied to /var/lib/postgresql/9.5/data
+    [2016-01-27 16:38:36] [NOTICE] 5 files copied to /var/lib/postgresql/9.5/data
-    NOTICE: restarting server using '/usr/local/bin/pg_ctl -w -D /var/lib/postgresql/9.5/node_1/data -m fast restart'
+    [2016-01-27 16:38:36] [NOTICE] restarting server using '/usr/local/bin/pg_ctl -w -D /var/lib/postgresql/9.5/node_1/data -m fast restart'
     pg_ctl: PID file "/var/lib/postgresql/9.5/node_1/data/postmaster.pid" does not exist
     Is server running?
     starting server anyway
-    NOTICE: node 1 is replicating in state "streaming"
+    [2016-01-27 16:38:37] [NOTICE] node 1 is replicating in state "streaming"
-    NOTICE: switchover was successful
+    [2016-01-27 16:38:37] [NOTICE] switchover was successful
 
 Messages containing the line `connection to database failed: FATAL: the database
 system is shutting down` are not errors - `repmgr` is polling the old master database
@@ -1140,23 +1093,18 @@ Automatic failover with `repmgrd`
 and which can automate actions such as failover and updating standbys to
 follow the new master.
 
-To use `repmgrd` for automatic failover, `postgresql.conf` must contain the
+To use `repmgrd` for automatic failover, the following `repmgrd` options must
-following line:
+be set in `repmgr.conf`:
-
-    shared_preload_libraries = 'repmgr_funcs'
-
-(changing this setting requires a restart of PostgreSQL).
-
-Additionally the following `repmgrd` options must be set in `repmgr.conf`:
 
     failover=automatic
-    promote_command='repmgr standby promote -f /etc/repmgr.conf --log-to-file'
+    promote_command='repmgr standby promote -f /etc/repmgr.conf'
-    follow_command='repmgr standby follow -f /etc/repmgr.conf --log-to-file'
+    follow_command='repmgr standby follow -f /etc/repmgr.conf'
 
-Note that the `--log-to-file` option will cause `repmgr` output to be logged to
+(See `repmgr.conf.sample` for further `repmgrd`-specific settings).
-the destination configured to receive  log output `repmgrd`.
-See `repmgr.conf.sample` for further `repmgrd`-specific settings
 
+Additionally, `postgresql.conf` must contain the following line:
+
+    shared_preload_libraries = 'repmgr_funcs'
 
 When `failover` is set to `automatic`, upon detecting failure of the current
 master, `repmgrd` will execute one of `promote_command` or `follow_command`,
@@ -1459,9 +1407,7 @@ functionality will be included in a feature release (e.g. 3.0.x to 3.1.x).
 
 In general `repmgr` can be upgraded as-is without any further action required,
 however feature releases may require the `repmgr` database to be upgraded.
-An SQL script will be provided - please check the release notes for details:
+An SQL script will be provided - please check the release notes for details.
-
-* http://repmgr.org/release-notes-3.2.1.html#UPGRADING
 
 
 Distribution-specific configuration
@@ -1564,7 +1510,7 @@ which contains connection details for the local database.
     bootstrapping new installations. To update an existing but 'stale'
     data directory (for example belonging to a failed master), `rsync`
     must be used by specifying `--rsync-only`. In this case,
-    passwordless SSH connections between servers are required.
+    password-less SSH connections between servers are required.
 
 * `standby promote`
 
@@ -1578,13 +1524,13 @@ which contains connection details for the local database.
     by using `standby follow` (see below); if `repmgrd` is active, it will
     handle this.
 
-    This command will fail with an error if the current master is still running.
+    This command will not function if the current master is still running.
 
 * `standby switchover`
 
     Promotes a standby to master and demotes the existing master to a standby.
     This command must be run on the standby to be promoted, and requires a
-    passwordless SSH connection to the current master. Additionally the
+    password-less SSH connection to the current master. Additionally the
     location of the master's `repmgr.conf` file must be provided with
     `-C/--remote-config-file`.
 
@@ -1691,7 +1637,7 @@ which contains connection details for the local database.
       overview of connections between all databases in the cluster.
 
     These commands require a valid `repmgr.conf` file on each node.
-    Additionally passwordless `ssh` connections are required between
+    Additionally password-less `ssh` connections are required between
     all nodes.
 
     Example 1 (all nodes up):
@@ -1869,7 +1815,6 @@ Thanks from the repmgr core team.
 Further reading
 ---------------
 
-* http://blog.2ndquadrant.com/repmgr-3-2-is-here-barman-support-brand-new-high-availability-features/
 * http://blog.2ndquadrant.com/improvements-in-repmgr-3-1-4/
 * http://blog.2ndquadrant.com/managing-useful-clusters-repmgr/
 * http://blog.2ndquadrant.com/easier_postgresql_90_clusters/

compat.c

@@ -1,111 +0,0 @@
-/*
- *
- * 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

@@ -1,29 +0,0 @@
-/*
- * 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

@@ -10,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/>.
  *
  */
 
@@ -55,8 +55,8 @@ progname(void)
  *
  * Returns true if a configuration file could be parsed, otherwise false.
  *
- * Any *repmgrd-specific* configuration options added/changed in this function must also be
+ * Any configuration options changed in this function must also be changed in
- * added/changed in reload_config()
+ * 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,
@@ -99,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.
@@ -181,23 +181,6 @@ 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.
@@ -205,8 +188,8 @@ parse_config(t_configuration_options *options)
  * Ensure any default values set here are synced with repmgr.conf.sample
  * and any other documentation.
  */
-void
+bool
-_parse_config(t_configuration_options *options, ItemList *error_list)
+parse_config(t_configuration_options *options)
 {
 	FILE	   *fp;
 	char	   *s,
@@ -218,6 +201,9 @@ _parse_config(t_configuration_options *options, ItemList *error_list)
 	PQconninfoOption *conninfo_options;
 	char	   *conninfo_errmsg = NULL;
 
+	/* Collate configuration file errors here for friendlier reporting */
+	static ItemList config_errors = { NULL, NULL };
+
 	bool		node_found = false;
 
 	/* Initialize configuration options with sensible defaults
@@ -225,7 +211,7 @@ _parse_config(t_configuration_options *options, ItemList *error_list)
 	 * to be initialised here
 	 */
 	memset(options->cluster_name, 0, sizeof(options->cluster_name));
-	options->node = UNKNOWN_NODE_ID;
+	options->node = -1;
 	options->upstream_node = NO_UPSTREAM_NODE;
 	options->use_replication_slots = 0;
 	memset(options->conninfo, 0, sizeof(options->conninfo));
@@ -276,7 +262,7 @@ _parse_config(t_configuration_options *options, ItemList *error_list)
 	{
 		log_verbose(LOG_NOTICE, _("no configuration file provided and no default file found - "
 					 "continuing with default values\n"));
-		return;
+		return true;
 	}
 
 	fp = fopen(config_file_path, "r");
@@ -321,11 +307,11 @@ _parse_config(t_configuration_options *options, ItemList *error_list)
 			strncpy(options->cluster_name, value, MAXLEN);
 		else if (strcmp(name, "node") == 0)
 		{
-			options->node = repmgr_atoi(value, "node", error_list, false);
+			options->node = repmgr_atoi(value, "node", &config_errors, false);
 			node_found = true;
 		}
 		else if (strcmp(name, "upstream_node") == 0)
-			options->upstream_node = repmgr_atoi(value, "upstream_node", error_list, false);
+			options->upstream_node = repmgr_atoi(value, "upstream_node", &config_errors, false);
 		else if (strcmp(name, "conninfo") == 0)
 			strncpy(options->conninfo, value, MAXLEN);
 		else if (strcmp(name, "barman_server") == 0)
@@ -356,11 +342,11 @@ _parse_config(t_configuration_options *options, ItemList *error_list)
 			}
 			else
 			{
-				item_list_append(error_list, _("value for 'failover' must be 'automatic' or 'manual'\n"));
+				item_list_append(&config_errors,_("value for 'failover' must be 'automatic' or 'manual'\n"));
 			}
 		}
 		else if (strcmp(name, "priority") == 0)
-			options->priority = repmgr_atoi(value, "priority", error_list, true);
+			options->priority = repmgr_atoi(value, "priority", &config_errors, true);
 		else if (strcmp(name, "node_name") == 0)
 			strncpy(options->node_name, value, MAXLEN);
 		else if (strcmp(name, "promote_command") == 0)
@@ -378,17 +364,17 @@ _parse_config(t_configuration_options *options, ItemList *error_list)
 		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", error_list, false);
+			options->master_response_timeout = repmgr_atoi(value, "master_response_timeout", &config_errors, 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", error_list, false);
+			options->master_response_timeout = repmgr_atoi(value, "primary_response_timeout", &config_errors, false);
 		else if (strcmp(name, "reconnect_attempts") == 0)
-			options->reconnect_attempts = repmgr_atoi(value, "reconnect_attempts", error_list, false);
+			options->reconnect_attempts = repmgr_atoi(value, "reconnect_attempts", &config_errors, false);
 		else if (strcmp(name, "reconnect_interval") == 0)
-			options->reconnect_interval = repmgr_atoi(value, "reconnect_interval", error_list, false);
+			options->reconnect_interval = repmgr_atoi(value, "reconnect_interval", &config_errors, false);
 		else if (strcmp(name, "pg_bindir") == 0)
 			strncpy(options->pg_bindir, value, MAXLEN);
 		else if (strcmp(name, "pg_ctl_options") == 0)
@@ -398,14 +384,14 @@ _parse_config(t_configuration_options *options, ItemList *error_list)
 		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", error_list, false);
+			options->monitor_interval_secs = repmgr_atoi(value, "monitor_interval_secs", &config_errors, false);
 		else if (strcmp(name, "retry_promote_interval_secs") == 0)
-			options->retry_promote_interval_secs = repmgr_atoi(value, "retry_promote_interval_secs", error_list, false);
+			options->retry_promote_interval_secs = repmgr_atoi(value, "retry_promote_interval_secs", &config_errors, 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", error_list, false);
+			options->witness_repl_nodes_sync_interval_secs = repmgr_atoi(value, "witness_repl_nodes_sync_interval_secs", &config_errors, 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", error_list, false);
+			options->use_replication_slots = repmgr_atoi(value, "use_replication_slots", &config_errors, false);
 		else if (strcmp(name, "event_notification_command") == 0)
 			strncpy(options->event_notification_command, value, MAXLEN);
 		else if (strcmp(name, "event_notifications") == 0)
@@ -433,7 +419,7 @@ _parse_config(t_configuration_options *options, ItemList *error_list)
 					 _("no value provided for parameter \"%s\""),
 					 name);
 
-			item_list_append(error_list, error_message_buf);
+			item_list_append(&config_errors, error_message_buf);
 		}
 	}
 
@@ -442,11 +428,11 @@ _parse_config(t_configuration_options *options, ItemList *error_list)
 
 	if (node_found == false)
 	{
-		item_list_append(error_list, _("\"node\": parameter was not found"));
+		item_list_append(&config_errors, _("\"node\": parameter was not found"));
 	}
 	else if (options->node == 0)
 	{
-		item_list_append(error_list, _("\"node\": must be greater than zero"));
+		item_list_append(&config_errors, _("\"node\": must be greater than zero"));
 	}
 
 	if (strlen(options->conninfo))
@@ -466,11 +452,18 @@ _parse_config(t_configuration_options *options, ItemList *error_list)
 					 _("\"conninfo\": %s"),
 					 conninfo_errmsg);
 
-			item_list_append(error_list, error_message_buf);
+			item_list_append(&config_errors, error_message_buf);
 		}
 
 		PQconninfoFree(conninfo_options);
 	}
+
+	if (config_errors.head != NULL)
+	{
+		exit_with_errors(&config_errors);
+	}
+
+	return true;
 }
 
 
@@ -560,85 +553,70 @@ 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_INITIALIZER;
+	t_configuration_options new_options;
 	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\n"));
+	log_info(_("reloading configuration file and updating repmgr tables\n"));
 
-	_parse_config(&new_options, &config_errors);
+	parse_config(&new_options);
-
+	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(_("cluster_name cannot be changed, retaining current configuration\n"));
+		log_warning(_("unable to change cluster name, retaining current configuration\n"));
 		return false;
 	}
 
 	if (new_options.node != orig_options->node)
 	{
-		log_warning(_("node ID cannot be changed, retaining current configuration\n"));
+		log_warning(_("unable to change node ID, retaining current configuration\n"));
 		return false;
 	}
 
 	if (strcmp(new_options.node_name, orig_options->node_name) != 0)
 	{
-		log_warning(_("node_name cannot be changed, keeping current configuration\n"));
+		log_warning(_("unable to change standby name, 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 works*/
+		/* Test conninfo string */
 		conn = establish_db_connection(new_options.conninfo, false);
 		if (!conn || (PQstatus(conn) != CONNECTION_OK))
 		{
@@ -655,6 +633,34 @@ 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;
+	}
+
+	/* barman_server */
+	if (strcmp(orig_options->barman_server, new_options.barman_server) != 0)
+	{
+		strcpy(orig_options->barman_server, new_options.barman_server);
+		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)
 	{
@@ -662,27 +668,6 @@ reload_config(t_configuration_options *orig_options)
 		config_changed = true;
 	}
 
-	/* follow_command */
-	if (strcmp(orig_options->follow_command, new_options.follow_command) != 0)
-	{
-		strcpy(orig_options->follow_command, new_options.follow_command);
-		config_changed = true;
-	}
-
-	/* master_response_timeout */
-	if (orig_options->master_response_timeout != new_options.master_response_timeout)
-	{
-		orig_options->master_response_timeout = new_options.master_response_timeout;
-		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)
 	{
@@ -690,6 +675,13 @@ reload_config(t_configuration_options *orig_options)
 		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)
 	{
@@ -697,6 +689,44 @@ reload_config(t_configuration_options *orig_options)
 		config_changed = true;
 	}
 
+	/* follow_command */
+	if (strcmp(orig_options->follow_command, new_options.follow_command) != 0)
+	{
+		strcpy(orig_options->follow_command, new_options.follow_command);
+		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)
+	{
+		orig_options->master_response_timeout = new_options.master_response_timeout;
+		config_changed = true;
+	}
+
 	/* reconnect_attempts */
 	if (orig_options->reconnect_attempts != new_options.reconnect_attempts)
 	{
@@ -711,6 +741,27 @@ 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)
 	{
@@ -718,54 +769,20 @@ reload_config(t_configuration_options *orig_options)
 		config_changed = true;
 	}
 
-
+	/* use_replication_slots */
-	/* witness_repl_nodes_sync_interval_secs */
+	if (orig_options->use_replication_slots != new_options.use_replication_slots)
-	if (orig_options->witness_repl_nodes_sync_interval_secs != new_options.witness_repl_nodes_sync_interval_secs)
 	{
-		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;
 		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_notice(_("configuration file reloaded with changed parameters\n"));
+		log_debug(_("reload_config(): configuration has changed\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_info(_("configuration has not changed\n"));
+		log_debug(_("reload_config(): configuration has not changed\n"));
 	}
 
 	return config_changed;
@@ -939,7 +956,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;
 
 

config.h

@@ -97,7 +97,7 @@ 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 { "", UNKNOWN_NODE_ID, NO_UPSTREAM_NODE, "", "", "", MANUAL_FAILOVER, -1, "", "", "", "", "", "", "", "", "", "", "", "", -1, -1, -1, "", "", "", "", "", 0, 0, 0, 0, "", { NULL, NULL }, { NULL, NULL } }
+#define T_CONFIGURATION_OPTIONS_INITIALIZER { "", -1, NO_UPSTREAM_NODE, "", "", "", MANUAL_FAILOVER, -1, "", "", "", "", "", "", "", "", "", "", "", "", -1, -1, -1, "", "", "", "", "", 0, 0, 0, 0, "", { NULL, NULL }, { NULL, NULL } }
 
 typedef struct ItemListCell
 {
@@ -131,11 +131,8 @@ 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);
-
-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);
-
+bool		parse_config(t_configuration_options *options);
 void		parse_line(char *buff, char *name, char *value);
 char	   *trim(char *s);
 void		item_list_append(ItemList *item_list, char *error_message);

dbutils.c

@@ -279,6 +279,7 @@ 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)
@@ -1436,11 +1437,10 @@ 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.
-	 * Only attempt to write a record if a connection handle was provided.
+	   Also check that the repmgr schema has been properly intialised - if
-	 * Also check that the repmgr schema has been properly initialised - if
+	   not it means no configuration file was provided, which can happen with
-	 * 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.
-	 * 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)
 	{
@@ -1629,89 +1629,6 @@ 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)

dbutils.h

@@ -130,7 +130,6 @@ 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);

docs/repmgrd-node-fencing.md

@@ -22,14 +22,15 @@ of this document).
 
 * * *
 
-In a failover situation, `repmgrd` promotes a standby to master by executing
+In a failover situation, `repmgrd` promotes a standby to master by
-the command defined in `promote_command`. Normally this would be something like:
+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
+By wrapping this in a custom script which adjusts the `pgbouncer`
-on all nodes, it's possible to fence the failed master and redirect write
+configuration on all nodes, it's possible to fence the failed master
-connections to the new master.
+and redirect write connections to the new master.
 
 The script consists of three sections:
 
@@ -37,19 +38,20 @@ The script consists of three sections:
 * 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
+Note that it requires password-less SSH access between all nodes to be
-update the `pgbouncer` configuration files.
+able to update the `pgbouncer` configuration files.
 
-For the purposes of this demonstration, we'll assume there are 3 nodes (master
+For the purposes of this demonstration, we'll assume there are 3 nodes
-and two standbys), with `pgbouncer` listening on port 6432 handling connections
+(master and two standbys), with `pgbouncer` listening on port 6432
-to a database called `appdb`.  The `postgres` system user must have write
+handling connections to a database called `appdb`. The `postgres`
-access to the `pgbouncer` configuration files on all nodes. We'll assume
+system user must have write access to the `pgbouncer` configuration
-there's a main `pgbouncer` configuration file, `/etc/pgbouncer.ini`, which uses
+file on all nodes, assumed to be at `/etc/pgbouncer.ini`.
-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:
+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]
 
@@ -78,8 +80,6 @@ configuration file, `/etc/pgbouncer.database.ini`, which will be modified by
     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`
@@ -91,52 +91,50 @@ The actual script is as follows; adjust the configurable items as appropriate:
 
     # 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"
+    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 $PGBOUNCER_PORT -U postgres pgbouncer
+        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_DATABASE_INI_NEW="/tmp/pgbouncer.database.ini"
+    PGBOUNCER_INI_NEW="/tmp/pgbouncer.ini.new"
 
     for HOST in $PGBOUNCER_HOSTS
     do
         # Recreate the pgbouncer config file
-        echo -e "[databases]\n" > $PGBOUNCER_DATABASE_INI_NEW
+        echo -e "[databases]\n" > $PGBOUNCER_INI_NEW
 
         psql -d $REPMGR_DB -U $REPMGR_USER -t -A \
-          -c "SELECT '${PGBOUNCER_DATABASE}-rw= ' || conninfo || ' application_name=pgbouncer_${HOST}' \
+          -c "SELECT '$PGBOUNCER_DATABASE= ' || 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
+              WHERE active = TRUE AND type='master'" >> $PGBOUNCER_INI_NEW
 
-        rsync $PGBOUNCER_DATABASE_INI_NEW $HOST:$PGBOUNCER_DATABASE_INI
+        cat $PGBOUNCER_INI_TEMPLATE >> $PGBOUNCER_INI_NEW
 
-        psql -tc "reload" -h $HOST -p $PGBOUNCER_PORT -U postgres pgbouncer
+        rsync $PGBOUNCER_INI_NEW $HOST:$PGBOUNCER_CONFIG
-        psql -tc "resume" -h $HOST -p $PGBOUNCER_PORT -U postgres pgbouncer
+
+        psql -tc "reload" -h $HOST -U postgres pgbouncer
+        psql -tc "resume" -h $HOST -U postgres pgbouncer
 
     done
 
     # Clean up generated file
-    rm $PGBOUNCER_DATABASE_INI_NEW
+    rm $PGBOUNCER_INI_NEW
 
     echo "Reconfiguration of pgbouncer complete"
 

log.c

@@ -48,11 +48,6 @@ 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, ...)
@@ -67,7 +62,9 @@ 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)
 {
-	char		buf[100];
+	time_t		t;
+	struct tm  *tm;
+	char		buff[100];
 
 	/*
 	 * Store the requested level so that if there's a subsequent
@@ -77,21 +74,10 @@ _stderr_log_with_level(const char *level_name, int level, const char *fmt, va_li
 
 	if (log_level >= level)
 	{
-
+		time(&t);
-		/* Format log line prefix with timestamp if in daemon mode */
+		tm = localtime(&t);
-		if (logger_output_mode == OM_DAEMON)
+		strftime(buff, 100, "[%Y-%m-%d %H:%M:%S]", tm);
-		{
+		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);
 
@@ -190,13 +176,6 @@ 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)
 	{
 
@@ -257,10 +236,9 @@ 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;
-		 * 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(),
-		 * as it's not feasible to restore stderr after a failed freopen(),
+		   we'll write to stdout as a last resort.
-		 * we'll write to stdout as a last resort.
 		 */
 		if (fd == NULL)
 		{

log.h

@@ -25,9 +25,6 @@
 #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)));
@@ -135,6 +132,5 @@ extern int	log_type;
 extern int	log_level;
 extern int	verbose_logging;
 extern int	terse_logging;
-extern int	logger_output_mode;
 
 #endif /* _REPMGR_LOG_H_ */

repmgr.conf.sample

@@ -66,12 +66,6 @@
 # (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
@@ -80,12 +74,6 @@
 #
 #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:
 #
@@ -175,8 +163,9 @@
 # monitoring interval in seconds; default is 2
 #monitor_interval_secs=2
 
-# Maximum number of seconds to wait for a response from the primary server
+# Number of seconds to wait for a response from the primary server before
-# before deciding it has failed.
+# deciding it has failed.
+
 #master_response_timeout=60
 
 # Number of attempts at what interval (in seconds) to try and

repmgr.h

@@ -62,10 +62,6 @@
 #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
@@ -84,11 +80,10 @@ typedef struct
 	bool		verbose;
 	bool		terse;
 	bool		force;
-	char		pg_bindir[MAXLEN]; /* overrides setting in repmgr.conf */
 
-	/* logging parameters */
+	/* options which override setting in repmgr.conf */
-	char		loglevel[MAXLEN];  /* overrides setting in repmgr.conf */
+	char		loglevel[MAXLEN];
-	bool		log_to_file;
+	char		pg_bindir[MAXLEN];
 
 	/* connection parameters */
 	char		dbname[MAXLEN];
@@ -110,18 +105,13 @@ typedef struct
 	bool		fast_checkpoint;
 	bool		without_barman;
 	bool		no_upstream_connection;
-	bool		no_conninfo_password;
 	bool		copy_external_config_files;
 	int			copy_external_config_files_destination;
-	char		upstream_conninfo[MAXLEN];
-	char		replication_user[MAXLEN];
-
-	char		recovery_min_apply_delay[MAXLEN];
-
-	/* standby register paarameters */
 	bool		wait_register_sync;
 	int			wait_register_sync_seconds;
 
+	char		recovery_min_apply_delay[MAXLEN];
+
 	/* witness create parameters */
 	bool		witness_pwprompt;
 
@@ -150,17 +140,15 @@ typedef struct
 
 #define T_RUNTIME_OPTIONS_INITIALIZER { \
 		/* general repmgr options */	\
-		"", false, false, false, "",	\
+		"", false, false, false,		\
-		/* logging parameters */ \
+		/* options which override setting in repmgr.conf */ \
-		"", false,                      \
+		"", "",                         \
 		/* connection parameters */		\
 		"", "", "", "", "", "", "", 	\
 		false, false, false,		    \
 		/* standby clone parameters */  \
 		false, DEFAULT_WAL_KEEP_SEGMENTS, false, false, false, false, false, false, \
-		false, CONFIG_FILE_SAMEPATH, "", "", "", \
+		CONFIG_FILE_SAMEPATH, false, 0, "", \
-		/* standby register paarameters */ \
-	    false, 0,							 \
 		/* witness create parameters */ \
 		false,                          \
 		/* standby follow parameters */ \

repmgrd.c

@@ -207,13 +207,6 @@ main(int argc, char **argv)
 		}
 	}
 
-
-	/*
-	 * 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
@@ -254,7 +247,6 @@ main(int argc, char **argv)
 	}
 
 	logger_init(&local_options, progname());
-
 	if (verbose)
 		logger_set_verbose();
 
@@ -662,8 +654,8 @@ witness_monitor(void)
 				 * Update the repl_nodes table from the new master to reflect the changed
 				 * node configuration
 				 *
-				 * It would be neat to be able to handle this with e.g. table-based
+				 * XXX it would be neat to be able to handle this with e.g. table-based
-				 * logical replication if available in core
+				 * logical replication
 				 */
 				witness_copy_node_records(master_conn, my_local_conn, local_options.cluster_name);
 
@@ -782,6 +774,7 @@ 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;
@@ -963,8 +956,6 @@ 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)
@@ -1022,8 +1013,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 warning
+				 * XXX check if the original master is still active and display a
-				 * XXX add event notification
+				 * warning
 				 */
 				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"));
@@ -1068,6 +1059,9 @@ standby_monitor(void)
 	 * from the upstream node to write monitoring information
 	 */
 
+	/* XXX not used? */
+	upstream_node = get_node_info(my_local_conn, local_options.cluster_name, upstream_node_id);
+
 	sprintf(sqlquery,
 			"SELECT id "
 			"  FROM %s.repl_nodes "
@@ -1519,6 +1513,7 @@ 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"));
@@ -2091,21 +2086,18 @@ check_connection(PGconn **conn, const char *type, const char *conninfo)
 /*
  * set_local_node_status()
  *
- * Attempt to connect to the current master server (as stored in the global
+ * If failure of the local node is detected, attempt to connect
- * variable `master_conn`) and set the local node's status to the result
+ * to the current master server (as stored in the global variable
- * of `is_standby(my_local_conn)`. Normally this will be used to mark
+ * `master_conn`) and update its record to failed.
- * 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))
 	{
@@ -2164,29 +2156,24 @@ 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,
-					    local_node_status))
+					    is_standby(my_local_conn)==1))
 	{
-		log_err(_("unable to set local node %i as %s on master: %s\n"),
+		log_err(_("unable to set local node %i as inactive 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 %s on master\n"),
+	log_notice(_("marking this node (%i) as inactive on master\n"), node_info.node_id);
-			   node_info.node_id,
-			   local_node_status == false ? "inactive" : "active");
-
 	return true;
 }
 

strutil.c

@@ -90,18 +90,31 @@ maxlen_snprintf(char *str, const char *format,...)
 
 
 /*
- * Escape a string for use as a parameter in recovery.conf
+ * Adapted from: src/fe_utils/string_utils.c
- * Caller must free returned value
+ *
+ * Function not publicly available before PostgreSQL 9.6.
  */
-char *
+void
-escape_recovery_conf_value(const char *src)
+appendShellString(PQExpBuffer buf, const char *str)
 {
-	char	   *result = escape_single_quotes_ascii(src);
+	const char *p;
 
-	if (!result)
+	appendPQExpBufferChar(buf, '\'');
+	for (p = str; *p; p++)
 	{
-		fprintf(stderr, _("%s: out of memory\n"), progname());
+		if (*p == '\n' || *p == '\r')
-		exit(ERR_INTERNAL);
+		{
+			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);
 	}
-	return result;
+
+	appendPQExpBufferChar(buf, '\'');
 }

strutil.h

@@ -49,6 +49,6 @@ extern int
 maxlen_snprintf(char *str, const char *format,...)
 __attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 3)));
 
-extern char *
+extern void
-escape_recovery_conf_value(const char *src);
+appendShellString(PQExpBuffer buf, const char *str);
 #endif   /* _STRUTIL_H_ */

version.h

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