Update code comments

From 3.1.4 `repmgr` will behave like other PostgreSQL utilities
when handling database connection parameters, in particular
accepting a conninfo string and honouring libpq connection defaults.

README.md

@@ -215,6 +215,34 @@ command line options:
 - `-b/--pg_bindir`
 
 
+### Command line options and environment variables
+
+For some commands, e.g. `repmgr standby clone`, database connection parameters
+need to be provided. Like other PostgreSQL utilities, following standard
+parameters can be used:
+
+- `-d/--dbname=DBNAME`
+- `-h/--host=HOSTNAME`
+- `-p/--port=PORT`
+- `-U/--username=USERNAME`
+
+If `-d/--dbname` contains an `=` sign or starts with a valid URI prefix (`postgresql://`
+or `postgres://`), it is treated as a conninfo string. See the PostgreSQL
+documentation for further details:
+
+  https://www.postgresql.org/docs/current/static/libpq-connect.html#LIBPQ-CONNSTRING
+
+Note that if a `conninfo` string is provided, values set in this will override any
+provided as individual parameters. For example, with `-d 'host=foo' --host bar`, `foo`
+will be chosen over `bar`.
+
+Like other PostgreSQL utilities, `repmgr` will default to any values set in environment
+variables if explicit command line parameters are not provided. See the PostgreSQL
+documentation for further details:
+
+  https://www.postgresql.org/docs/current/static/libpq-envars.html
+
+
 Setting up a simple replication cluster with repmgr
 ---------------------------------------------------
 
@@ -470,7 +498,11 @@ so should be used with care.
 Further options can be passed to the `pg_basebackup` utility via
 the setting `pg_basebackup_options` in `repmgr.conf`. See the PostgreSQL
 documentation for more details of available options:
+<<<<<<< HEAD
+  http://www.postgresql.org/docs/current/static/app-pgbasebackup.html
+=======
   https://www.postgresql.org/docs/current/static/app-pgbasebackup.html
+>>>>>>> 72f9b0145afab1060dd1202c8f8937653c8b2e39
 
 ### Using rsync to clone a standby
 
@@ -488,7 +520,6 @@ and destination server as the contents of files existing on both servers need
 to be compared, meaning this method is not necessarily faster than making a
 fresh clone with `pg_basebackup`.
 
-
 ### Dealing with PostgreSQL configuration files
 
 By default, `repmgr` will attempt to copy the standard configuration files
@@ -503,6 +534,21 @@ which enables any valid `rsync` options to be passed to that command, e.g.:
 
     rsync_options='--exclude=postgresql.local.conf'
 
+### Controlling `primary_conninfo` in `recovery.conf`
+
+`repmgr` will create the `primary_conninfo` setting in `recovery.conf` based
+on the connection parameters provided to `repmgr standby clone` and PostgreSQL's
+standard connection defaults, including any environment variables set on the
+local node.
+
+To include specific connection parameters other than the standard host, port,
+username and database values (e.g. `sslmode`), include these in a `conninfo`-style
+tring 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`.
+
 
 Setting up cascading replication with repmgr
 --------------------------------------------
@@ -1359,20 +1405,22 @@ which contains connection details for the local database.
 `repmgr` or `repmgrd` will return one of the following error codes on program
 exit:
 
-* SUCCESS (0)              Program ran successfully.
+* SUCCESS (0)               Program ran successfully.
-* ERR_BAD_CONFIG (1)       Configuration file could not be parsed or was invalid
+* ERR_BAD_CONFIG (1)        Configuration file could not be parsed or was invalid
-* ERR_BAD_RSYNC (2)        An rsync call made by the program returned an error
+* ERR_BAD_RSYNC (2)         An rsync call made by the program returned an error (repmgr only)
-* ERR_NO_RESTART (4)       An attempt to restart a PostgreSQL instance failed
+* ERR_NO_RESTART (4)        An attempt to restart a PostgreSQL instance failed
-* ERR_DB_CON (6)           Error when trying to connect to a database
+* ERR_DB_CON (6)            Error when trying to connect to a database
-* ERR_DB_QUERY (7)         Error while executing a database query
+* ERR_DB_QUERY (7)          Error while executing a database query
-* ERR_PROMOTED (8)         Exiting program because the node has been promoted to master
+* ERR_PROMOTED (8)          Exiting program because the node has been promoted to master
-* ERR_BAD_PASSWORD (9)     Password used to connect to a database was rejected
+* ERR_STR_OVERFLOW (10)     String overflow error
-* ERR_STR_OVERFLOW (10)    String overflow error
+* ERR_FAILOVER_FAIL (11)    Error encountered during failover (repmgrd only)
-* ERR_FAILOVER_FAIL (11)   Error encountered during failover (repmgrd only)
+* ERR_BAD_SSH (12)          Error when connecting to remote host via SSH (repmgr only)
-* ERR_BAD_SSH (12)         Error when connecting to remote host via SSH
+* ERR_SYS_FAILURE (13)      Error when forking (repmgrd only)
-* ERR_SYS_FAILURE (13)     Error when forking (repmgrd only)
+* ERR_BAD_BASEBACKUP (14)   Error when executing pg_basebackup (repmgr only)
-* ERR_BAD_BASEBACKUP (14)  Error when executing pg_basebackup
+* ERR_MONITORING_FAIL (16)  Unrecoverable error encountered during monitoring (repmgrd only)
-* ERR_MONITORING_FAIL (16) Unrecoverable error encountered during monitoring (repmgrd only)
+* ERR_BAD_BACKUP_LABEL (17) Corrupt or unreadable backup label encountered (repmgr only)
+* ERR_SWITCHOVER_FAIL (18)  Error encountered during switchover (repmgr only)
+
 
 Support and Assistance
 ----------------------

errcode.h

@@ -29,7 +29,6 @@
 #define ERR_DB_CON 6
 #define ERR_DB_QUERY 7
 #define ERR_PROMOTED 8
-#define ERR_BAD_PASSWORD 9
 #define ERR_STR_OVERFLOW 10
 #define ERR_FAILOVER_FAIL 11
 #define ERR_BAD_SSH 12

repmgr.c

@@ -83,14 +83,14 @@
 #define CLUSTER_CLEANUP		   12
 
 
-static bool create_recovery_file(const char *data_dir);
 static int	test_ssh_connection(char *host, char *remote_user);
 static int  copy_remote_files(char *host, char *remote_user, char *remote_path,
 							  char *local_path, bool is_directory, int server_version_num);
 static int  run_basebackup(const char *data_dir, int server_version);
 static void check_parameters_for_action(const int action);
 static bool create_schema(PGconn *conn);
-static void write_primary_conninfo(char *line);
+static bool create_recovery_file(const char *data_dir, PGconn *primary_conn);
+static void write_primary_conninfo(char *line, PGconn *primary_conn);
 static bool write_recovery_file_line(FILE *recovery_file, char *recovery_file_path, char *line);
 static void check_master_standby_version_match(PGconn *conn, PGconn *master_conn);
 static int	check_server_version(PGconn *conn, char *server_type, bool exit_on_error, char *server_version_string);
@@ -123,9 +123,15 @@ static bool copy_file(const char *old_filename, const char *new_filename);
 
 static bool read_backup_label(const char *local_data_directory, struct BackupLabel *out_backup_label);
 
+static void param_set(const char *param, const char *value);
+
 /* Global variables */
-static const char *keywords[6];
+static PQconninfoOption *opts = NULL;
-static const char *values[6];
+
+static int param_count = 0;
+static char **param_keywords;
+static char **param_values;
+
 static bool		   config_file_required = true;
 
 /* Initialization of runtime options */
@@ -134,6 +140,7 @@ t_configuration_options options = T_CONFIGURATION_OPTIONS_INITIALIZER;
 
 bool 	 	 wal_keep_segments_used = false;
 bool 	 	 connection_param_provided = false;
+bool 	 	 host_param_provided = false;
 bool 	 	 pg_rewind_supplied = false;
 
 static char *server_mode = NULL;
@@ -199,7 +206,9 @@ main(int argc, char **argv)
 	bool 		check_upstream_config = false;
 	bool 		config_file_parsed = false;
 	char 	   *ptr = NULL;
-	const char *env;
+
+	PQconninfoOption *defs = NULL;
+	PQconninfoOption *def;
 
 	set_progname(argv[0]);
 
@@ -216,43 +225,74 @@ main(int argc, char **argv)
 		exit(1);
 	}
 
-	/* Initialise some defaults */
 
-	/* set default user */
+	param_count = 0;
-	env = getenv("PGUSER");
+	defs = PQconndefaults();
-	if (!env)
+
+	/* Count maximum number of parameters */
+	for (def = defs; def->keyword; def++)
+		param_count ++;
+
+	/* Initialize our internal parameter list */
+	param_keywords = pg_malloc0(sizeof(char *) * (param_count + 1));
+	param_values = pg_malloc0(sizeof(char *) * (param_count + 1));
+
+	for (c = 0; c <= param_count; c++)
 	{
-		struct passwd *pw = NULL;
+		param_keywords[c] = NULL;
-		pw = getpwuid(geteuid());
+		param_values[c] = NULL;
-		if (pw)
+	}
+
+	/*
+	 * Pre-set any defaults, which can be overwritten if matching
+	 * command line parameters are provided
+	 */
+
+	for (def = defs; def->keyword; def++)
+	{
+		if (def->val != NULL && def->val[0] != '\0')
 		{
-			env = pw->pw_name;
+			param_set(def->keyword, def->val);
 		}
-		else
+
+		if (strcmp(def->keyword, "host") == 0 &&
+			(def->val != NULL && def->val[0] != '\0'))
 		{
-			fprintf(stderr, _("could not get current user name: %s\n"), strerror(errno));
+			strncpy(runtime_options.host, def->val, MAXLEN);
-			exit(ERR_BAD_CONFIG);
+		}
+		else if (strcmp(def->keyword, "hostaddr") == 0 &&
+			(def->val != NULL && def->val[0] != '\0'))
+		{
+			strncpy(runtime_options.host, def->val, MAXLEN);
+		}
+		else if (strcmp(def->keyword, "port") == 0 &&
+				 (def->val != NULL && def->val[0] != '\0'))
+		{
+			strncpy(runtime_options.masterport, def->val, MAXLEN);
+		}
+		else if (strcmp(def->keyword, "dbname") == 0 &&
+				 (def->val != NULL && def->val[0] != '\0'))
+		{
+			strncpy(runtime_options.dbname, def->val, MAXLEN);
+		}
+		else if (strcmp(def->keyword, "user") == 0 &&
+				 (def->val != NULL && def->val[0] != '\0'))
+		{
+			strncpy(runtime_options.username, def->val, MAXLEN);
 		}
 	}
-	strncpy(runtime_options.username, env, MAXLEN);
 
-	/* set default database */
+	PQconninfoFree(defs);
-	env = getenv("PGDATABASE");
+
-	if (!env)
+
+	/*
+	 * Though libpq will default to the username as dbname, PQconndefaults()
+	 * doesn't return this
+	 */
+	if (runtime_options.dbname[0] == '\0')
 	{
-		env = runtime_options.username;
+		strncpy(runtime_options.dbname, runtime_options.username, MAXLEN);
 	}
-	strncpy(runtime_options.dbname, env, MAXLEN);
-
-	/* set default port */
-
-	env = getenv("PGPORT");
-	if (!env)
-	{
-		env = DEF_PGPORT_STR;
-	}
-
-	strncpy(runtime_options.masterport, env, MAXLEN);
 
 	/* Prevent getopt_long() from printing an error message */
 	opterr = 0;
@@ -276,14 +316,18 @@ main(int argc, char **argv)
 				exit(SUCCESS);
 			case 'd':
 				strncpy(runtime_options.dbname, optarg, MAXLEN);
+				/* we'll set the dbname parameter below  if we detect it's not a conninfo string */
 				connection_param_provided = true;
 				break;
 			case 'h':
 				strncpy(runtime_options.host, optarg, MAXLEN);
+				param_set("host", optarg);
 				connection_param_provided = true;
+				host_param_provided = true;
 				break;
 			case 'p':
 				repmgr_atoi(optarg, "-p/--port", &cli_errors, false);
+				param_set("port", optarg);
 				strncpy(runtime_options.masterport,
 						optarg,
 						MAXLEN);
@@ -291,6 +335,7 @@ main(int argc, char **argv)
 				break;
 			case 'U':
 				strncpy(runtime_options.username, optarg, MAXLEN);
+				param_set("user", optarg);
 				connection_param_provided = true;
 				break;
 			case 'S':
@@ -443,6 +488,77 @@ main(int argc, char **argv)
 		}
 	}
 
+	/*
+	 * If -d/--dbname appears to be a conninfo string, validate by attempting
+	 * to parse it (and if successful, store the parsed parameters)
+	 */
+	if (runtime_options.dbname)
+	{
+		if(strncmp(runtime_options.dbname, "postgresql://", 13) == 0 ||
+		   strncmp(runtime_options.dbname, "postgres://", 11) == 0 ||
+		   strchr(runtime_options.dbname, '=') != NULL)
+		{
+			char	   *errmsg = NULL;
+
+			opts = PQconninfoParse(runtime_options.dbname, &errmsg);
+
+			if (opts == NULL)
+			{
+				PQExpBufferData conninfo_error;
+				initPQExpBuffer(&conninfo_error);
+				appendPQExpBuffer(&conninfo_error, _("error parsing conninfo:\n%s"), errmsg);
+				error_list_append(&cli_errors, conninfo_error.data);
+
+				termPQExpBuffer(&conninfo_error);
+				free(errmsg);
+			}
+			else
+			{
+				/*
+				 * Store any parameters provided in the conninfo string in our
+				 * internal array; also overwrite any options set in
+				 * runtime_options.(host|port|username), as the conninfo
+				 * settings take priority
+				 */
+				PQconninfoOption *opt;
+				for (opt = opts; opt->keyword != NULL; opt++)
+				{
+					if (opt->val != NULL && opt->val[0] != '\0')
+					{
+						param_set(opt->keyword, opt->val);
+					}
+
+					if (strcmp(opt->keyword, "host") == 0 &&
+						(opt->val != NULL && opt->val[0] != '\0'))
+					{
+						strncpy(runtime_options.host, opt->val, MAXLEN);
+						host_param_provided = true;
+					}
+					if (strcmp(opt->keyword, "hostaddr") == 0 &&
+						(opt->val != NULL && opt->val[0] != '\0'))
+					{
+						strncpy(runtime_options.host, opt->val, MAXLEN);
+						host_param_provided = true;
+					}
+					else if (strcmp(opt->keyword, "port") == 0 &&
+							 (opt->val != NULL && opt->val[0] != '\0'))
+					{
+						strncpy(runtime_options.masterport, opt->val, MAXLEN);
+					}
+					else if (strcmp(opt->keyword, "user") == 0 &&
+							 (opt->val != NULL && opt->val[0] != '\0'))
+					{
+						strncpy(runtime_options.username, opt->val, MAXLEN);
+					}
+				}
+			}
+		}
+		else
+		{
+			param_set("dbname", runtime_options.dbname);
+		}
+	}
+
 
 	/* Exit here already if errors in command line options found */
 	if (cli_errors.head != NULL)
@@ -450,7 +566,6 @@ main(int argc, char **argv)
 		exit_with_errors();
 	}
 
-
 	if (check_upstream_config == true)
 	{
 		do_check_upstream_config();
@@ -620,14 +735,14 @@ main(int argc, char **argv)
 		}
 	}
 
-	keywords[2] = "user";
+/*	keywords[2] = "user";
 	values[2] = (runtime_options.username[0]) ? runtime_options.username : NULL;
 	keywords[3] = "dbname";
 	values[3] = runtime_options.dbname;
 	keywords[4] = "application_name";
 	values[4] = (char *) progname();
 	keywords[5] = NULL;
-	values[5] = NULL;
+	values[5] = NULL;*/
 
 	/*
 	 * Initialize the logger.  If verbose command line parameter was input,
@@ -1386,15 +1501,11 @@ do_standby_clone(void)
 				   runtime_options.dest_dir);
 	}
 
-	/* Connection parameters for master only */
+	param_set("application_name", options.node_name);
-	keywords[0] = "host";
-	values[0] = runtime_options.host;
-	keywords[1] = "port";
-	values[1] = runtime_options.masterport;
 
 	/* Connect to check configuration */
 	log_info(_("connecting to upstream node\n"));
-	upstream_conn = establish_db_connection_by_params(keywords, values, true);
+	upstream_conn = establish_db_connection_by_params((const char**)param_keywords, (const char**)param_values, true);
 
 	/* Verify that upstream node is a supported server version */
 	log_verbose(LOG_INFO, _("connected to upstream node, checking its state\n"));
@@ -2079,7 +2190,7 @@ stop_backup:
 		if (server_version_num >= 90400 &&
 			backup_label.min_failover_slot_lsn == InvalidXLogRecPtr)
 		{
-			maxlen_snprintf(dirpath, "%s/pg_replslot/*",
+			maxlen_snprintf(dirpath, "%s/pg_replslot/",
 							local_data_directory);
 
 			log_debug("deleting pg_replslot directory contents\n");
@@ -2101,7 +2212,7 @@ stop_backup:
 	}
 
 	/* Finally, write the recovery.conf file */
-	create_recovery_file(local_data_directory);
+	create_recovery_file(local_data_directory, upstream_conn);
 
 	if (runtime_options.rsync_only)
 	{
@@ -2533,7 +2644,7 @@ do_standby_follow(void)
 	 * to determine primary, and carry out some other checks while we're
 	 * at it.
 	 */
-	if ( *runtime_options.host == '\0')
+	if (host_param_provided == false)
 	{
 		/* We need to connect to check configuration */
 		log_info(_("connecting to standby database\n"));
@@ -2596,12 +2707,7 @@ do_standby_follow(void)
 	/* primary server info explictly provided - attempt to connect to that */
 	else
 	{
-		keywords[0] = "host";
+		master_conn = establish_db_connection_by_params((const char**)param_keywords, (const char**)param_values, true);
-		values[0] = runtime_options.host;
-		keywords[1] = "port";
-		values[1] = runtime_options.masterport;
-
-		master_conn = establish_db_connection_by_params(keywords, values, true);
 
 		master_id = get_master_node_id(master_conn, options.cluster_name);
 
@@ -2620,16 +2726,6 @@ do_standby_follow(void)
 		exit(ERR_BAD_CONFIG);
 	}
 
-
-	/*
-	 * set the host and masterport variables with the master ones before
-	 * closing the connection because we will need them to recreate the
-	 * recovery.conf file
-	 */
-	strncpy(runtime_options.host, PQhost(master_conn), MAXLEN);
-	strncpy(runtime_options.masterport, PQport(master_conn), MAXLEN);
-	strncpy(runtime_options.username, PQuser(master_conn), MAXLEN);
-
 	/*
 	 * If 9.4 or later, and replication slots in use, we'll need to create a
 	 * slot on the new master
@@ -2667,7 +2763,7 @@ do_standby_follow(void)
 	log_info(_("changing standby's master\n"));
 
 	/* write the recovery.conf file */
-	if (!create_recovery_file(data_dir))
+	if (!create_recovery_file(data_dir, master_conn))
 		exit(ERR_BAD_CONFIG);
 
 	/* Finally, restart the service */
@@ -3707,12 +3803,6 @@ do_witness_create(void)
 	char		repmgr_user[MAXLEN];
 	char		repmgr_db[MAXLEN];
 
-	/* Connection parameters for master only */
-	keywords[0] = "host";
-	values[0] = runtime_options.host;
-	keywords[1] = "port";
-	values[1] = runtime_options.masterport;
-
 	/*
 	 * Extract the repmgr user and database names from the conninfo string
 	 * provided in repmgr.conf
@@ -3721,7 +3811,8 @@ do_witness_create(void)
 	get_conninfo_value(options.conninfo, "dbname", repmgr_db);
 
 	/* We need to connect to check configuration and copy it */
-	masterconn = establish_db_connection_by_params(keywords, values, true);
+	masterconn = establish_db_connection_by_params((const char**)param_keywords, (const char**)param_values, true);
+
 	if (!masterconn)
 	{
 		/* No event logging possible here as we can't connect to the master */
@@ -4171,8 +4262,6 @@ do_witness_create(void)
 static void
 do_help(void)
 {
-	const char *host;
-
 	printf(_("%s: replication management tool for PostgreSQL\n"), progname());
 	printf(_("\n"));
 	printf(_("Usage:\n"));
@@ -4192,8 +4281,10 @@ do_help(void)
 	printf(_("\n"));
 	printf(_("Connection options:\n"));
 	printf(_("  -d, --dbname=DBNAME                 database to connect to (default: \"%s\")\n"), runtime_options.dbname);
-	host = getenv("PGHOST");
+	printf(_("  -h, --host=HOSTNAME                 database server host"));
-	printf(_("  -h, --host=HOSTNAME                 database server host or socket directory (default: \"%s\")\n"), host ? host : _("local socket"));
+	if (runtime_options.host[0] != '\0')
+		printf(_(" (default: \"%s\")"), runtime_options.host);
+	printf(_("\n"));
 	printf(_("  -p, --port=PORT                     database server port (default: \"%s\")\n"), runtime_options.masterport);
 	printf(_("  -U, --username=USERNAME             database user name to connect as (default: \"%s\")\n"), runtime_options.username);
 	printf(_("\n"));
@@ -4250,7 +4341,7 @@ do_help(void)
  * Creates a recovery file for a standby.
  */
 static bool
-create_recovery_file(const char *data_dir)
+create_recovery_file(const char *data_dir, PGconn *primary_conn)
 {
 	FILE	   *recovery_file;
 	char		recovery_file_path[MAXLEN];
@@ -4276,7 +4367,7 @@ create_recovery_file(const char *data_dir)
 	log_debug(_("recovery.conf: %s"), line);
 
 	/* primary_conninfo = '...' */
-	write_primary_conninfo(line);
+	write_primary_conninfo(line, primary_conn);
 
 	if (write_recovery_file_line(recovery_file, recovery_file_path, line) == false)
 		return false;
@@ -4490,11 +4581,25 @@ run_basebackup(const char *data_dir, int server_version)
 
 	appendPQExpBuffer(&params, " -D %s", data_dir);
 
+	if (opts != NULL && strlen(runtime_options.dbname))
+	{
+		appendPQExpBuffer(&params, " -d '%s'", runtime_options.dbname);
+	}
+
 	if (strlen(runtime_options.host))
 	{
 		appendPQExpBuffer(&params, " -h %s", runtime_options.host);
 	}
 
+	/*
+	 * XXX -p and -U will be duplicated if provided in conninfo string
+	 * but not as explict repmgr command line parameters, e.g.:
+	 *
+	 * pg_basebackup -l "repmgr base backup" -d 'host=localhost port=5501 dbname=repmgr user=repmgr' -p 5501 -U repmgr -X stream
+	 *
+	 * this is ugly but harmless; we should fix it some time
+	 *
+	 */
 	if (strlen(runtime_options.masterport))
 	{
 		appendPQExpBuffer(&params, " -p %s", runtime_options.masterport);
@@ -4650,7 +4755,7 @@ check_parameters_for_action(const int action)
 					error_list_append(&cli_errors, _("master hostname (-h/--host) required when executing STANDBY FOLLOW with -D/--data-dir option"));
 				}
 
-				if (!runtime_options.dest_dir[0])
+				if (host_param_provided && !runtime_options.dest_dir[0])
 				{
 					error_list_append(&cli_errors, _("local data directory (-D/--data-dir) required when executing STANDBY FOLLOW with -h/--host option"));
 				}
@@ -4679,12 +4784,14 @@ check_parameters_for_action(const int action)
 		case STANDBY_SWITCHOVER:
 			/* allow all parameters to be supplied */
 			break;
+
 		case STANDBY_ARCHIVE_CONFIG:
 			if (strcmp(runtime_options.config_archive_dir, "") == 0)
 			{
 				error_list_append(&cli_errors, _("--config-archive-dir required when executing STANDBY ARCHIVE_CONFIG"));
 			}
 			break;
+
 		case STANDBY_RESTORE_CONFIG:
 			if (strcmp(runtime_options.config_archive_dir, "") == 0)
 			{
@@ -4698,6 +4805,7 @@ check_parameters_for_action(const int action)
 
 			config_file_required = false;
 			break;
+
 		case WITNESS_CREATE:
 			/* Require data directory */
 			if (strcmp(runtime_options.dest_dir, "") == 0)
@@ -4706,9 +4814,11 @@ check_parameters_for_action(const int action)
 			}
 			/* allow all parameters to be supplied */
 			break;
+
 		case CLUSTER_SHOW:
 			/* allow all parameters to be supplied */
 			break;
+
 		case CLUSTER_CLEANUP:
 			/* allow all parameters to be supplied */
 			break;
@@ -5056,46 +5166,45 @@ create_schema(PGconn *conn)
 }
 
 
-/* This function uses global variables to determine connection settings. Special
- * usage of the PGPASSWORD variable is handled, but strongly discouraged */
 static void
-write_primary_conninfo(char *line)
+write_primary_conninfo(char *line, PGconn *primary_conn)
 {
-	char		host_buf[MAXLEN] = "";
+	PQconninfoOption *connOptions;
-	char		conn_buf[MAXLEN] = "";
+	PQconninfoOption *option;
-	char		user_buf[MAXLEN] = "";
+	PQExpBufferData conninfo_buf;
-	char		appname_buf[MAXLEN] = "";
+	bool application_name_provided = false;
-	char		password_buf[MAXLEN] = "";
 
-	/* Environment variable for password (UGLY, please use .pgpass!) */
+	connOptions = PQconninfo(primary_conn);
-	const char *password = getenv("PGPASSWORD");
 
-	if (password != NULL)
+	initPQExpBuffer(&conninfo_buf);
+
+	for (option = connOptions; option && option->keyword; option++)
 	{
-		maxlen_snprintf(password_buf, " password=%s", password);
+		/*
+		 * Skip empty settings and ones which don't make any sense in
+		 * recovery.conf
+		 */
+		if (strcmp(option->keyword, "dbname") == 0 ||
+		    strcmp(option->keyword, "replication") == 0 ||
+		    (option->val == NULL) ||
+		    (option->val != NULL && option->val[0] == '\0'))
+			continue;
+
+		if (conninfo_buf.len != 0)
+			appendPQExpBufferChar(&conninfo_buf, ' ');
+
+		if (strcmp(option->keyword, "application_name") == 0)
+			application_name_provided = true;
+
+		/* XXX escape option->val */
+		appendPQExpBuffer(&conninfo_buf, "%s=%s", option->keyword, option->val);
 	}
 
-	if (runtime_options.host[0])
+	/* `application_name` not provided - default to repmgr node name */
-	{
+	if (application_name_provided == false)
-		maxlen_snprintf(host_buf, " host=%s", runtime_options.host);
+		appendPQExpBuffer(&conninfo_buf, " application_name=%s", options.node_name);
-	}
 
-	if (runtime_options.username[0])
+	maxlen_snprintf(line, "primary_conninfo = '%s'\n", conninfo_buf.data);
-	{
-		maxlen_snprintf(user_buf, " user=%s", runtime_options.username);
-	}
-
-	if (options.node_name[0])
-	{
-		maxlen_snprintf(appname_buf, " application_name=%s", options.node_name);
-	}
-
-	maxlen_snprintf(conn_buf, "port=%s%s%s%s%s",
-	   (runtime_options.masterport[0]) ? runtime_options.masterport : DEF_PGPORT_STR,
-					host_buf, user_buf, password_buf,
-					appname_buf);
-
-	maxlen_snprintf(line, "primary_conninfo = '%s'\n", conn_buf);
 }
 
 
@@ -5484,17 +5593,11 @@ do_check_upstream_config(void)
 
 	parse_config(&options);
 
-	/* Connection parameters for upstream server only */
-	keywords[0] = "host";
-	values[0] = runtime_options.host;
-	keywords[1] = "port";
-	values[1] = runtime_options.masterport;
-	keywords[2] = "dbname";
-	values[2] = runtime_options.dbname;
-
 	/* We need to connect to check configuration and start a backup */
 	log_info(_("connecting to upstream server\n"));
-	conn = establish_db_connection_by_params(keywords, values, true);
+
+	conn = establish_db_connection_by_params((const char**)param_keywords, (const char**)param_values, true);
+
 
 	/* Verify that upstream server is a supported server version */
 	log_verbose(LOG_INFO, _("connected to upstream server, checking its state\n"));
@@ -5695,3 +5798,45 @@ copy_file(const char *old_filename, const char *new_filename)
 	return true;
 }
 
+static void
+param_set(const char *param, const char *value)
+{
+	int c;
+	int value_len = strlen(value) + 1;
+
+	/*
+	 * Scan array to see if the parameter is already set - if so replace it
+	 */
+	for (c = 0; c <= param_count && param_keywords[c] != NULL; c++)
+	{
+		if (strcmp(param_keywords[c], param) == 0)
+		{
+			if (param_values[c] != NULL)
+				pfree(param_values[c]);
+
+			param_values[c] = pg_malloc0(value_len);
+			strncpy(param_values[c], value, value_len);
+			return;
+		}
+	}
+
+	/*
+	 * Parameter not in array - add it and its associated value
+	 */
+	if (c < param_count)
+	{
+		int param_len = strlen(param) + 1;
+		param_keywords[c] = pg_malloc0(param_len);
+		param_values[c] = pg_malloc0(value_len);
+
+		strncpy(param_keywords[c], param, param_len);
+		strncpy(param_values[c], value, value_len);
+	}
+
+	/*
+	 * It's theoretically possible a parameter couldn't be added as
+	 * the array is full, but it's highly improbable so we won't
+	 * handle it at the moment.
+	 */
+}
+

repmgrd.c

@@ -1121,7 +1121,6 @@ standby_monitor(void)
 					  last_xlog_receive_location,
 					  replication_lag,
 					  apply_lag);
-
 	/*
 	 * Execute the query asynchronously, but don't check for a result. We will
 	 * check the result next time we pause for a monitor step.

version.h

@@ -1,6 +1,6 @@
 #ifndef _VERSION_H_
 #define _VERSION_H_
 
-#define REPMGR_VERSION "3.2dev"
+#define REPMGR_VERSION "3.1.4"
 
 #endif