Finalize release date

Probably a remnant from the 9.1 era, where it was not possible to
take a base backup from a standby.

HISTORY

@@ -1,3 +1,14 @@
+5.2.1   2020-12-07
+        config: fix parsing of "replication_type"; GitHub #672 (Ian)
+        standby clone: handle missing "postgresql.auto.conf" (Ian)
+        standby clone: add option --recovery-min-apply-delay (Ian)
+        standby clone: fix data directory permissions handling for
+          PostgreSQL 11 and later (Ian)
+        repmgrd: prevent termination when local node not available and
+          standby_disconnect_on_failover; GitHub #675 (Ian)
+        repmgrd: ensure reconnect_interval" is correctly handled;
+          GitHub #673 (Ian)
+
 5.2.0   2020-10-22
         general: add support for PostgreSQL 13 (Ian)
         general: remove support for PostgreSQL 9.3 (Ian)

Makefile.in

@@ -76,10 +76,10 @@ configfile-scan.c: configfile-scan.l
 $(REPMGR_CLIENT_OBJS): repmgr-client.h repmgr_version.h
 
 repmgr: $(REPMGR_CLIENT_OBJS)
-	$(CC) $(CFLAGS) $(REPMGR_CLIENT_OBJS) $(libpq_pgport) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X)
+	$(CC) $(CFLAGS) $(REPMGR_CLIENT_OBJS) $(libpq_pgport) $(LDFLAGS) $(LDFLAGS_EX) -o $@$(X)
 
 repmgrd: $(REPMGRD_OBJS)
-	$(CC) $(CFLAGS) $(REPMGRD_OBJS) $(libpq_pgport) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X)
+	$(CC) $(CFLAGS) $(REPMGRD_OBJS) $(libpq_pgport) $(LDFLAGS) $(LDFLAGS_EX) -o $@$(X)
 
 $(REPMGR_CLIENT_OBJS): $(HEADERS)
 $(REPMGRD_OBJS): $(HEADERS)

README.md

@@ -7,8 +7,8 @@ replication capabilities with utilities to set up standby servers, monitor
 replication, and perform administrative tasks such as failover or switchover
 operations.
 
-PostgreSQL 12, 11, 10, 9.6 and 9.5 are fully supported.
-PostgreSQL 9.4 and 9.3 are supported, with some restrictions.
+The most recent `repmgr` version (5.2.0) supports all PostgreSQL versions from
+9.5 to 13. PostgreSQL 9.4 is also supported, with some restrictions.
 
 `repmgr` is distributed under the GNU GPL 3 and maintained by 2ndQuadrant.
 
@@ -24,7 +24,8 @@ The old `README` file for `repmgr` 3.x is available here:
 > https://github.com/2ndQuadrant/repmgr/blob/REL3_3_STABLE/README.md
 
 Note that the `repmgr` 3.x series is no longer supported and contains known bugs;
-please upgrade to the current `repmgr` version as soon as possible.
+please upgrade to the [current repmgr version](https://repmgr.org/docs/current/appendix-release-notes.html)
+as soon as possible.
 
 Versions
 --------
@@ -70,8 +71,6 @@ Please report bugs and other issues to:
 
 * https://github.com/2ndQuadrant/repmgr
 
-See 
-
 Further information is available at https://repmgr.org/
 
 We'd love to hear from you about how you use repmgr. Case studies and
@@ -100,7 +99,3 @@ Further reading
 * [repmgr documentation](https://repmgr.org/docs/current/index.html)
 * [How to Automate PostgreSQL 12 Replication and Failover with repmgr - Part 1](https://www.2ndquadrant.com/en/blog/how-to-automate-postgresql-12-replication-and-failover-with-repmgr-part-1/)
 * [How to Automate PostgreSQL 12 Replication and Failover with repmgr - Part 2](https://www.2ndquadrant.com/en/blog/how-to-automate-postgresql-12-replication-and-failover-with-repmgr-part-2/)
-* https://blog.2ndquadrant.com/repmgr-3-2-is-here-barman-support-brand-new-high-availability-features/
-* https://blog.2ndquadrant.com/improvements-in-repmgr-3-1-4/
-* https://blog.2ndquadrant.com/managing-useful-clusters-repmgr/
-* https://blog.2ndquadrant.com/easier_postgresql_90_clusters/

configdata.c

@@ -120,10 +120,10 @@ struct ConfigFileSetting config_file_settings[] =
 	/* replication_type */
 	{
 		"replication_type",
-		CONFIG_INT,
-		{ .intptr = &config_file_options.replication_type },
-		{ .intdefault = REPLICATION_TYPE_PHYSICAL },
-		{ .intminval = -1 },
+		CONFIG_REPLICATION_TYPE,
+		{ .replicationtypeptr = &config_file_options.replication_type },
+		{ .replicationtypedefault = DEFAULT_REPLICATION_TYPE },
+		{},
 		{},
 		{}
 	},

configfile-scan.l

@@ -107,9 +107,9 @@ ProcessRepmgrConfigFile(const char *config_file, const char *base_dir, ItemList
 
 
 extern bool
-ProcessPostgresConfigFile(const char *config_file, const char *base_dir, KeyValueList *contents, ItemList *error_list, ItemList *warning_list)
+ProcessPostgresConfigFile(const char *config_file, const char *base_dir, bool strict, KeyValueList *contents, ItemList *error_list, ItemList *warning_list)
 {
-	return ProcessConfigFile(base_dir, config_file, NULL, true, 0, contents, error_list, warning_list);
+	return ProcessConfigFile(base_dir, config_file, NULL, strict, 0, contents, error_list, warning_list);
 }
 
 static bool

configfile.c

@@ -313,6 +313,9 @@ _parse_config(ItemList *error_list, ItemList *warning_list)
 			case CONFIG_CONNECTION_CHECK_TYPE:
 				*setting->val.checktypeptr = setting->defval.checktypedefault;
 				break;
+			case CONFIG_REPLICATION_TYPE:
+				*setting->val.replicationtypeptr = setting->defval.replicationtypedefault;
+				break;
 			case CONFIG_EVENT_NOTIFICATION_LIST:
 			case CONFIG_TABLESPACE_MAPPING:
 				/* no default for these types; lists cleared above */
@@ -566,6 +569,20 @@ parse_configuration_item(ItemList *error_list, ItemList *warning_list, const cha
 					}
 					break;
 				}
+				case CONFIG_REPLICATION_TYPE:
+				{
+					if (strcasecmp(value, "physical") == 0)
+					{
+						*(ReplicationType *)setting->val.replicationtypeptr = REPLICATION_TYPE_PHYSICAL;
+					}
+					else
+					{
+						item_list_append_format(error_list,
+												_("value for \"%s\" must be \"physical\"\n"),
+												name);
+					}
+					break;
+				}
 				case CONFIG_EVENT_NOTIFICATION_LIST:
 				{
 					parse_event_notifications_list((EventNotificationList *)setting->val.notificationlistptr,
@@ -1394,6 +1411,9 @@ dump_config(void)
 			case CONFIG_CONNECTION_CHECK_TYPE:
 				printf("%s", print_connection_check_type(*setting->val.checktypeptr));
 				break;
+			case CONFIG_REPLICATION_TYPE:
+				printf("%s", print_replication_type(*setting->val.replicationtypeptr));
+				break;
 			case CONFIG_EVENT_NOTIFICATION_LIST:
 			{
 				char *list = print_event_notification_list(setting->val.notificationlistptr);
@@ -1765,7 +1785,7 @@ modify_auto_conf(const char *data_dir, KeyValueList *items)
 
 	FILE	   *fp;
 	mode_t		um;
-	struct stat auto_conf_st;
+	struct stat data_dir_st;
 
 	KeyValueList config = {NULL, NULL};
 	KeyValueListCell *cell = NULL;
@@ -1776,7 +1796,12 @@ modify_auto_conf(const char *data_dir, KeyValueList *items)
 	appendPQExpBuffer(&auto_conf, "%s/%s",
 					  data_dir, PG_AUTOCONF_FILENAME);
 
-	success = ProcessPostgresConfigFile(auto_conf.data, NULL, &config, NULL, NULL);
+	success = ProcessPostgresConfigFile(auto_conf.data,
+										NULL,
+										false, /* we don't care if the file does not exist */
+										&config,
+										NULL,
+										NULL);
 
 	if (success == false)
 	{
@@ -1787,7 +1812,7 @@ modify_auto_conf(const char *data_dir, KeyValueList *items)
 	}
 
 	/*
-	 * Append requested items to items extracted from the existing file.
+	 * Append requested items to any items extracted from the existing file.
 	 */
 	for (cell = items->head; cell; cell = cell->next)
 	{
@@ -1816,27 +1841,46 @@ modify_auto_conf(const char *data_dir, KeyValueList *items)
 						  cell->key, cell->value);
 	}
 
-	stat(auto_conf.data, &auto_conf_st);
+	/* stat the data directory for the file mode */
+	if (stat(data_dir, &data_dir_st) != 0)
+	{
+		/*
+		 * This is highly unlikely to happen, but if it does (e.g. freak
+		 * race condition with some rogue process which is messing about
+		 * with the data directory), there's not a lot we can do.
+		 */
+		log_error(_("error encountered when checking \"%s\""),
+				  data_dir);
+		log_detail("%s", strerror(errno));
+		exit(ERR_BAD_CONFIG);
+	}
 
 	/*
-	 * Set umask so the temporary file is created in the same mode as the original
-	 * postgresql.auto.conf file.
+	 * Set umask so the temporary file is created in the same mode as the data
+	 * directory. In PostgreSQL 11 and later this can be 0700 or 0750.
 	 */
-	um = umask(~(auto_conf_st.st_mode));
+	um = umask(~(data_dir_st.st_mode));
+
 	fp = fopen(auto_conf_tmp.data, "w");
+
 	umask(um);
 
 	if (fp == NULL)
 	{
-		fprintf(stderr, "unable to open \"%s\": %s\n",
+		fprintf(stderr, "unable to open \"%s\" for writing: %s\n",
 				auto_conf_tmp.data,
 				strerror(errno));
+		success = false;
 	}
 	else
 	{
 		if (fwrite(auto_conf_contents.data, strlen(auto_conf_contents.data), 1, fp) != 1)
 		{
+			fprintf(stderr, "unable to write to \"%s\": %s\n",
+					auto_conf_tmp.data,
+					strerror(errno));
 			fclose(fp);
+			success = false;
 		}
 		else
 		{
@@ -2167,6 +2211,20 @@ parse_pg_basebackup_options(const char *pg_basebackup_options, t_basebackup_opti
 }
 
 
+const char *
+print_replication_type(ReplicationType type)
+{
+	switch (type)
+	{
+		case REPLICATION_TYPE_PHYSICAL:
+			return "physical";
+	}
+
+	/* should never reach here */
+	return "UNKNOWN";
+}
+
+
 const char *
 print_connection_check_type(ConnectionCheckType type)
 {
@@ -2192,6 +2250,7 @@ print_event_notification_list(EventNotificationList *list)
 	PQExpBufferData buf;
 	char *ptr;
 	EventNotificationListCell *cell;
+	int ptr_len;
 
 	initPQExpBuffer(&buf);
 	cell = list->head;
@@ -2206,8 +2265,10 @@ print_event_notification_list(EventNotificationList *list)
 		cell = cell->next;
 	}
 
-	ptr = palloc0(strlen(buf.data) + 1);
-	strncpy(ptr, buf.data, strlen(buf.data));
+	ptr_len = strlen(buf.data);
+	ptr = palloc0(ptr_len + 1);
+
+	strncpy(ptr, buf.data, ptr_len);
 
 	termPQExpBuffer(&buf);
 

configfile.h

@@ -50,6 +50,11 @@ typedef enum
 	CHECK_CONNECTION
 } ConnectionCheckType;
 
+typedef enum
+{
+	REPLICATION_TYPE_PHYSICAL
+} ReplicationType;
+
 typedef struct EventNotificationListCell
 {
 	struct EventNotificationListCell *next;
@@ -86,7 +91,8 @@ typedef enum
 	CONFIG_FAILOVER_MODE,
 	CONFIG_CONNECTION_CHECK_TYPE,
 	CONFIG_EVENT_NOTIFICATION_LIST,
-	CONFIG_TABLESPACE_MAPPING
+	CONFIG_TABLESPACE_MAPPING,
+    CONFIG_REPLICATION_TYPE
 } ConfigItemType;
 
 
@@ -103,6 +109,7 @@ typedef struct ConfigFileSetting
 		ConnectionCheckType *checktypeptr;
 		EventNotificationList *notificationlistptr;
 		TablespaceList *tablespacemappingptr;
+		ReplicationType *replicationtypeptr;
 	} val;
 	union {
 		int			intdefault;
@@ -110,6 +117,7 @@ typedef struct ConfigFileSetting
 		bool		booldefault;
 		failover_mode_opt failovermodedefault;
 		ConnectionCheckType checktypedefault;
+		ReplicationType replicationtypedefault;
 	} defval;
 	union {
 		int				intminval;
@@ -138,7 +146,7 @@ typedef struct
 	char		config_directory[MAXPGPATH];
 	char		pg_bindir[MAXPGPATH];
 	char		repmgr_bindir[MAXPGPATH];
-	int			replication_type;
+	ReplicationType	replication_type;
 
 	/* log settings */
 	char		log_level[MAXLEN];
@@ -356,6 +364,7 @@ const char *format_failover_mode(failover_mode_opt failover);
 void		exit_with_cli_errors(ItemList *error_list, const char *repmgr_command);
 
 void		print_item_list(ItemList *item_list);
+const char *print_replication_type(ReplicationType type);
 const char *print_connection_check_type(ConnectionCheckType type);
 char 	   *print_event_notification_list(EventNotificationList *list);
 char 	   *print_tablespace_mapping(TablespaceList *tablespacemappingptr);
@@ -364,6 +373,6 @@ extern bool modify_auto_conf(const char *data_dir, KeyValueList *items);
 
 extern bool ProcessRepmgrConfigFile(const char *config_file, const char *base_dir, ItemList *error_list, ItemList *warning_list);
 
-extern bool ProcessPostgresConfigFile(const char *config_file, const char *base_dir, KeyValueList *contents, ItemList *error_list, ItemList *warning_list);
+extern bool ProcessPostgresConfigFile(const char *config_file, const char *base_dir, bool strict, KeyValueList *contents, ItemList *error_list, ItemList *warning_list);
 
 #endif							/* _REPMGR_CONFIGFILE_H_ */

configure

@@ -1,6 +1,6 @@
 #! /bin/sh
 # Guess values for system-dependent variables and create Makefiles.
-# Generated by GNU Autoconf 2.69 for repmgr 5.2.0.
+# Generated by GNU Autoconf 2.69 for repmgr 5.2.1.
 #
 # Report bugs to <repmgr@googlegroups.com>.
 #
@@ -582,8 +582,8 @@ MAKEFLAGS=
 # Identity of this package.
 PACKAGE_NAME='repmgr'
 PACKAGE_TARNAME='repmgr'
-PACKAGE_VERSION='5.2.0'
-PACKAGE_STRING='repmgr 5.2.0'
+PACKAGE_VERSION='5.2.1'
+PACKAGE_STRING='repmgr 5.2.1'
 PACKAGE_BUGREPORT='repmgr@googlegroups.com'
 PACKAGE_URL='https://repmgr.org/'
 
@@ -1181,7 +1181,7 @@ if test "$ac_init_help" = "long"; then
   # Omit some internal or obsolete options to make the list less imposing.
   # This message is too long to be a string in the A/UX 3.1 sh.
   cat <<_ACEOF
-\`configure' configures repmgr 5.2.0 to adapt to many kinds of systems.
+\`configure' configures repmgr 5.2.1 to adapt to many kinds of systems.
 
 Usage: $0 [OPTION]... [VAR=VALUE]...
 
@@ -1242,7 +1242,7 @@ fi
 
 if test -n "$ac_init_help"; then
   case $ac_init_help in
-     short | recursive ) echo "Configuration of repmgr 5.2.0:";;
+     short | recursive ) echo "Configuration of repmgr 5.2.1:";;
    esac
   cat <<\_ACEOF
 
@@ -1316,7 +1316,7 @@ fi
 test -n "$ac_init_help" && exit $ac_status
 if $ac_init_version; then
   cat <<\_ACEOF
-repmgr configure 5.2.0
+repmgr configure 5.2.1
 generated by GNU Autoconf 2.69
 
 Copyright (C) 2012 Free Software Foundation, Inc.
@@ -1335,7 +1335,7 @@ cat >config.log <<_ACEOF
 This file contains any messages produced by compilers while
 running configure, to aid debugging if configure makes a mistake.
 
-It was created by repmgr $as_me 5.2.0, which was
+It was created by repmgr $as_me 5.2.1, which was
 generated by GNU Autoconf 2.69.  Invocation command line was
 
   $ $0 $@
@@ -2487,7 +2487,7 @@ cat >>$CONFIG_STATUS <<\_ACEOF || ac_write_fail=1
 # report actual input values of CONFIG_FILES etc. instead of their
 # values after options handling.
 ac_log="
-This file was extended by repmgr $as_me 5.2.0, which was
+This file was extended by repmgr $as_me 5.2.1, which was
 generated by GNU Autoconf 2.69.  Invocation command line was
 
   CONFIG_FILES    = $CONFIG_FILES
@@ -2550,7 +2550,7 @@ _ACEOF
 cat >>$CONFIG_STATUS <<_ACEOF || ac_write_fail=1
 ac_cs_config="`$as_echo "$ac_configure_args" | sed 's/^ //; s/[\\""\`\$]/\\\\&/g'`"
 ac_cs_version="\\
-repmgr config.status 5.2.0
+repmgr config.status 5.2.1
 configured by $0, generated by GNU Autoconf 2.69,
   with options \\"\$ac_cs_config\\"
 

configure.in

@@ -1,4 +1,4 @@
-AC_INIT([repmgr], [5.2.0], [repmgr@googlegroups.com], [repmgr], [https://repmgr.org/])
+AC_INIT([repmgr], [5.2.1], [repmgr@googlegroups.com], [repmgr], [https://repmgr.org/])
 
 AC_COPYRIGHT([Copyright (c) 2010-2020, 2ndQuadrant Ltd.])
 

controldata.c

@@ -333,7 +333,7 @@ get_controlfile(const char *DataDir)
 		control_file_info->minRecoveryPointTLI = ptr->minRecoveryPointTLI;
 		control_file_info->minRecoveryPoint = ptr->minRecoveryPoint;
 #else
-		fprintf(stderr, "ERROR: please use a repmgr version built for PostgreSQL 12\n");
+		fprintf(stderr, "ERROR: please use a repmgr version built for PostgreSQL 12 or later\n");
 		exit(ERR_BAD_CONFIG);
 #endif
 	}

dbutils.c

@@ -1255,7 +1255,7 @@ bool
 pg_reload_conf(PGconn *conn)
 {
 	PGresult   *res = NULL;
-	bool		success = false;
+	bool		success = true;
 
 	res = PQexec(conn, "SELECT pg_catalog.pg_reload_conf()");
 

dirutil.c

@@ -109,9 +109,56 @@ create_dir(const char *path)
 
 
 bool
-set_dir_permissions(const char *path)
+set_dir_permissions(const char *path, int server_version_num)
 {
-	return (chmod(path, 0700) != 0) ? false : true;
+	struct stat stat_buf;
+	bool no_group_access =
+		(server_version_num != UNKNOWN_SERVER_VERSION_NUM) &&
+		(server_version_num < 110000);
+	/*
+	 * At this point the path should exist, so this check is very
+	 * much just-in-case.
+	 */
+	if (stat(path, &stat_buf) != 0)
+	{
+		if (errno == ENOENT)
+		{
+			log_warning(_("directory \"%s\" does not exist"), path);
+		}
+		else
+		{
+			log_warning(_("could not read permissions of directory \"%s\""),
+						path);
+			log_detail("%s", strerror(errno));
+		}
+
+		return false;
+	}
+
+	/*
+	 * If mode is not 0700 or 0750, attempt to change.
+	 */
+	if ((no_group_access == true  && (stat_buf.st_mode & (S_IRWXG | S_IRWXO)))
+	 || (no_group_access == false && (stat_buf.st_mode & (S_IWGRP | S_IRWXO))))
+	{
+		/*
+		 * Currently we default to 0700.
+		 * There is no facility to override this directly,
+		 * but the user can manually create the directory with
+		 * the desired permissions.
+		 */
+
+		if (chmod(path, 0700) != 0) {
+			log_error(_("unable to change permissions of directory \"%s\""), path);
+			log_detail("%s", strerror(errno));
+			return false;
+		}
+
+		return true;
+	}
+
+	/* Leave as-is */
+	return true;
 }
 
 
@@ -303,7 +350,7 @@ create_pg_dir(const char *path, bool force)
 	switch (check_dir(path))
 	{
 		case DIR_NOENT:
-			/* directory does not exist, attempt to create it */
+			/* Directory does not exist, attempt to create it. */
 			log_info(_("creating directory \"%s\"..."), path);
 
 			if (!create_dir(path))
@@ -314,14 +361,23 @@ create_pg_dir(const char *path, bool force)
 			}
 			break;
 		case DIR_EMPTY:
-			/* exists but empty, fix permissions and use it */
+			/*
+			 * Directory exists but empty, fix permissions and use it.
+			 *
+			 * Note that at this point the caller might not know the server
+			 * version number, so in this case "set_dir_permissions()" will
+			 * accept 0750 as a valid setting. As this is invalid in Pg10 and
+			 * earlier,  the caller should call "set_dir_permissions()" again
+			 * when it has the number.
+			 *
+			 * We need to do the permissions check here in any case to catch
+			 * fatal permissions early.
+			 */
 			log_info(_("checking and correcting permissions on existing directory \"%s\""),
 					 path);
 
-			if (!set_dir_permissions(path))
+			if (!set_dir_permissions(path, UNKNOWN_SERVER_VERSION_NUM))
 			{
-				log_error(_("unable to change permissions of directory \"%s\""), path);
-				log_detail("%s", strerror(errno));
 				return false;
 			}
 			break;

dirutil.h

@@ -35,7 +35,7 @@ typedef enum
 } PgDirState;
 
 extern int	mkdir_p(char *path, mode_t omode);
-extern bool set_dir_permissions(const char *path);
+extern bool set_dir_permissions(const char *path, int server_version_num);
 
 extern DataDirState	check_dir(const char *path);
 extern bool create_dir(const char *path);

doc/appendix-release-notes.xml

@@ -17,8 +17,84 @@
 
 <!-- remember to update the release date in ../repmgr_version.h.in -->
 
+  <sect1 id="release-5.2.1">
+    <title id="release-current">Release 5.2.1</title>
+    <para><emphasis>Mon 7 December, 2020</emphasis></para>
+
+    <para>
+      &repmgr; 5.2.1 is a minor release.
+    </para>
+    <sect2>
+      <title>Improvements</title>
+      <para>
+        <itemizedlist>
+          <listitem>
+            <para>
+              <link linkend="repmgr-standby-clone">repmgr standby clone</link>:
+              option <option>--recovery-min-apply-delay</option> added, overriding any
+              setting present in <filename>repmgr.conf</filename>.
+            </para>
+          </listitem>
+        </itemizedlist>
+      </para>
+    </sect2>
+
+    <sect2>
+      <title>Bug fixes</title>
+      <para>
+
+         <itemizedlist>
+          <listitem>
+            <para>
+              Configuration: fix parsing of <option>replication_type</option> configuration parameter.  GitHub #672.
+            </para>
+          </listitem>
+
+          <listitem>
+            <para>
+              <link linkend="repmgr-standby-clone">repmgr standby clone</link>:
+              handle case where <filename>postgresql.auto.conf</filename> is absent on the
+              source node.
+            </para>
+          </listitem>
+
+          <listitem>
+            <para>
+              <link linkend="repmgr-standby-clone">repmgr standby clone</link>:
+              in PostgreSQL 11 and later, an existing data directory's permissions
+              will not be changed to <option>0700</option> if they are already set to
+              <option>0750</option>.
+            </para>
+          </listitem>
+
+          <listitem>
+            <para>
+              &repmgrd;: prevent termination when local node not available and
+              <option>standby_disconnect_on_failover</option> is set.  GitHub #675.
+            </para>
+          </listitem>
+          <listitem>
+            <para>
+              &repmgrd;: ensure <option>reconnect_interval</option> is correctly handled.
+              GitHub #673.
+            </para>
+          </listitem>
+
+          <listitem>
+            <para>
+              <command>repmgr witness --help</command>: fix <command>witness unregister</command>
+              description.  GitHub #676.
+            </para>
+          </listitem>
+
+       </itemizedlist>
+      </para>
+    </sect2>
+  </sect1>
+
+
   <sect1 id="release-5.2.0">
-    <title id="release-current">Release 5.2.0</title>
+    <title>Release 5.2.0</title>
     <para><emphasis>Thu 22 October, 2020</emphasis></para>
 
     <para>

doc/repmgr-standby-clone.xml

@@ -260,16 +260,29 @@ pg_basebackup_options='--waldir=/path/to/wal-directory'</programlisting>
      upstream node if required.
    </para>
    <para>
-     Note that the upstream node must be running. In PostgreSQL 11 and earlier, an existing
-     <filename>recovery.conf</filename> will not be overwritten unless the
-     <option>-F/--force</option> option is provided.
+     The upstream node must be running so the correct replication configuration can be obtained.
    </para>
    <para>
-     Execute <command>repmgr standby clone --replication-conf-only --dry-run</command>
-     to check the prerequisites for creating the recovery configuration,
-     and display the contents of the configuration which would be added without actually
-	 making any changes.
+     If the standby is running, the replication configuration will not be written unless the
+     <option>-F/--force</option> option is provided.
    </para>
+   <tip>
+     <para>
+       Execute <command>repmgr standby clone --replication-conf-only --dry-run</command>
+       to check the prerequisites for creating the recovery configuration,
+       and display the configuration changes which would be made without actually
+	   making any changes.
+     </para>
+   </tip>
+   <para>
+     In PostgreSQL 13 and later, the PostgreSQL configuration must be reloaded for replication
+     configuration changes to take effect.
+   </para>
+   <para>
+     In PostgreSQL 12 and earlier, the PostgreSQL instance must be restarted for replication
+     configuration changes to take effect.
+   </para>
+
 
   </refsect1>
 
@@ -331,6 +344,25 @@ pg_basebackup_options='--waldir=/path/to/wal-directory'</programlisting>
         </listitem>
       </varlistentry>
 
+
+      <varlistentry>
+        <term><option>--recovery-min-apply-delay</option></term>
+        <listitem>
+          <para>
+            Set PostgreSQL configuration <option>recovery_min_apply_delay</option> parameter
+            to the provided value.
+          </para>
+          <para>
+            This overrides any <option>recovery_min_apply_delay</option> provided via
+            <filename>repmgr.conf</filename>.
+          </para>
+          <para>
+            For more details on this parameter, see:
+            <ulink url="https://www.postgresql.org/docs/current/runtime-config-replication.html#GUC-RECOVERY-MIN-APPLY-DELAY">recovery_min_apply_delay</ulink>.
+          </para>
+        </listitem>
+      </varlistentry>
+
       <varlistentry>
         <term><option>-R, --remote-user=USERNAME</option></term>
         <listitem>
@@ -346,14 +378,14 @@ pg_basebackup_options='--waldir=/path/to/wal-directory'</programlisting>
           <para>
             Create recovery configuration for a previously cloned instance.
           </para>
-		  <para>
-			In PostgreSQL 11 and earlier, the replication configuration will be
-			written to <filename>recovery.conf</filename>.
-		  </para>
 		  <para>
 			In PostgreSQL 12 and later, the replication configuration will be
 			written to <filename>postgresql.auto.conf</filename>.
 		  </para>
+		  <para>
+			In PostgreSQL 11 and earlier, the replication configuration will be
+			written to <filename>recovery.conf</filename>.
+		  </para>
         </listitem>
       </varlistentry>
 

repmgr-action-standby.c

@@ -733,7 +733,7 @@ do_standby_clone(void)
 			drop_replication_slot_if_exists(source_conn, UNKNOWN_NODE_ID, local_node_record.slot_name);
 		}
 
-		log_error(_("unable to take a base backup of the primary server"));
+		log_error(_("unable to take a base backup of the source server"));
 		log_hint(_("data directory (\"%s\") may need to be cleaned up manually"),
 				 local_data_directory);
 
@@ -763,7 +763,7 @@ do_standby_clone(void)
 			exit(ERR_BAD_CONFIG);
 		}
 
-		appendPQExpBufferStr(&command, " ");
+		appendPQExpBufferChar(&command, ' ');
 
 		/* Somewhat inconsistent, but pg_verifybackup doesn't accept a -D option  */
 		appendShellString(&command,
@@ -838,6 +838,16 @@ do_standby_clone(void)
 			break;
 	}
 
+	/*
+	 * Do a final check on the data directory permissions - if the user
+	 * is cloning into an existing directory set to 0750, and the server
+	 * is Pg10 or earlier, Pg will refuse to start. We might not have
+	 * known the server version when creating the data directory
+	 * (mainly if cloning from Barman with no upstream connection), hence
+	 * the additional check here.
+	 */
+	set_dir_permissions(local_data_directory, source_server_version_num);
+
 	/*
 	 * TODO: It might be nice to provide an option to have repmgr start the
 	 * PostgreSQL server automatically
@@ -1444,7 +1454,18 @@ _do_create_replication_conf(void)
 
 			if (node_is_running == true)
 			{
-				log_hint(_("node must be restarted for the new file to take effect"));
+				if (PQserverVersion(upstream_conn) >= 130000)
+				{
+					log_hint(_("configuration must be reloaded for the configuration changes to take effect"));
+				}
+				else if (PQserverVersion(upstream_conn) >= 120000)
+				{
+					log_hint(_("node must be restarted for the configuration changes to take effect"));
+				}
+				else
+				{
+					log_hint(_("node must be restarted for the new file to take effect"));
+				}
 			}
 		}
 	}
@@ -7762,7 +7783,9 @@ static void
 tablespace_data_append(TablespaceDataList *list, const char *name, const char *oid, const char *location)
 {
 	TablespaceDataListCell *cell = NULL;
-
+	int oid_len = strlen(oid);
+	int name_len = strlen(name);
+	int location_len = strlen(location);
 	cell = (TablespaceDataListCell *) pg_malloc0(sizeof(TablespaceDataListCell));
 
 	if (cell == NULL)
@@ -7771,13 +7794,13 @@ tablespace_data_append(TablespaceDataList *list, const char *name, const char *o
 		exit(ERR_OUT_OF_MEMORY);
 	}
 
-	cell->oid = pg_malloc(1 + strlen(oid));
-	cell->name = pg_malloc(1 + strlen(name));
-	cell->location = pg_malloc(1 + strlen(location));
+	cell->oid = pg_malloc0(1 + oid_len);
+	cell->name = pg_malloc0(1 + name_len);
+	cell->location = pg_malloc0(1 + location_len);
 
-	strncpy(cell->oid, oid, 1 + strlen(oid));
-	strncpy(cell->name, name, 1 + strlen(name));
-	strncpy(cell->location, location, 1 + strlen(location));
+	strncpy(cell->oid, oid, oid_len);
+	strncpy(cell->name, name, name_len);
+	strncpy(cell->location, location, location_len);
 
 	if (list->tail)
 		list->tail->next = cell;
@@ -8911,6 +8934,8 @@ do_standby_help(void)
 #endif
 	printf(_("  --without-barman                    do not clone from Barman even if configured\n"));
 	printf(_("  --replication-conf-only             generate replication configuration for a previously cloned instance\n"));
+	printf(_("  --recovery-min-apply-delay          set PostgreSQL configuration parameter \"recovery_min_apply_delay\"\n" \
+			 "                                      (overrides any setting in repmgr.conf)\n"));
 
 	puts("");
 

repmgr-action-witness.c

@@ -560,7 +560,7 @@ void do_witness_help(void)
 
 	printf(_("WITNESS UNREGISTER\n"));
 	puts("");
-	printf(_("  \"witness register\" unregisters a witness node.\n"));
+	printf(_("  \"witness unregister\" unregisters a witness node.\n"));
 	puts("");
 	printf(_("  --dry-run                check prerequisites but don't make any changes\n"));
 	printf(_("  -F, --force              unregister when witness node not running\n"));

repmgr-client-global.h

@@ -84,7 +84,7 @@ typedef struct
 	bool		fast_checkpoint;
 	bool		rsync_only;
 	bool		no_upstream_connection;
-	char		recovery_min_apply_delay[MAXLEN];
+	char		recovery_min_apply_delay[MAXLEN];	/* overrides setting in repmgr.conf */
 	char		replication_user[MAXLEN];
 	char		upstream_conninfo[MAXLEN];
 	bool		without_barman;

repmgr-client.c

@@ -438,6 +438,11 @@ main(int argc, char **argv)
 				runtime_options.replication_conf_only = true;
 				break;
 
+				/* --recovery-min-apply-delay */
+			case OPT_RECOVERY_MIN_APPLY_DELAY:
+				strncpy(runtime_options.recovery_min_apply_delay, optarg, sizeof(runtime_options.recovery_min_apply_delay));
+				break;
+
 				/* --verify-backup */
 			case OPT_VERIFY_BACKUP:
 				runtime_options.verify_backup = true;
@@ -1105,10 +1110,25 @@ main(int argc, char **argv)
 		exit(SUCCESS);
 	}
 
-
-
 	check_cli_parameters(action);
 
+
+	/*
+	 * Command-line parameter --recovery-min-apply-delay overrides the equivalent
+	 * setting in the config file. Note we'll need to parse it here to handle
+	 * any formatting errors.
+	 */
+
+	if (*runtime_options.recovery_min_apply_delay != '\0')
+	{
+		parse_time_unit_parameter("--recovery-min-apply-delay",
+								  runtime_options.recovery_min_apply_delay,
+								  config_file_options.recovery_min_apply_delay,
+								  &cli_errors);
+
+		config_file_options.recovery_min_apply_delay_provided = true;
+	}
+
 	/*
 	 * Sanity checks for command line parameters completed by now; any further
 	 * errors will be runtime ones
@@ -1169,7 +1189,7 @@ main(int argc, char **argv)
 	}
 
 	/*
-	 * Check for configuration file items which can be overriden by runtime
+	 * Check for configuration file items which can be overridden by runtime
 	 * options
 	 * =====================================================================
 	 */

repmgr-client.h

@@ -99,6 +99,7 @@
 #define OPT_REPLICATION_CONFIG_OWNER	   1046
 #define OPT_DB_CONNECTION				   1047
 #define OPT_VERIFY_BACKUP				   1048
+#define OPT_RECOVERY_MIN_APPLY_DELAY       1049
 
 /* These options are for internal use only */
 #define OPT_CONFIG_ARCHIVE_DIR			   2001
@@ -164,6 +165,7 @@ static struct option long_options[] =
 	{"without-barman", no_argument, NULL, OPT_WITHOUT_BARMAN},
 	{"replication-conf-only", no_argument, NULL, OPT_REPLICATION_CONF_ONLY},
 	{"verify-backup", no_argument, NULL, OPT_VERIFY_BACKUP },
+	{"recovery-min-apply-delay", required_argument, NULL, OPT_RECOVERY_MIN_APPLY_DELAY },
 	/* deprecate this once Pg11 and earlier are unsupported */
 	{"recovery-conf-only", no_argument, NULL, OPT_REPLICATION_CONF_ONLY},
 

repmgr.h

@@ -77,8 +77,6 @@
 #define MIN_SUPPORTED_VERSION		"9.4"
 #define MIN_SUPPORTED_VERSION_NUM	90400
 
-#define REPLICATION_TYPE_PHYSICAL 1
-
 #define UNKNOWN_SERVER_VERSION_NUM -1
 #define UNKNOWN_REPMGR_VERSION_NUM -1
 
@@ -122,6 +120,7 @@
 #define DEFAULT_NODE_REJOIN_TIMEOUT          60  /* seconds */
 #define DEFAULT_ARCHIVE_READY_WARNING        16  /* WAL files */
 #define DEFAULT_ARCHIVE_READY_CRITICAL       128 /* WAL files */
+#define	DEFAULT_REPLICATION_TYPE             REPLICATION_TYPE_PHYSICAL
 #define	DEFAULT_REPLICATION_LAG_WARNING      300 /* seconds */
 #define DEFAULT_REPLICATION_LAG_CRITICAL     600 /* seconds */
 #define DEFAULT_WITNESS_SYNC_INTERVAL        15  /* seconds */

repmgr_version.h.in

@@ -1,5 +1,5 @@
 #define REPMGR_VERSION_DATE ""
-#define REPMGR_VERSION "5.2.0"
-#define REPMGR_VERSION_NUM 50200
-#define REPMGR_RELEASE_DATE "2020-10-22"
+#define REPMGR_VERSION "5.2.1"
+#define REPMGR_VERSION_NUM 50201
+#define REPMGR_RELEASE_DATE "2020-12-07"
 #define PG_ACTUAL_VERSION_NUM 

repmgrd-physical.c

@@ -3921,6 +3921,7 @@ follow_new_primary(int new_primary_id)
 
 				termPQExpBuffer(&event_details);
 
+				close_connection(&upstream_conn);
 				close_connection(&old_primary_conn);
 
 				return FAILOVER_STATE_PRIMARY_REAPPEARED;
@@ -3931,6 +3932,8 @@ follow_new_primary(int new_primary_id)
 			close_connection(&old_primary_conn);
 		}
 
+		close_connection(&upstream_conn);
+
 		return FAILOVER_STATE_FOLLOW_FAIL;
 	}
 
@@ -5381,7 +5384,7 @@ try_primary_reconnect(PGconn **conn, PGconn *local_conn, t_node_info *node_info)
 		time_t started_at = time(NULL);
 		int up_to;
 		bool sleep_now = false;
-		bool max_sleep_seconds;
+		int max_sleep_seconds;
 
 		log_info(_("checking state of node \"%s\" (ID: %i), %i of %i attempts"),
 				 node_info->node_name,

sysutils.c

@@ -289,10 +289,19 @@ disable_wal_receiver(PGconn *conn)
 
 	if (wal_retrieve_retry_interval < WALRECEIVER_DISABLE_TIMEOUT_VALUE)
 	{
+		bool success;
+
 		log_notice(_("setting \"wal_retrieve_retry_interval\" to %i milliseconds"),
 				   new_wal_retrieve_retry_interval);
 		alter_system_int(conn, "wal_retrieve_retry_interval", new_wal_retrieve_retry_interval);
-		pg_reload_conf(conn);
+
+		success = pg_reload_conf(conn);
+
+		if (success == false)
+		{
+			log_warning(_("unable to reload configuration"));
+			return UNKNOWN_PID;
+		}
 	}
 
 	/*
@@ -354,6 +363,12 @@ enable_wal_receiver(PGconn *conn, bool wait_startup)
 	/* make timeout configurable */
 	int i, timeout = 30;
 
+	if (PQstatus(conn) != CONNECTION_OK)
+	{
+		log_error(_("database connection not available"));
+		return UNKNOWN_PID;
+	}
+
 	if (is_superuser_connection(conn, NULL) == false)
 	{
 		log_error(_("superuser connection required"));
@@ -394,7 +409,13 @@ enable_wal_receiver(PGconn *conn, bool wait_startup)
 			return UNKNOWN_PID;
 		}
 
-		pg_reload_conf(conn);
+		success = pg_reload_conf(conn);
+
+		if (success == false)
+		{
+			log_warning(_("unable to reload configuration"));
+			return UNKNOWN_PID;
+		}
 	}
 	else
 	{