repmgr: only require 'wal_keep_segments' to be set in certain corner cases

Now that repmgr uses pg_basebackup's `--xlog-method=stream` setting by
default, and enables provision of `restore_command`, there's no reason
to require `wal_keep_segments` to be set in the default use-case.

`repmgr standby clone` will now only fail with an error if `wal_keep_segments`
is zero and one of the following cases applies:

* `--rsync-only` clone with no `restore_command` set
* clone with pg_basebackup and `--xlog-method=fetch`
* -w/--wal-keep-segments specified on the command line

If, for whatever reason, it's necessary to perform a standby clone
with `wal_keep_segments=0` in one of the above cases, specifying
`-w/--wal-keep-segments=0` on the command line will effectively
override the check.

GitHub #204

README.md

@@ -287,7 +287,7 @@ both servers.
 ### PostgreSQL configuration
 
 On the master server, a PostgreSQL instance must be initialised and running.
-The following replication settings must be included in `postgresql.conf`:
+The following replication settings may need to be adjusted:
 
 
     # Enable replication connections; set this figure to at least one more
@@ -302,13 +302,6 @@ The following replication settings must be included in `postgresql.conf`:
 
     wal_level = 'hot_standby'
 
-    # How much WAL to retain on the master to allow a temporarily
-    # disconnected standby to catch up again. The larger this is, the
-    # longer the standby can be disconnected. This is needed only in
-    # 9.3; from 9.4, replication slots can be used instead (see below).
-
-    wal_keep_segments = 5000
-
     # Enable read-only queries on a standby
     # (Note: this will be ignored on a master but we recommend including
     # it anyway)
@@ -323,6 +316,14 @@ The following replication settings must be included in `postgresql.conf`:
     # ignores archiving. Use something more sensible.
     archive_command = '/bin/true'
 
+    # If cloning using rsync, or you have configured `pg_basebackup_options`
+    # in `repmgr.conf` to include the setting `--xlog-method=fetch`, *and*
+    # you have not set `restore_command` in `repmgr.conf`to fetch WAL files
+    # from another source such as Barman, you'll need to set `wal_keep_segments`
+    # to a high enough value to ensure that all WAL files generated while
+    # the standby is being cloned are retained until the standby starts up.
+
+    # wal_keep_segments = 5000
 
 * * *
 

repmgr.c

@@ -6478,7 +6478,17 @@ check_upstream_config(PGconn *conn, int server_version_num, bool exit_on_error)
 	int			i;
 	bool		config_ok = true;
 	char	   *wal_error_message = NULL;
+	t_basebackup_options  backup_options = T_BASEBACKUP_OPTIONS_INITIALIZER;
+	bool		xlog_stream = true;
 
+	/*
+	 * 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)
 	{
@@ -6582,35 +6592,66 @@ check_upstream_config(PGconn *conn, int server_version_num, bool exit_on_error)
 
 	}
 	/*
-	 * physical replication slots not available or not requested, and Barman mode not used -
+	 * physical replication slots not available or not requested - check if
-	 * ensure some reasonably high value set for `wal_keep_segments`
+	 * there are any circumstances where `wal_keep_segments` should be set
 	 */
 	else if (! runtime_options.without_barman && strcmp(options.barman_server, "") == 0)
 	{
-		i = guc_set_typed(conn, "wal_keep_segments", ">=",
+		bool check_wal_keep_segments = false;
-						  runtime_options.wal_keep_segments, "integer");
+		char min_wal_keep_segments[MAXLEN] = "1";
-		if (i == 0 || i == -1)
+
+		/*
+		 * -w/--wal-keep-segments was supplied - check against that value
+		 */
+		if (wal_keep_segments_used == true)
 		{
-			if (i == 0)
+			check_wal_keep_segments = true;
+			strncpy(min_wal_keep_segments, runtime_options.wal_keep_segments, MAXLEN);
+		}
+
+		/*
+		 * A non-zero `wal_keep_segments` value will almost certainly be required
+		 * if rsync mode is being used, or pg_basebackup with --xlog-method=fetch,
+		 * *and* no restore command has been specified
+		 */
+		else if ( (runtime_options.rsync_only == true || xlog_stream == false)
+			 && strcmp(options.restore_command, "") == 0)
+		{
+			check_wal_keep_segments = true;
+		}
+
+		if (check_wal_keep_segments == true)
+		{
+			i = guc_set_typed(conn, "wal_keep_segments", ">=", min_wal_keep_segments, "integer");
+
+			if (i == 0 || i == -1)
 			{
-				log_err(_("parameter 'wal_keep_segments' must be be set to %s or greater (see the '-w' option or edit the postgresql.conf of the upstream server.)\n"),
+				if (i == 0)
-						runtime_options.wal_keep_segments);
-				if (server_version_num >= 90400)
 				{
-					log_hint(_("in PostgreSQL 9.4 and later, replication slots can be used, which "
+					log_err(_("parameter 'wal_keep_segments' on the upstream server must be be set to %s or greater\n"),
-							   "do not require 'wal_keep_segments' to be set to a high value "
+							min_wal_keep_segments);
-							   "(set parameter 'use_replication_slots' in the configuration file to enable)\n"
+					log_hint(_("Choose a value sufficiently high enough to retain enough WAL "
+							   "until the standby has been cloned and started.\n "
+							   "Alternatively set up WAL archiving using e.g. PgBarman and configure "
+							   "'restore_command' in repmgr.conf to fetch WALs from there.\n"
 								 ));
+					if (server_version_num >= 90400)
+					{
+						log_hint(_("In PostgreSQL 9.4 and later, replication slots can be used, which "
+								   "do not require 'wal_keep_segments' to be set "
+								   "(set parameter 'use_replication_slots' in repmgr.conf to enable)\n"
+									 ));
+					}
 				}
-			}
 
-			if (exit_on_error == true)
+				if (exit_on_error == true)
-			{
+				{
-				PQfinish(conn);
+					PQfinish(conn);
-				exit(ERR_BAD_CONFIG);
+					exit(ERR_BAD_CONFIG);
-			}
+				}
 
-			config_ok = false;
+				config_ok = false;
+			}
 		}
 	}
 
@@ -6687,18 +6728,14 @@ check_upstream_config(PGconn *conn, int server_version_num, bool exit_on_error)
 
 	if (!runtime_options.rsync_only)
 	{
-		t_basebackup_options  backup_options = T_BASEBACKUP_OPTIONS_INITIALIZER;
-		PGconn	  **connections;
-		bool		xlog_stream = true;
-		int			i;
 
+		PGconn	  **connections;
+		int			i;
 		int			min_replication_connections = 1,
 					possible_replication_connections = 0;
 
 		t_conninfo_param_list repl_conninfo;
 
-		parse_pg_basebackup_options(options.pg_basebackup_options, &backup_options);
-
 		/* Make a copy of the connection parameter arrays, and append "replication" */
 
 		initialize_conninfo_params(&repl_conninfo, false);
@@ -6715,8 +6752,6 @@ check_upstream_config(PGconn *conn, int server_version_num, bool exit_on_error)
 		 * --xlog-method set by user to something other than "stream" (should be "fetch",
 		 * but we're just checking it's not "stream")
 		 */
-		if (strlen(backup_options.xlog_method) && strcmp(backup_options.xlog_method, "stream") != 0)
-			xlog_stream = false;
 
 		if (xlog_stream == true)
 			min_replication_connections += 1;
@@ -6751,7 +6786,6 @@ check_upstream_config(PGconn *conn, int server_version_num, bool exit_on_error)
 			log_err(_("unable to establish necessary replication connections\n"));
 			log_hint(_("increase 'max_wal_senders' by at least %i\n"), min_replication_connections - possible_replication_connections);
 
-
 			if (exit_on_error == true)
 			{
 				PQfinish(conn);

repmgr.h

@@ -36,7 +36,7 @@
 
 #define ERRBUFF_SIZE	512
 
-#define DEFAULT_WAL_KEEP_SEGMENTS	"5000"
+#define DEFAULT_WAL_KEEP_SEGMENTS	"0"
 #define DEFAULT_DEST_DIR		"."
 #define DEFAULT_REPMGR_SCHEMA_PREFIX	"repmgr_"
 #define DEFAULT_PRIORITY		100