From ec554e56948bd59b8793627016990666c2221baf Mon Sep 17 00:00:00 2001 From: Ian Barwick Date: Mon, 17 Jul 2017 11:10:37 +0900 Subject: [PATCH] Improve connection handling Set "connect_timeout" and "fallback_application_name" if not present. --- dbutils.c | 87 +++++++++++++++++++++++++++++++++-------- dbutils.h | 4 +- repmgr-action-standby.c | 13 ++---- repmgr-client.c | 2 +- 4 files changed, 77 insertions(+), 29 deletions(-) diff --git a/dbutils.c b/dbutils.c index 85947d8b..0d73ae9b 100644 --- a/dbutils.c +++ b/dbutils.c @@ -100,12 +100,27 @@ static PGconn * _establish_db_connection(const char *conninfo, const bool exit_on_error, const bool log_notice, const bool verbose_only) { PGconn *conn = NULL; - char connection_string[MAXLEN]; + char *connection_string = NULL; + char *errmsg = NULL; - strncpy(connection_string, conninfo, MAXLEN); + t_conninfo_param_list conninfo_params; + bool parse_success; - /* TODO: only set if not already present */ - strcat(connection_string, " fallback_application_name='repmgr'"); + initialize_conninfo_params(&conninfo_params, false); + + parse_success = parse_conninfo_string(conninfo, &conninfo_params, errmsg, false); + + if (parse_success == false) + { + log_error(_("unable to pass provided conninfo string:\n %s"), errmsg); + return NULL; + } + + /* set some default values if not explicitly provided */ + param_set_ine(&conninfo_params, "connect_timeout", "2"); + param_set_ine(&conninfo_params, "fallback_application_name", "repmgr"); + + connection_string = param_list_to_string(&conninfo_params); log_debug(_("connecting to: '%s'"), connection_string); @@ -123,12 +138,12 @@ _establish_db_connection(const char *conninfo, const bool exit_on_error, const b { if (log_notice) { - log_notice(_("connection to database failed: %s"), + log_notice(_("connection to database failed:\n %s"), PQerrorMessage(conn)); } else { - log_error(_("connection to database failed: %s"), + log_error(_("connection to database failed:\n %s"), PQerrorMessage(conn)); } log_detail(_("attempted to connect using:\n %s"), @@ -157,6 +172,8 @@ _establish_db_connection(const char *conninfo, const bool exit_on_error, const b } } + pfree(connection_string); + return conn; } @@ -222,9 +239,7 @@ establish_db_connection_as_user(const char *conninfo, param_set(&conninfo_params, "user", user); - conn = establish_db_connection_by_params((const char**)conninfo_params.keywords, - (const char**)conninfo_params.values, - false); + conn = establish_db_connection_by_params(&conninfo_params, false); return conn; } @@ -233,13 +248,17 @@ establish_db_connection_as_user(const char *conninfo, PGconn * -establish_db_connection_by_params(const char *keywords[], const char *values[], +establish_db_connection_by_params(t_conninfo_param_list *param_list, const bool exit_on_error) { PGconn *conn; + /* set some default values if not explicitly provided */ + param_set_ine(param_list, "connect_timeout", "2"); + param_set_ine(param_list, "fallback_application_name", "repmgr"); + /* Connect to the database using the provided parameters */ - conn = PQconnectdbParams(keywords, values, true); + conn = PQconnectdbParams((const char**)param_list->keywords, (const char**)param_list->values, true); /* Check to see that the backend connection was successfully made */ if ((PQstatus(conn) != CONNECTION_OK)) @@ -254,7 +273,7 @@ establish_db_connection_by_params(const char *keywords[], const char *values[], } else { - bool replication_connection = false; + bool is_replication_connection = false; int i; /* @@ -262,13 +281,13 @@ establish_db_connection_by_params(const char *keywords[], const char *values[], * use (provided this is not a replication connection) */ - for (i = 0; keywords[i]; i++) + for (i = 0; param_list->keywords[i]; i++) { - if (strcmp(keywords[i], "replication") == 0) - replication_connection = true; + if (strcmp(param_list->keywords[i], "replication") == 0) + is_replication_connection = true; } - if (replication_connection == false && set_config(conn, "synchronous_commit", "local") == false) + if (is_replication_connection == false && set_config(conn, "synchronous_commit", "local") == false) { if (exit_on_error) { @@ -446,6 +465,42 @@ param_set(t_conninfo_param_list *param_list, const char *param, const char *valu } +/* + * Like param_set(), but will only set the parameter if it doesn't exist + */ +void +param_set_ine(t_conninfo_param_list *param_list, 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, do nothing + */ + for (c = 0; c < param_list->size && param_list->keywords[c] != NULL; c++) + { + if (strcmp(param_list->keywords[c], param) == 0) + { + /* parameter exists, do nothing */ + return; + } + } + + /* + * Parameter not in array - add it and its associated value + */ + if (c < param_list->size) + { + int param_len = strlen(param) + 1; + param_list->keywords[c] = pg_malloc0(param_len); + param_list->values[c] = pg_malloc0(value_len); + + strncpy(param_list->keywords[c], param, param_len); + strncpy(param_list->values[c], value, value_len); + } +} + + char * param_get(t_conninfo_param_list *param_list, const char *param) { diff --git a/dbutils.h b/dbutils.h index 5cd640e9..11ed36c3 100644 --- a/dbutils.h +++ b/dbutils.h @@ -209,8 +209,7 @@ PGconn *establish_db_connection_as_user(const char *conninfo, const char *user, const bool exit_on_error); -PGconn *establish_db_connection_by_params(const char *keywords[], - const char *values[], +PGconn *establish_db_connection_by_params(t_conninfo_param_list *param_list, const bool exit_on_error); PGconn *establish_primary_db_connection(PGconn *conn, const bool exit_on_error); @@ -227,6 +226,7 @@ void initialize_conninfo_params(t_conninfo_param_list *param_list, bool set_def void copy_conninfo_params(t_conninfo_param_list *dest_list, t_conninfo_param_list *source_list); void conn_to_param_list(PGconn *conn, t_conninfo_param_list *param_list); void param_set(t_conninfo_param_list *param_list, const char *param, const char *value); +void param_set_ine(t_conninfo_param_list *param_list, const char *param, const char *value); char *param_get(t_conninfo_param_list *param_list, const char *param); bool parse_conninfo_string(const char *conninfo_str, t_conninfo_param_list *param_list, char *errmsg, bool ignore_application_name); char *param_list_to_string(t_conninfo_param_list *param_list); diff --git a/repmgr-action-standby.c b/repmgr-action-standby.c index f55df9ca..3e6ba288 100644 --- a/repmgr-action-standby.c +++ b/repmgr-action-standby.c @@ -604,9 +604,7 @@ do_standby_register(void) /* User is forcing a registration and must have supplied primary connection info */ else { - primary_conn = establish_db_connection_by_params((const char**)source_conninfo.keywords, - (const char**)source_conninfo.values, - false); + primary_conn = establish_db_connection_by_params(&source_conninfo, false); } @@ -1325,10 +1323,7 @@ do_standby_follow(void) */ else { - primary_conn = establish_db_connection_by_params( - (const char**)source_conninfo.keywords, - (const char**)source_conninfo.values, - true); + primary_conn = establish_db_connection_by_params(&source_conninfo, true); primary_id = get_primary_node_id(primary_conn); strncpy(data_dir, runtime_options.data_dir, MAXPGPATH); @@ -1601,9 +1596,7 @@ check_source_server() /* Attempt to connect to the upstream server to verify its configuration */ log_info(_("connecting to upstream node")); - source_conn = establish_db_connection_by_params((const char**)source_conninfo.keywords, - (const char**)source_conninfo.values, - false); + source_conn = establish_db_connection_by_params(&source_conninfo, false); /* * Unless in barman mode, exit with an error; diff --git a/repmgr-client.c b/repmgr-client.c index f3cf6ac3..32e85690 100644 --- a/repmgr-client.c +++ b/repmgr-client.c @@ -1964,7 +1964,7 @@ check_upstream_config(PGconn *conn, int server_version_num, bool exit_on_error) { PGconn *replication_conn; - replication_conn = establish_db_connection_by_params((const char**)repl_conninfo.keywords, (const char**)repl_conninfo.values, false); + replication_conn = establish_db_connection_by_params(&repl_conninfo, false); if (PQstatus(replication_conn) == CONNECTION_OK) {