Refactor server version detection

Most of the time we can simply get the version number directly from
the connection handle. Previously it was held in a global variable,
which was an icky way of doing things.

In a few special cases we also need the actual version string, which
is obtained directly from the database.

dbutils.c

@@ -32,8 +32,6 @@
 
 #define NODE_RECORD_PARAM_COUNT 11
 
-/* mainly for use by repmgrd */
-int			server_version_num = UNKNOWN_SERVER_VERSION_NUM;
 
 /*
  * This is set by is_bdr_db(), which is called by every BDR-related
@@ -1096,14 +1094,16 @@ get_cluster_size(PGconn *conn, char *size)
 	return success;
 }
 
+
 /*
  * Return the server version number for the connection provided
  */
 int
-get_server_version(PGconn *conn, char *server_version)
+get_server_version(PGconn *conn, char *server_version_buf)
 {
 	PGresult   *res = NULL;
-	int			server_version_num;
+	int			_server_version_num = UNKNOWN_SERVER_VERSION_NUM;
+
 	const char	   *sqlquery =
 		"SELECT pg_catalog.current_setting('server_version_num'), "
 		"       pg_catalog.current_setting('server_version')";
@@ -1115,15 +1115,17 @@ get_server_version(PGconn *conn, char *server_version)
 		log_db_error(conn, sqlquery, _("unable to determine server version number"));
 		PQclear(res);
 
-		return -1;
+		return UNKNOWN_SERVER_VERSION_NUM;
 	}
 
-	if (server_version != NULL)
-	{
-		char		server_version_buf[MAXVERSIONSTR];
-		int			i;
+	_server_version_num = atoi(PQgetvalue(res, 0, 0));
 
-		memset(server_version_buf, 0, MAXVERSIONSTR);
+	if (server_version_buf != NULL)
+	{
+		int			i;
+		char		_server_version_buf[MAXVERSIONSTR] = "";
+
+		memset(_server_version_buf, 0, MAXVERSIONSTR);
 
 		/*
 		 * Some distributions may add extra info after the actual version number,
@@ -1131,22 +1133,20 @@ get_server_version(PGconn *conn, char *server_version)
 		 * first space.
 		 */
 
-		strncpy(server_version_buf, PQgetvalue(res, 0, 1), MAXVERSIONSTR);
+		strncpy(_server_version_buf, PQgetvalue(res, 0, 1), MAXVERSIONSTR);
 
 		for (i = 0; i < MAXVERSIONSTR; i++)
 		{
-			if (server_version_buf[i] == ' ')
+			if (_server_version_buf[i] == ' ')
 				break;
 
-			*server_version++ = server_version_buf[i];
+			*server_version_buf++ = _server_version_buf[i];
 		}
 	}
 
-	server_version_num = atoi(PQgetvalue(res, 0, 0));
-
 	PQclear(res);
 
-	return server_version_num;
+	return _server_version_num;
 }
 
 
@@ -1382,9 +1382,6 @@ get_replication_info(PGconn *conn, ReplInfo *replication_info)
 	PGresult   *res = NULL;
 	bool		success = true;
 
-	if (server_version_num == UNKNOWN_SERVER_VERSION_NUM)
-		server_version_num = get_server_version(conn, NULL);
-
 	initPQExpBuffer(&query);
 	appendPQExpBufferStr(&query,
 						 " SELECT ts, "
@@ -1399,7 +1396,7 @@ get_replication_info(PGconn *conn, ReplInfo *replication_info)
 						 "        COALESCE(last_wal_receive_lsn, '0/0') >= last_wal_replay_lsn AS receiving_streamed_wal "
 						 "   FROM ( ");
 
-	if (server_version_num >= 100000)
+	if (PQserverVersion(conn) >= 100000)
 	{
 		appendPQExpBufferStr(&query,
 							 " SELECT CURRENT_TIMESTAMP AS ts, "
@@ -1456,10 +1453,7 @@ get_ready_archive_files(PGconn *conn, const char *data_directory)
 
 	int			ready_count = 0;
 
-	if (server_version_num == UNKNOWN_SERVER_VERSION_NUM)
-		server_version_num = get_server_version(conn, NULL);
-
-	if (server_version_num >= 100000)
+	if (PQserverVersion(conn) >= 100000)
 	{
 		snprintf(archive_status_dir, MAXPGPATH,
 				 "%s/pg_wal/archive_status",
@@ -1533,12 +1527,9 @@ get_replication_lag_seconds(PGconn *conn)
 	PGresult   *res = NULL;
 	int			lag_seconds = 0;
 
-	if (server_version_num == UNKNOWN_SERVER_VERSION_NUM)
-		server_version_num = get_server_version(conn, NULL);
-
 	initPQExpBuffer(&query);
 
-	if (server_version_num >= 100000)
+	if (PQserverVersion(conn) >= 100000)
 	{
 		appendPQExpBufferStr(&query,
 							 " SELECT CASE WHEN (pg_catalog.pg_last_wal_receive_lsn() = pg_catalog.pg_last_wal_replay_lsn()) ");
@@ -3107,16 +3098,11 @@ update_node_record_slot_name(PGconn *primary_conn, int node_id, char *slot_name)
 
 
 void
-get_node_replication_stats(PGconn *conn, int server_version_num, t_node_info *node_info)
+get_node_replication_stats(PGconn *conn, t_node_info *node_info)
 {
 	PQExpBufferData query;
 	PGresult   *res = NULL;
 
-	if (server_version_num == UNKNOWN_SERVER_VERSION_NUM)
-		server_version_num = get_server_version(conn, NULL);
-
-	Assert(server_version_num != UNKNOWN_SERVER_VERSION_NUM);
-
 	initPQExpBuffer(&query);
 
 	appendPQExpBufferStr(&query,
@@ -3124,7 +3110,7 @@ get_node_replication_stats(PGconn *conn, int server_version_num, t_node_info *no
 						 "        (SELECT pg_catalog.count(*) FROM pg_catalog.pg_stat_replication) AS attached_wal_receivers, ");
 
 	/* no replication slots in PostgreSQL 9.3 */
-	if (server_version_num < 90400)
+	if (PQserverVersion(conn) < 90400)
 	{
 		appendPQExpBufferStr(&query,
 							 "        0 AS max_replication_slots, "
@@ -3958,9 +3944,6 @@ create_replication_slot(PGconn *conn, char *slot_name, int server_version_num, P
 	PGresult   *res = NULL;
 	t_replication_slot slot_info = T_REPLICATION_SLOT_INITIALIZER;
 
-	if (server_version_num == UNKNOWN_SERVER_VERSION_NUM)
-		server_version_num = get_server_version(conn, NULL);
-
 	/*
 	 * Check whether slot exists already; if it exists and is active, that
 	 * means another active standby is using it, which creates an error
@@ -3997,7 +3980,7 @@ create_replication_slot(PGconn *conn, char *slot_name, int server_version_num, P
 	initPQExpBuffer(&query);
 
 	/* In 9.6 and later, reserve the LSN straight away */
-	if (server_version_num >= 90600)
+	if (PQserverVersion(conn) >= 90600)
 	{
 		appendPQExpBuffer(&query,
 						  "SELECT * FROM pg_catalog.pg_create_physical_replication_slot('%s', TRUE)",
@@ -4762,7 +4745,7 @@ get_current_wal_lsn(PGconn *conn)
 	PGresult   *res = NULL;
 	XLogRecPtr	ptr = InvalidXLogRecPtr;
 
-	if (server_version_num >= 100000)
+	if (PQserverVersion(conn) >= 100000)
 	{
 		res = PQexec(conn, "SELECT pg_catalog.pg_current_wal_lsn()");
 	}
@@ -4787,7 +4770,7 @@ get_last_wal_receive_location(PGconn *conn)
 	PGresult   *res = NULL;
 	XLogRecPtr	ptr = InvalidXLogRecPtr;
 
-	if (server_version_num >= 100000)
+	if (PQserverVersion(conn) >= 100000)
 	{
 		res = PQexec(conn, "SELECT pg_catalog.pg_last_wal_receive_lsn()");
 	}
@@ -4815,47 +4798,7 @@ get_current_lsn(PGconn *conn)
 
 	initPQExpBuffer(&query);
 
-	if (server_version_num == UNKNOWN_SERVER_VERSION_NUM)
-		server_version_num = get_server_version(conn, NULL);
-
-/*
-WITH lsn_states AS (
-  SELECT
-    CASE WHEN pg_catalog.pg_is_in_recovery() IS FALSE
-      THEN pg_catalog.pg_current_wal_lsn()
-      ELSE NULL
-    END
-      AS current_wal_lsn,
-    CASE WHEN pg_catalog.pg_is_in_recovery() IS TRUE
-	   THEN pg_catalog.pg_last_wal_receive_lsn()
-       ELSE NULL
-    END
-      AS last_wal_receive_lsn,
-    CASE WHEN pg_catalog.pg_is_in_recovery() IS TRUE
-	   THEN pg_catalog.pg_last_wal_replay_lsn()
-       ELSE NULL
-    END
-      AS last_wal_replay_lsn
-)
-SELECT
-  CASE WHEN pg_catalog.pg_is_in_recovery() IS FALSE
-    THEN current_wal_lsn
-    ELSE
-      CASE WHEN last_wal_receive_lsn IS NULL
-         THEN last_wal_replay_lsn
-         ELSE
-           CASE WHEN last_wal_replay_lsn > last_wal_receive_lsn
-             THEN last_wal_replay_lsn
-             ELSE last_wal_receive_lsn
-           END
-       END
-  END
-    AS current_lsn
-   FROM lsn_states
-
-
-*/
-	if (server_version_num >= 100000)
+	if (PQserverVersion(conn) >= 100000)
 	{
 		appendPQExpBufferStr(&query,
 							 " WITH lsn_states AS ( "

dbutils.h

@@ -358,10 +358,6 @@ typedef struct RepmgrdInfo {
 } RepmgrdInfo;
 
 
-/* global variables */
-
-extern int	server_version_num;
-
 /* macros */
 
 #define is_streaming_replication(x) (x == PRIMARY || x == STANDBY)
@@ -420,7 +416,8 @@ bool		get_pg_setting(PGconn *conn, const char *setting, char *output);
 
 /* server information functions */
 bool		get_cluster_size(PGconn *conn, char *size);
-int			get_server_version(PGconn *conn, char *server_version);
+int			get_server_version(PGconn *conn, char *server_version_buf);
+
 RecoveryType get_recovery_type(PGconn *conn);
 int			get_primary_node_id(PGconn *conn);
 int			get_ready_archive_files(PGconn *conn, const char *data_directory);
@@ -548,7 +545,7 @@ XLogRecPtr	get_last_wal_receive_location(PGconn *conn);
 XLogRecPtr	get_current_lsn(PGconn *conn);
 bool		get_replication_info(PGconn *conn, ReplInfo *replication_info);
 int			get_replication_lag_seconds(PGconn *conn);
-void		get_node_replication_stats(PGconn *conn, int server_version_num, t_node_info *node_info);
+void		get_node_replication_stats(PGconn *conn, t_node_info *node_info);
 bool		is_downstream_node_attached(PGconn *conn, char *node_name);
 void		set_primary_last_seen(PGconn *conn);
 int			get_primary_last_seen(PGconn *conn);

repmgr-action-node.c

@@ -66,7 +66,6 @@ do_node_status(void)
 	PGconn	   *conn = NULL;
 
 	t_node_info node_info = T_NODE_INFO_INITIALIZER;
-	char		server_version[MAXLEN];
 	char		cluster_size[MAXLEN];
 	PQExpBufferData output;
 
@@ -80,6 +79,8 @@ do_node_status(void)
 	t_recovery_conf recovery_conf = T_RECOVERY_CONF_INITIALIZER;
 
 	char		data_dir[MAXPGPATH] = "";
+	int			server_version_num = UNKNOWN_SERVER_VERSION_NUM;
+	char		server_version_str[MAXVERSIONSTR] = "";
 
 	if (runtime_options.is_shutdown_cleanly == true)
 	{
@@ -90,7 +91,7 @@ do_node_status(void)
 	conn = establish_db_connection(config_file_options.conninfo, true);
 	strncpy(data_dir, config_file_options.data_directory, MAXPGPATH);
 
-	server_version_num = get_server_version(conn, NULL);
+	server_version_num = get_server_version(conn, server_version_str);
 
 	/* check node exists  */
 
@@ -101,18 +102,16 @@ do_node_status(void)
 		exit(ERR_BAD_CONFIG);
 	}
 
-	(void) get_server_version(conn, server_version);
-
 	if (get_cluster_size(conn, cluster_size) == false)
 		strncpy(cluster_size, _("unknown"), MAXLEN);
 
 	recovery_type = get_recovery_type(conn);
 
-	get_node_replication_stats(conn, server_version_num, &node_info);
+	get_node_replication_stats(conn, &node_info);
 
 	key_value_list_set(&node_status,
 					   "PostgreSQL version",
-					   server_version);
+					   server_version_str);
 
 	key_value_list_set(&node_status,
 					   "Total data size",
@@ -725,10 +724,8 @@ do_node_check(void)
 		exit(ERR_BAD_CONFIG);
 	}
 
-	server_version_num = get_server_version(conn, NULL);
-
 	/* add replication statistics to node record */
-	get_node_replication_stats(conn, server_version_num, &node_info);
+	get_node_replication_stats(conn, &node_info);
 
 	/*
 	 * handle specific checks ======================
@@ -1618,7 +1615,7 @@ do_node_check_slots(PGconn *conn, OutputMode mode, t_node_info *node_info, Check
 
 	initPQExpBuffer(&details);
 
-	if (server_version_num < 90400)
+	if (PQserverVersion(conn) < 90400)
 	{
 		appendPQExpBufferStr(&details,
 							 _("replication slots not available for this PostgreSQL version"));
@@ -1694,7 +1691,7 @@ do_node_check_missing_slots(PGconn *conn, OutputMode mode, t_node_info *node_inf
 
 	initPQExpBuffer(&details);
 
-	if (server_version_num < 90400)
+	if (PQserverVersion(conn) < 90400)
 	{
 		appendPQExpBufferStr(&details,
 							 _("replication slots not available for this PostgreSQL version"));
@@ -2005,7 +2002,6 @@ do_node_rejoin(void)
 	t_node_info primary_node_record = T_NODE_INFO_INITIALIZER;
 
 	bool		success = true;
-	int			server_version_num = UNKNOWN_SERVER_VERSION_NUM;
 	int			follow_error_code = SUCCESS;
 
 	/* check node is not actually running */
@@ -2063,9 +2059,8 @@ do_node_rejoin(void)
 	upstream_conn = establish_db_connection_by_params(&source_conninfo, true);
 
 	/* sanity checks for 9.3 */
-	server_version_num = get_server_version(upstream_conn, NULL);
 
-	if (server_version_num < 90400)
+	if (PQserverVersion(upstream_conn) < 90400)
 		check_93_config();
 
 	if (get_primary_node_record(upstream_conn, &primary_node_record) == false)

repmgr-action-standby.c

@@ -1083,7 +1083,7 @@ _do_create_recovery_conf(void)
 		/* if not, if check one can and should be created */
 		else
 		{
-			get_node_replication_stats(upstream_conn, UNKNOWN_SERVER_VERSION_NUM, &upstream_node_record);
+			get_node_replication_stats(upstream_conn, &upstream_node_record);
 
 		    if (upstream_node_record.max_replication_slots > upstream_node_record.total_replication_slots)
 			{
@@ -2194,7 +2194,6 @@ do_standby_follow(void)
 	t_event_info event_info = T_EVENT_INFO_INITIALIZER;
 
 	int			timer = 0;
-	int			server_version_num = UNKNOWN_SERVER_VERSION_NUM;
 
 	PQExpBufferData follow_output;
 	bool		success = false;
@@ -2221,9 +2220,7 @@ do_standby_follow(void)
 	check_recovery_type(local_conn);
 
 	/* sanity-checks for 9.3 */
-	server_version_num = get_server_version(local_conn, NULL);
-
-	if (server_version_num < 90400)
+	if (PQserverVersion(local_conn) < 90400)
 		check_93_config();
 
 	/*
@@ -2577,7 +2574,7 @@ do_standby_follow_internal(PGconn *primary_conn, t_node_info *primary_node_recor
 
 	if (config_file_options.use_replication_slots)
 	{
-		int			primary_server_version_num = get_server_version(primary_conn, NULL);
+		int			primary_server_version_num = PQserverVersion(primary_conn);
 
 		/*
 		 * Here we add a sanity check for the "slot_name" field - it's possible
@@ -3202,7 +3199,7 @@ do_standby_switchover(void)
 	 * populate local node record with current state of various replication-related
 	 * values, so we can check for sufficient walsenders and replication slots
 	 */
-	get_node_replication_stats(local_conn, server_version_num, &local_node_record);
+	get_node_replication_stats(local_conn, &local_node_record);
 
 	available_wal_senders = local_node_record.max_wal_senders -
 		local_node_record.attached_wal_receivers;

repmgr-client.c

@@ -2238,17 +2238,15 @@ create_repmgr_extension(PGconn *conn)
 int
 check_server_version(PGconn *conn, char *server_type, bool exit_on_error, char *server_version_string)
 {
-	int			conn_server_version_num = UNKNOWN_SERVER_VERSION_NUM;
+	int			conn_server_version_num = get_server_version(conn, server_version_string);
 
-	conn_server_version_num = get_server_version(conn, server_version_string);
 	if (conn_server_version_num < MIN_SUPPORTED_VERSION_NUM)
 	{
 		if (conn_server_version_num > 0)
 			log_error(_("%s requires %s to be PostgreSQL %s or later"),
 					  progname(),
 					  server_type,
-					  MIN_SUPPORTED_VERSION
-				);
+					  MIN_SUPPORTED_VERSION);
 
 		if (exit_on_error == true)
 		{
@@ -2256,7 +2254,7 @@ check_server_version(PGconn *conn, char *server_type, bool exit_on_error, char *
 			exit(ERR_BAD_CONFIG);
 		}
 
-		return -1;
+		return UNKNOWN_SERVER_VERSION_NUM;
 	}
 
 	return conn_server_version_num;
@@ -3032,10 +3030,9 @@ bool
 can_use_pg_rewind(PGconn *conn, const char *data_directory, PQExpBufferData *reason)
 {
 	bool		can_use = true;
-	int			server_version_num = get_server_version(conn, NULL);
 
 	/* wal_log_hints not available in 9.3, so just determine if data checksums enabled */
-	if (server_version_num < 90400)
+	if (PQserverVersion(conn) < 90400)
 	{
 		int			data_checksum_version = get_data_checksum_version(data_directory);
 

repmgrd.c

@@ -372,12 +372,6 @@ main(int argc, char **argv)
 	/* abort if local node not available at startup */
 	local_conn = establish_db_connection(config_file_options.conninfo, true);
 
-	/*
-	 * store the server version number - we'll need this to generate
-	 * version-dependent queries etc.
-	 */
-	server_version_num = get_server_version(local_conn, NULL);
-
 	/*
 	 * sanity checks
 	 *