From f94626bf7b8198355dd3f815bfc4623f7ec43541 Mon Sep 17 00:00:00 2001 From: Ian Barwick Date: Mon, 29 Dec 2014 14:54:04 +0900 Subject: [PATCH] Refactor version number detection Use the reported `server_version_num` integer for version number detection and comparison. This makes it easier to set an arbitrary minimum supported version (rather than "9.0 or later") as well as future-proofing for 10.x and later. --- dbutils.c | 36 ++--------- dbutils.h | 2 +- repmgr.c | 187 +++++++++++++++++++++++++++++------------------------- repmgr.h | 4 ++ repmgrd.c | 20 +++--- 5 files changed, 123 insertions(+), 126 deletions(-) diff --git a/dbutils.c b/dbutils.c index 18527331..81565dd0 100644 --- a/dbutils.c +++ b/dbutils.c @@ -178,46 +178,24 @@ is_pgup(PGconn *conn, int timeout) /* - * If postgreSQL version is 9 or superior returns the major version - * if 8 or inferior returns an empty string + * Return the server version number for the connection provided */ -char * -pg_version(PGconn *conn, char *major_version) +int +get_server_version_num(PGconn *conn) { PGresult *res; - - int major_version1; - char *major_version2; - res = PQexec(conn, - "WITH pg_version(ver) AS " - "(SELECT split_part(version(), ' ', 2)) " - "SELECT split_part(ver, '.', 1), split_part(ver, '.', 2) " - "FROM pg_version"); + "SELECT current_setting('server_version_num')"); if (PQresultStatus(res) != PGRES_TUPLES_OK) { - log_err(_("Version check PQexec failed: %s"), + log_err(_("Unable to determine server verson number:\n%s"), PQerrorMessage(conn)); PQclear(res); - return NULL; + return -1; } - major_version1 = atoi(PQgetvalue(res, 0, 0)); - major_version2 = PQgetvalue(res, 0, 1); - - if (major_version1 >= 9) - { - /* form a major version string */ - xsnprintf(major_version, MAXVERSIONSTR, "%d.%s", major_version1, - major_version2); - } - else - strcpy(major_version, ""); - - PQclear(res); - - return major_version; + return atoi(PQgetvalue(res, 0, 0)); } diff --git a/dbutils.h b/dbutils.h index 39d2233f..b36fef0e 100644 --- a/dbutils.h +++ b/dbutils.h @@ -30,7 +30,7 @@ PGconn *establish_db_connection_by_params(const char *keywords[], int is_standby(PGconn *conn); int is_witness(PGconn *conn, char *schema, char *cluster, int node_id); bool is_pgup(PGconn *conn, int timeout); -char *pg_version(PGconn *conn, char *major_version); +int get_server_version_num(PGconn *conn); int guc_set(PGconn *conn, const char *parameter, const char *op, const char *value); int guc_set_typed(PGconn *conn, const char *parameter, const char *op, diff --git a/repmgr.c b/repmgr.c index fbafda0e..9d0d0021 100644 --- a/repmgr.c +++ b/repmgr.c @@ -525,28 +525,30 @@ do_master_register(void) { PGconn *conn; PGresult *res; - char sqlquery[QUERY_STR_LEN], - *ret_ver; + char sqlquery[QUERY_STR_LEN]; bool schema_exists = false; char schema_quoted[MAXLEN]; - char master_version[MAXVERSIONSTR]; + int master_version_num = 0; int ret; conn = establish_db_connection(options.conninfo, true); - /* master should be v9 or better */ + /* Verify that master is a supported server version */ log_info(_("%s connecting to master database\n"), progname); - ret_ver = pg_version(conn, master_version); - if (ret_ver == NULL || strcmp(master_version, "") == 0) + master_version_num = get_server_version_num(conn); + if(master_version_num < MIN_SUPPORTED_VERSION_NUM) { PQfinish(conn); - if (ret_ver != NULL) - log_err(_("%s needs master to be PostgreSQL 9.0 or better\n"), - progname); - return; + if (master_version_num > 0) + log_err(_("%s needs master to be PostgreSQL %s or better\n"), + progname, + MIN_SUPPORTED_VERSION + ); + exit(ERR_BAD_CONFIG); } + /* Check we are a master */ log_info(_("%s connected to master, checking its state\n"), progname); ret = is_standby(conn); @@ -678,27 +680,27 @@ do_standby_register(void) ret; PGresult *res; - char sqlquery[QUERY_STR_LEN], - *ret_ver; + char sqlquery[QUERY_STR_LEN]; char schema_quoted[MAXLEN]; - char master_version[MAXVERSIONSTR]; - char standby_version[MAXVERSIONSTR]; + int master_version_num = 0; + int standby_version_num = 0; /* XXX: A lot of copied code from do_master_register! Refactor */ log_info(_("%s connecting to standby database\n"), progname); conn = establish_db_connection(options.conninfo, true); - /* should be v9 or better */ - log_info(_("%s connected to standby, checking its state\n"), progname); - ret_ver = pg_version(conn, standby_version); - if (ret_ver == NULL || strcmp(standby_version, "") == 0) + /* Verify that standby is a supported server version */ + standby_version_num = get_server_version_num(conn); + if(standby_version_num < MIN_SUPPORTED_VERSION_NUM) { PQfinish(conn); - if (ret_ver != NULL) - log_err(_("%s needs standby to be PostgreSQL 9.0 or better\n"), - progname); + if (standby_version_num > 0) + log_err(_("%s needs standby to be PostgreSQL %s or better\n"), + progname, + MIN_SUPPORTED_VERSION + ); exit(ERR_BAD_CONFIG); } @@ -733,6 +735,7 @@ do_standby_register(void) res = PQexec(conn, sqlquery); if (PQresultStatus(res) != PGRES_TUPLES_OK) { + /* XXX error message does not match query */ log_err(_("Can't get info about tablespaces: %s\n"), PQerrorMessage(conn)); PQclear(res); @@ -760,26 +763,29 @@ do_standby_register(void) exit(ERR_BAD_CONFIG); } - /* master should be v9 or better */ + /* Verify that master is a supported server version */ log_info(_("%s connected to master, checking its state\n"), progname); - ret_ver = pg_version(master_conn, master_version); - if (ret_ver == NULL || strcmp(master_version, "") == 0) + master_version_num = get_server_version_num(conn); + if(master_version_num < MIN_SUPPORTED_VERSION_NUM) { + if (master_version_num > 0) + log_err(_("%s needs master to be PostgreSQL %s or better\n"), + progname, + MIN_SUPPORTED_VERSION + ); PQfinish(conn); PQfinish(master_conn); - if (ret_ver != NULL) - log_err(_("%s needs master to be PostgreSQL 9.0 or better\n"), - progname); exit(ERR_BAD_CONFIG); } /* master and standby version should match */ - if (strcmp(master_version, standby_version) != 0) + if ((master_version_num / 100) != (standby_version_num / 100)) { PQfinish(conn); PQfinish(master_conn); - log_err(_("%s needs versions of both master (%s) and standby (%s) to match.\n"), - progname, master_version, standby_version); + /* XXX format version numbers */ + log_err(_("%s needs versions of both master (%i) and standby (%i) to match.\n"), + progname, master_version_num, standby_version_num); exit(ERR_BAD_CONFIG); } @@ -835,8 +841,8 @@ do_standby_clone(void) { PGconn *conn; PGresult *res; - char sqlquery[QUERY_STR_LEN], - *ret; + char sqlquery[QUERY_STR_LEN]; + const char *cluster_size; int r = 0, @@ -867,7 +873,7 @@ do_standby_clone(void) char *first_wal_segment = NULL; const char *last_wal_segment = NULL; - char master_version[MAXVERSIONSTR]; + int master_version_num = 0; /* * if dest_dir has been provided, we copy everything in the same path if @@ -892,15 +898,18 @@ do_standby_clone(void) log_info(_("%s connecting to master database\n"), progname); conn = establish_db_connection_by_params(keywords, values, true); - /* primary should be v9 or better */ + /* Verify that master is a supported server version */ log_info(_("%s connected to master, checking its state\n"), progname); - ret = pg_version(conn, master_version); - if (ret == NULL || strcmp(master_version, "") == 0) + + master_version_num = get_server_version_num(conn); + if(master_version_num < MIN_SUPPORTED_VERSION_NUM) { PQfinish(conn); - if (ret != NULL) - log_err(_("%s needs master to be PostgreSQL 9.0 or better\n"), - progname); + if (master_version_num > 0) + log_err(_("%s needs master to be PostgreSQL %s or better\n"), + progname, + MIN_SUPPORTED_VERSION + ); exit(ERR_BAD_CONFIG); } @@ -960,8 +969,7 @@ do_standby_clone(void) /* * Check if the tablespace locations exists and that we can write to them. */ - if (strcmp(master_version, "9.0") == 0 || - strcmp(master_version, "9.1") == 0) + if (master_version_num < 90200) sqlquery_snprintf(sqlquery, "SELECT spclocation " " FROM pg_tablespace " @@ -1207,8 +1215,7 @@ do_standby_clone(void) * test_mode but it does not hurt too much (except if a tablespace is * created during the test) */ - if (strcmp(master_version, "9.0") == 0 || - strcmp(master_version, "9.1") == 0) + if (master_version_num < 90200) sqlquery_snprintf(sqlquery, "SELECT spclocation " " FROM pg_tablespace " @@ -1367,8 +1374,8 @@ do_standby_promote(void) { PGconn *conn; PGresult *res; - char sqlquery[QUERY_STR_LEN], - *ret; + char sqlquery[QUERY_STR_LEN]; + char script[MAXLEN]; PGconn *old_master_conn; @@ -1380,21 +1387,24 @@ do_standby_promote(void) char recovery_file_path[MAXFILENAME]; char recovery_done_path[MAXFILENAME]; - char standby_version[MAXVERSIONSTR]; + int standby_version_num = 0; /* We need to connect to check configuration */ log_info(_("%s connecting to standby database\n"), progname); conn = establish_db_connection(options.conninfo, true); - /* we need v9 or better */ + /* Verify that standby is a supported server version */ log_info(_("%s connected to standby, checking its state\n"), progname); - ret = pg_version(conn, standby_version); - if (ret == NULL || strcmp(standby_version, "") == 0) + + standby_version_num = get_server_version_num(conn); + if(standby_version_num < MIN_SUPPORTED_VERSION_NUM) { - if (ret != NULL) - log_err(_("%s needs standby to be PostgreSQL 9.0 or better\n"), - progname); PQfinish(conn); + if (standby_version_num > 0) + log_err(_("%s needs standby to be PostgreSQL %s or better\n"), + progname, + MIN_SUPPORTED_VERSION + ); exit(ERR_BAD_CONFIG); } @@ -1485,8 +1495,7 @@ do_standby_follow(void) { PGconn *conn; PGresult *res; - char sqlquery[QUERY_STR_LEN], - *ret; + char sqlquery[QUERY_STR_LEN]; char script[MAXLEN]; char master_conninfo[MAXLEN]; PGconn *master_conn; @@ -1496,13 +1505,27 @@ do_standby_follow(void) retval; char data_dir[MAXLEN]; - char master_version[MAXVERSIONSTR]; - char standby_version[MAXVERSIONSTR]; + int master_version_num = 0; + int standby_version_num = 0; + /* We need to connect to check configuration */ log_info(_("%s connecting to standby database\n"), progname); conn = establish_db_connection(options.conninfo, true); + /* Verify that standby is a supported server version */ + standby_version_num = get_server_version_num(conn); + if(standby_version_num < MIN_SUPPORTED_VERSION_NUM) + { + PQfinish(conn); + if (standby_version_num > 0) + log_err(_("%s needs standby to be PostgreSQL %s or better\n"), + progname, + MIN_SUPPORTED_VERSION + ); + exit(ERR_BAD_CONFIG); + } + /* Check we are in a standby node */ log_info(_("%s connected to standby, checking its state\n"), progname); retval = is_standby(conn); @@ -1515,17 +1538,6 @@ do_standby_follow(void) exit(ERR_BAD_CONFIG); } - /* should be v9 or better */ - ret = pg_version(conn, standby_version); - if (ret == NULL || strcmp(standby_version, "") == 0) - { - if (ret != NULL) - log_err(_("%s needs standby to be PostgreSQL 9.0 or better\n"), - progname); - PQfinish(conn); - exit(ERR_BAD_CONFIG); - } - /* * we also need to check if there is any master in the cluster or wait for * one to appear if we have set the wait option @@ -1563,26 +1575,29 @@ do_standby_follow(void) exit(ERR_BAD_CONFIG); } - /* should be v9 or better */ + /* Verify that master is a supported server version */ log_info(_("%s connected to master, checking its state\n"), progname); - ret = pg_version(master_conn, master_version); - if (ret == NULL || strcmp(master_version, "") == 0) + master_version_num = get_server_version_num(conn); + if(master_version_num < MIN_SUPPORTED_VERSION_NUM) { - if (ret != NULL) - log_err(_("%s needs master to be PostgreSQL 9.0 or better\n"), - progname); + if (master_version_num > 0) + log_err(_("%s needs master to be PostgreSQL %s or better\n"), + progname, + MIN_SUPPORTED_VERSION + ); PQfinish(conn); PQfinish(master_conn); exit(ERR_BAD_CONFIG); } /* master and standby version should match */ - if (strcmp(master_version, standby_version) != 0) + if ((master_version_num / 100) != (standby_version_num / 100)) { - log_err(_("%s needs versions of both master (%s) and standby (%s) to match.\n"), - progname, master_version, standby_version); PQfinish(conn); PQfinish(master_conn); + /* XXX format version numbers */ + log_err(_("%s needs versions of both master (%i) and standby (%i) to match.\n"), + progname, master_version_num, standby_version_num); exit(ERR_BAD_CONFIG); } @@ -1639,8 +1654,7 @@ do_witness_create(void) PGconn *masterconn; PGconn *witnessconn; PGresult *res; - char sqlquery[QUERY_STR_LEN], - *ret; + char sqlquery[QUERY_STR_LEN]; char script[MAXLEN]; char buf[MAXLEN]; @@ -1650,9 +1664,8 @@ do_witness_create(void) retval; int i; - char master_version[MAXVERSIONSTR]; - char master_hba_file[MAXLEN]; + int master_version_num = 0; /* Connection parameters for master only */ keywords[0] = "host"; @@ -1668,14 +1681,16 @@ do_witness_create(void) exit(ERR_DB_CON); } - /* primary should be v9 or better */ - ret = pg_version(masterconn, master_version); - if (ret == NULL || strcmp(master_version, "") == 0) + /* Verify that master is a supported server version */ + master_version_num = get_server_version_num(masterconn); + if(master_version_num < MIN_SUPPORTED_VERSION_NUM) { - if (ret != NULL) - log_err(_("%s needs master to be PostgreSQL 9.0 or better\n"), - progname); PQfinish(masterconn); + if (master_version_num > 0) + log_err(_("%s needs master to be PostgreSQL %s or better\n"), + progname, + MIN_SUPPORTED_VERSION + ); exit(ERR_BAD_CONFIG); } diff --git a/repmgr.h b/repmgr.h index c7c43f1e..4888c271 100644 --- a/repmgr.h +++ b/repmgr.h @@ -32,6 +32,9 @@ #define STANDBY_MODE 1 #define WITNESS_MODE 2 +#define MIN_SUPPORTED_VERSION "9.1" +#define MIN_SUPPORTED_VERSION_NUM 90100 + #include "config.h" #define MAXFILENAME 1024 #define ERRBUFF_SIZE 512 @@ -46,6 +49,7 @@ #define MANUAL_FAILOVER 0 #define AUTOMATIC_FAILOVER 1 + /* Run time options type */ typedef struct { diff --git a/repmgrd.c b/repmgrd.c index ad4dcd9b..1e28d189 100644 --- a/repmgrd.c +++ b/repmgrd.c @@ -174,9 +174,7 @@ main(int argc, char **argv) bool daemonize = false; FILE *fd; - char standby_version[MAXVERSIONSTR], - *ret_ver; - + int server_version_num = 0; progname = get_progname(argv[0]); if (argc > 1) @@ -280,14 +278,16 @@ main(int argc, char **argv) local_options.conninfo); my_local_conn = establish_db_connection(local_options.conninfo, true); - /* should be v9 or better */ - log_info(_("%s Connected to database, checking its state\n"), progname); - ret_ver = pg_version(my_local_conn, standby_version); - if (ret_ver == NULL || strcmp(standby_version, "") == 0) + /* Verify that server is a supported version */ + log_info(_("%s connected to database, checking its state\n"), progname); + server_version_num = get_server_version_num(my_local_conn); + if(server_version_num < MIN_SUPPORTED_VERSION_NUM) { - if (ret_ver != NULL) - log_err(_("%s needs standby to be PostgreSQL 9.0 or better\n"), - progname); + if (server_version_num > 0) + log_err(_("%s requires PostgreSQL %s or better\n"), + progname, + MIN_SUPPORTED_VERSION + ); terminate(ERR_BAD_CONFIG); }