From 3d9d0d98af1b869ac31805c95ce12c1ec42a2504 Mon Sep 17 00:00:00 2001
From: Ian Barwick <ian@2ndquadrant.com>
Date: Wed, 4 Feb 2015 09:57:51 +0900
Subject: [PATCH] Improve repmgr configuration file handling

Previous behaviour was somewhat counterintuitive, with an error
message being logged if no configuration file provided or found,
even though this is not actually an error.

Configuration files now handled like this:

- if a configuration file is explicitly provided (-f), error out
  if not found.

- if no configuration file explicitly provided, attempt to open
  default configuration file; if this does not exist, log
  notice and continue with default values.

Also, for 9.4 and later add a hint about replication slot usage
if 'use_replication_slots' not set.
---
 config.c |  5 +++--
 repmgr.c | 62 ++++++++++++++++++++++++++++++++++++++------------------
 2 files changed, 45 insertions(+), 22 deletions(-)

diff --git a/config.c b/config.c
index 8ec50e17..32249aff 100644
--- a/config.c
+++ b/config.c
@@ -65,8 +65,9 @@ parse_config(const char *config_file, t_configuration_options * options)
 	 */
 	if (fp == NULL)
 	{
-		log_err(_("Did not find the configuration file '%s', continuing\n"),
-				config_file);
+		log_notice(_("No configuration file provided and default file '%s' not found - "
+					 "continuing with default values\n"),
+				   config_file);
 		return;
 	}
 
diff --git a/repmgr.c b/repmgr.c
index 343c38ee..06ac4394 100644
--- a/repmgr.c
+++ b/repmgr.c
@@ -30,6 +30,7 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <sys/stat.h>			/* for stat() */
 #include <sys/wait.h>
 #include <time.h>
 #include <unistd.h>
@@ -340,17 +341,39 @@ main(int argc, char **argv)
 		strncpy(runtime_options.masterport, DEFAULT_MASTER_PORT, MAXLEN);
 	}
 
-	/* Read the configuration file, normally repmgr.conf */
-	if (!runtime_options.config_file[0])
+	/*
+	 * If a configuration file was provided, check it exists, otherwise
+	 * emit an error
+	 */
+	if (runtime_options.config_file[0])
+	{
+		struct stat config;
+		if(stat(runtime_options.config_file, &config) != 0)
+		{
+			log_err(_("Provided configuration file '%s' not found: %s\n"),
+					runtime_options.config_file,
+					strerror(errno)
+				);
+			exit(ERR_BAD_CONFIG);
+		}
+	}
+	/*
+	 * If no configuration file was provided, set to a default file
+	 * which `parse_config()` will attempt to read if it exists
+	 */
+	else
+	{
 		strncpy(runtime_options.config_file, DEFAULT_CONFIG_FILE, MAXLEN);
+	}
 
 	if (runtime_options.verbose)
 		printf(_("Opening configuration file: %s\n"),
 			   runtime_options.config_file);
 
 	/*
-	 * XXX Do not read config files for action where it is not required (clone
-	 * for example).
+	 * The configuration file is not required for some actions (e.g. 'standby clone'),
+	 * however if available we'll parse it anyway for options like 'log_level',
+	 * 'use_replication_slots' etc.
 	 */
 	parse_config(runtime_options.config_file, &options);
 
@@ -1984,22 +2007,12 @@ check_parameters_for_action(const int action)
 		case STANDBY_CLONE:
 
 			/*
-			 * Issue a friendly notice that the configuration file is not
-			 * necessary nor read at all in when performing a STANDBY CLONE
-			 * action.
-			 */
-			if (runtime_options.config_file[0])
-			{
-				log_notice(_("Only command line parameters for the connection "
-							 "to the master are used when issuing a STANDBY CLONE command. "
-							 "The passed configuration file is neither required nor used for "
-							 "its node configuration portions\n\n"));
-			}
-
-			/*
-			 * To clone a master into a standby we need connection parameters
-			 * repmgr.conf is useless because we don't have a server running
-			 * in the standby; warn the user, but keep going.
+			 * Previous repmgr versions issued a notice here if a configuration
+			 * file was provided saying it wasn't needed, which was confusing as
+			 * it was needed for the `pg_bindir` parameter.
+			 *
+			 * In any case it's sensible to read the configuration file if available
+			 * for `pg_bindir`, `loglevel` and `use_replication_slots`.
 			 */
 			if (runtime_options.host == NULL)
 			{
@@ -2477,8 +2490,17 @@ check_upstream_config(PGconn *conn, int server_version_num, bool exit_on_error)
 		if (i == 0 || i == -1)
 		{
 			if (i == 0)
+			{
 				log_err(_("%s needs parameter 'wal_keep_segments' to be set to %s or greater (see the '-w' option or edit the postgresql.conf of the upstream server.)\n"),
 						progname, runtime_options.wal_keep_segments);
+				if(server_version_num >= 90400)
+				{
+					log_notice(_("HINT: in PostgreSQL 9.4 and later, replication slots can be used, which "
+							   "do not require 'wal_keep_segments' to be set to a high value "
+							   "(set parameter 'use_replication_slots' in the configuration file to enable)\n"
+								 ));
+				}
+			}
 
 			if(exit_on_error == true)
 			{