repmgrd: fix configuration file reloading

Don't allow "promote_command" or "follow_command" to be empty.

GitHub #486.

configfile.c

@@ -28,6 +28,7 @@ char		config_file_path[MAXPGPATH] = "";
 static bool config_file_provided = false;
 bool		config_file_found = false;
 
+static void parse_config(t_configuration_options *options, bool terse);
 static void _parse_config(t_configuration_options *options, ItemList *error_list, ItemList *warning_list);
 
 static void _parse_line(char *buf, char *name, char *value);
@@ -238,7 +239,7 @@ end_search:
 }
 
 
-void
+static void
 parse_config(t_configuration_options *options, bool terse)
 {
 	/* Collate configuration file errors here for friendlier reporting */
@@ -785,7 +786,6 @@ _parse_config(t_configuration_options *options, ItemList *error_list, ItemList *
 		PQconninfoFree(conninfo_options);
 	}
 
-
 	/* set values for parameters which default to other parameters */
 
 	/*
@@ -1065,7 +1065,7 @@ parse_time_unit_parameter(const char *name, const char *value, char *dest, ItemL
 
  */
 bool
-reload_config(t_configuration_options *orig_options)
+reload_config(t_configuration_options *orig_options, t_server_type server_type)
 {
 	PGconn	   *conn;
 	t_configuration_options new_options = T_CONFIGURATION_OPTIONS_INITIALIZER;
@@ -1081,6 +1081,20 @@ reload_config(t_configuration_options *orig_options)
 
 	_parse_config(&new_options, &config_errors, &config_warnings);
 
+
+	if (server_type == PRIMARY || server_type == STANDBY)
+	{
+		if (new_options.promote_command[0] == '\0')
+		{
+			item_list_append(&config_errors, _("\"promote_command\": required parameter was not found"));
+		}
+
+		if (new_options.follow_command[0] == '\0')
+		{
+			item_list_append(&config_errors, _("\"follow_command\": required parameter was not found"));
+		}
+	}
+
 	if (config_errors.head != NULL)
 	{
 		ItemListCell *cell = NULL;

configfile.h

@@ -273,13 +273,13 @@ typedef struct
 	"", "", "", "" \
 }
 
+#include "dbutils.h"
 
 void		set_progname(const char *argv0);
 const char *progname(void);
 
 void		load_config(const char *config_file, bool verbose, bool terse, t_configuration_options *options, char *argv0);
-void		parse_config(t_configuration_options *options, bool terse);
+bool		reload_config(t_configuration_options *orig_options, t_server_type server_type);
-bool		reload_config(t_configuration_options *orig_options);
 
 bool		parse_recovery_conf(const char *data_dir, t_recovery_conf *conf);
 

dbutils.c

@@ -3941,7 +3941,7 @@ is_server_available(const char *conninfo)
 {
 	PGPing		status = PQping(conninfo);
 
-	log_verbose(LOG_DEBUG, "ping status for %s is %i", conninfo, (int)status);
+	log_verbose(LOG_DEBUG, "is_server_available(): ping status for %s is %i", conninfo, (int)status);
 	if (status == PQPING_OK)
 		return true;
 
@@ -3960,7 +3960,7 @@ is_server_available_params(t_conninfo_param_list *param_list)
 	if (log_level == LOG_DEBUG)
 	{
 		char *conninfo_str = param_list_to_string(param_list);
-		log_verbose(LOG_DEBUG, "ping status for %s is %i", conninfo_str, (int)status);
+		log_verbose(LOG_DEBUG, "is_server_available_params(): ping status for %s is %i", conninfo_str, (int)status);
 		pfree(conninfo_str);
 	}
 

repmgrd-bdr.c

@@ -293,7 +293,7 @@ loop:
 			/*
 			 * if we can reload, then could need to change local_conn
 			 */
-			if (reload_config(&config_file_options))
+			if (reload_config(&config_file_options, BDR))
 			{
 				PQfinish(local_conn);
 				local_conn = establish_db_connection(config_file_options.conninfo, true);
@@ -303,11 +303,12 @@ loop:
 			got_SIGHUP = false;
 		}
 
+		/* XXX this looks like it will never be called */
 		if (got_SIGHUP)
 		{
 			log_debug("SIGHUP received");
 
-			if (reload_config(&config_file_options))
+			if (reload_config(&config_file_options, BDR))
 			{
 				PQfinish(local_conn);
 				local_conn = establish_db_connection(config_file_options.conninfo, true);

repmgrd-physical.c

@@ -81,7 +81,7 @@ static bool do_witness_failover(void);
 
 static void update_monitoring_history(void);
 
-static void handle_sighup(PGconn *conn);
+static void handle_sighup(PGconn **conn, t_server_type server_type);
 
 static const char * format_failover_state(FailoverState failover_state);
 
@@ -547,7 +547,7 @@ loop:
 
 		if (got_SIGHUP)
 		{
-			handle_sighup(local_conn);
+			handle_sighup(&local_conn, PRIMARY);
 		}
 
 		log_verbose(LOG_DEBUG, "sleeping %i seconds (parameter \"monitor_interval_secs\")",
@@ -1148,7 +1148,7 @@ loop:
 
 		if (got_SIGHUP)
 		{
-			handle_sighup(local_conn);
+			handle_sighup(&local_conn, STANDBY);
 		}
 
 		log_verbose(LOG_DEBUG, "sleeping %i seconds (parameter \"monitor_interval_secs\")",
@@ -1477,7 +1477,7 @@ loop:
 
 		if (got_SIGHUP)
 		{
-			handle_sighup(local_conn);
+			handle_sighup(&local_conn, WITNESS);
 		}
 
 		log_verbose(LOG_DEBUG, "sleeping %i seconds (parameter \"monitor_interval_secs\")",
@@ -2928,14 +2928,14 @@ format_failover_state(FailoverState failover_state)
 
 
 static void
-handle_sighup(PGconn *conn)
+handle_sighup(PGconn **conn, t_server_type server_type)
 {
 	log_debug("SIGHUP received");
 
-	if (reload_config(&config_file_options))
+	if (reload_config(&config_file_options, server_type))
 	{
-		close_connection(&conn);
+		PQfinish(*conn);
-		conn = establish_db_connection(config_file_options.conninfo, true);
+		*conn = establish_db_connection(config_file_options.conninfo, true);
 	}
 
 	if (*config_file_options.log_file)