From b0a2ee22599e8d956ad18a40f0830babedef1458 Mon Sep 17 00:00:00 2001 From: Ian Barwick Date: Tue, 26 Jun 2018 10:44:31 +0900 Subject: [PATCH] get_all_node_records(): display any error encountered and return success status In many cases we'll want to bail out with an error if the node list can't be retrieved for any reason. This saves some repetitive coding. --- dbutils.c | 23 ++++++++++++----------- dbutils.h | 2 +- repmgr-action-bdr.c | 4 ++-- repmgr-action-cluster.c | 14 ++++++++++++-- repmgr-action-witness.c | 8 +++++++- repmgrd-bdr.c | 8 +++++++- 6 files changed, 41 insertions(+), 18 deletions(-) diff --git a/dbutils.c b/dbutils.c index 7139e0a0..fbe12281 100644 --- a/dbutils.c +++ b/dbutils.c @@ -2098,12 +2098,12 @@ _populate_node_records(PGresult *res, NodeInfoList *node_list) } -void +bool get_all_node_records(PGconn *conn, NodeInfoList *node_list) { PQExpBufferData query; PGresult *res = NULL; - + bool success = true; initPQExpBuffer(&query); appendPQExpBuffer(&query, @@ -2115,21 +2115,22 @@ get_all_node_records(PGconn *conn, NodeInfoList *node_list) res = PQexec(conn, query.data); - if (PQresultStatus(res) != PGRES_TUPLES_OK) - { - log_db_error(conn, query.data, _("get_all_node_records(): unable to execute query")); - } - - termPQExpBuffer(&query); - /* this will return an empty list if there was an error executing the query */ _populate_node_records(res, node_list); - PQclear(res); + if (PQresultStatus(res) != PGRES_TUPLES_OK) + { + log_db_error(conn, query.data, _("get_all_node_records(): unable to execute query")); + success = false; + } - return; + PQclear(res); + termPQExpBuffer(&query); + + return success; } + void get_downstream_node_records(PGconn *conn, int node_id, NodeInfoList *node_list) { diff --git a/dbutils.h b/dbutils.h index 0df3d7fa..8a56b963 100644 --- a/dbutils.h +++ b/dbutils.h @@ -421,7 +421,7 @@ t_node_info *get_node_record_pointer(PGconn *conn, int node_id); bool get_local_node_record(PGconn *conn, int node_id, t_node_info *node_info); bool get_primary_node_record(PGconn *conn, t_node_info *node_info); -void get_all_node_records(PGconn *conn, NodeInfoList *node_list); +bool get_all_node_records(PGconn *conn, NodeInfoList *node_list); void get_downstream_node_records(PGconn *conn, int node_id, NodeInfoList *nodes); void get_active_sibling_node_records(PGconn *conn, int node_id, int upstream_node_id, NodeInfoList *node_list); void get_node_records_by_priority(PGconn *conn, NodeInfoList *node_list); diff --git a/repmgr-action-bdr.c b/repmgr-action-bdr.c index 702a8a19..be6cc4a8 100644 --- a/repmgr-action-bdr.c +++ b/repmgr-action-bdr.c @@ -191,7 +191,7 @@ do_bdr_register(void) { NodeInfoList local_node_records = T_NODE_INFO_LIST_INITIALIZER; - get_all_node_records(conn, &local_node_records); + (void) get_all_node_records(conn, &local_node_records); if (local_node_records.node_count == 0) { @@ -239,7 +239,7 @@ do_bdr_register(void) continue; } - get_all_node_records(bdr_node_conn, &existing_nodes); + (void) get_all_node_records(bdr_node_conn, &existing_nodes); for (cell = existing_nodes.head; cell; cell = cell->next) { diff --git a/repmgr-action-cluster.c b/repmgr-action-cluster.c index 83083e6f..3a4aed8f 100644 --- a/repmgr-action-cluster.c +++ b/repmgr-action-cluster.c @@ -913,7 +913,12 @@ build_cluster_matrix(t_node_matrix_rec ***matrix_rec_dest, int *name_length) local_node_id = runtime_options.node_id; } - get_all_node_records(conn, &nodes); + if (get_all_node_records(conn, &nodes) == false) + { + /* get_all_node_records() will display the error */ + PQfinish(conn); + exit(ERR_BAD_CONFIG); + } PQfinish(conn); conn = NULL; @@ -1118,7 +1123,12 @@ build_cluster_crosscheck(t_node_status_cube ***dest_cube, int *name_length) else conn = establish_db_connection_by_params(&source_conninfo, true); - get_all_node_records(conn, &nodes); + if (get_all_node_records(conn, &nodes) == false) + { + /* get_all_node_records() will display the error */ + PQfinish(conn); + exit(ERR_BAD_CONFIG); + } PQfinish(conn); conn = NULL; diff --git a/repmgr-action-witness.c b/repmgr-action-witness.c index 9def498f..a7714c20 100644 --- a/repmgr-action-witness.c +++ b/repmgr-action-witness.c @@ -218,7 +218,13 @@ do_witness_register(void) * if repmgr.nodes contains entries, delete if -F/--force provided, * otherwise exit with error */ - get_all_node_records(witness_conn, &nodes); + if (get_all_node_records(witness_conn, &nodes) == false) + { + /* get_all_node_records() will display the error */ + PQfinish(witness_conn); + PQfinish(primary_conn); + exit(ERR_BAD_CONFIG); + } log_verbose(LOG_DEBUG, "%i node records found", nodes.node_count); diff --git a/repmgrd-bdr.c b/repmgrd-bdr.c index 40732756..2e5631db 100644 --- a/repmgrd-bdr.c +++ b/repmgrd-bdr.c @@ -150,7 +150,13 @@ monitor_bdr(void) * retrieve list of all nodes - we'll need these if the DB connection goes * away */ - get_all_node_records(local_conn, &nodes); + if (get_all_node_records(local_conn, &nodes) == false) + { + /* get_all_node_records() will display the error */ + PQfinish(local_conn); + exit(ERR_BAD_CONFIG); + } + /* we're expecting all (both) nodes to be up */ for (cell = nodes.head; cell; cell = cell->next)