From eede24bcf2b2c18bf76f53ce71f154bf547ed854 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Nordstr=C3=B6m?= Date: Thu, 17 Dec 2020 09:51:11 +0100 Subject: [PATCH] Add error detail when adding a data node fails When `add_data_node` fails, it often gives an opaque error that it couldn't connect to the data node. This change adds the libpq connection error as a detailed message in the error. --- tsl/src/data_node.c | 22 +++++++++++------- tsl/src/remote/connection.c | 45 ++++++++++++++++++++++++++++--------- tsl/src/remote/connection.h | 3 ++- 3 files changed, 51 insertions(+), 19 deletions(-) diff --git a/tsl/src/data_node.c b/tsl/src/data_node.c index 1465f70f3..f9c7126d6 100644 --- a/tsl/src/data_node.c +++ b/tsl/src/data_node.c @@ -552,16 +552,27 @@ connect_for_bootstrapping(const char *node_name, const char *const host, int32 p const char *username, const char *password) { TSConnection *conn = NULL; + char *err = NULL; int i; + for (i = 0; i < lengthof(bootstrap_databases); i++) { List *node_options = create_data_node_options(host, port, bootstrap_databases[i], username, password); - conn = remote_connection_open_with_options_nothrow(node_name, node_options); + conn = remote_connection_open_with_options_nothrow(node_name, node_options, &err); + if (conn) return conn; } - return conn; + + ereport(ERROR, + (errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION), + errmsg("could not connect to \"%s\"", node_name), + err == NULL ? 0 : errdetail("%s", err))); + + pg_unreachable(); + + return NULL; } /* @@ -726,12 +737,7 @@ data_node_add_internal(PG_FUNCTION_ARGS, bool set_distid) { TSConnection *conn = connect_for_bootstrapping(node_name, host, port, username, password); - - if (NULL == conn) - ereport(ERROR, - (errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION), - errmsg("could not connect to \"%s\"", node_name))); - + Assert(NULL != conn); data_node_validate_extension_availability(conn); database_created = data_node_bootstrap_database(conn, &database); remote_connection_close(conn); diff --git a/tsl/src/remote/connection.c b/tsl/src/remote/connection.c index 4b29c088e..5f07d92a7 100644 --- a/tsl/src/remote/connection.c +++ b/tsl/src/remote/connection.c @@ -1132,6 +1132,23 @@ set_ssl_options(const char *user_name, const char **keywords, const char **value *option_start = option_pos; } +/* + * Finish the connection and, optionally, save the connection error. + */ +static void +finish_connection(PGconn *conn, char **errmsg) +{ + if (NULL != errmsg) + { + if (NULL == conn) + *errmsg = "invalid connection"; + else + *errmsg = pchomp(PQerrorMessage(conn)); + } + + PQfinish(conn); +} + /* * This will only open a connection to a specific node, but not do anything * else. In particular, it will not perform any validation nor configure the @@ -1140,7 +1157,8 @@ set_ssl_options(const char *user_name, const char **keywords, const char **value * function. */ TSConnection * -remote_connection_open_with_options_nothrow(const char *node_name, List *connection_options) +remote_connection_open_with_options_nothrow(const char *node_name, List *connection_options, + char **errmsg) { PGconn *volatile pg_conn = NULL; const char *user_name = NULL; @@ -1150,6 +1168,9 @@ remote_connection_open_with_options_nothrow(const char *node_name, List *connect int option_count; int option_pos; + if (NULL != errmsg) + *errmsg = NULL; + /* * Construct connection params from generic options of ForeignServer * and user. (Some of them might not be libpq options, in @@ -1204,7 +1225,7 @@ remote_connection_open_with_options_nothrow(const char *node_name, List *connect if (PQstatus(pg_conn) == CONNECTION_BAD) { - PQfinish(pg_conn); + finish_connection(pg_conn, errmsg); pg_conn = NULL; } @@ -1221,7 +1242,7 @@ remote_connection_open_with_options_nothrow(const char *node_name, List *connect case PGRES_POLLING_FAILED: /* connection attempt failed */ stop_waiting = true; - PQfinish(pg_conn); + finish_connection(pg_conn, errmsg); pg_conn = NULL; break; case PGRES_POLLING_OK: @@ -1256,7 +1277,7 @@ remote_connection_open_with_options_nothrow(const char *node_name, List *connect PG_CATCH(); { if (NULL != pg_conn) - PQfinish(pg_conn); + finish_connection(pg_conn, errmsg); PG_RE_THROW(); } @@ -1271,14 +1292,14 @@ remote_connection_open_with_options_nothrow(const char *node_name, List *connect if (PQstatus(pg_conn) != CONNECTION_OK) { - PQfinish(pg_conn); + finish_connection(pg_conn, errmsg); return NULL; } ts_conn = remote_connection_create(pg_conn, false, node_name); if (NULL == ts_conn) - PQfinish(pg_conn); + finish_connection(pg_conn, errmsg); return ts_conn; } @@ -1297,12 +1318,15 @@ TSConnection * remote_connection_open_with_options(const char *node_name, List *connection_options, bool set_dist_id) { - TSConnection *conn = remote_connection_open_with_options_nothrow(node_name, connection_options); + char *err = NULL; + TSConnection *conn = + remote_connection_open_with_options_nothrow(node_name, connection_options, &err); if (NULL == conn) ereport(ERROR, (errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION), - errmsg("could not connect to \"%s\"", node_name))); + errmsg("could not connect to \"%s\"", node_name), + err == NULL ? 0 : errdetail_internal("%s", err))); /* * Use PG_TRY block to ensure closing connection on error. @@ -1477,11 +1501,12 @@ remote_connection_open_nothrow(Oid server_id, Oid user_id, char **errmsg) } connection_options = add_userinfo_to_server_options(server, user_id); - conn = remote_connection_open_with_options_nothrow(server->servername, connection_options); + conn = + remote_connection_open_with_options_nothrow(server->servername, connection_options, errmsg); if (NULL == conn) { - if (NULL != errmsg) + if (NULL != errmsg && NULL == *errmsg) *errmsg = "internal connection error"; return NULL; } diff --git a/tsl/src/remote/connection.h b/tsl/src/remote/connection.h index 557ffa23d..12961566c 100644 --- a/tsl/src/remote/connection.h +++ b/tsl/src/remote/connection.h @@ -39,7 +39,8 @@ extern TSConnection *remote_connection_open_with_options(const char *node_name, List *connection_options, bool set_dist_id); extern TSConnection *remote_connection_open_with_options_nothrow(const char *node_name, - List *connection_options); + List *connection_options, + char **errmsg); extern TSConnection *remote_connection_open_by_id(TSConnectionId id); extern TSConnection *remote_connection_open(Oid server_id, Oid user_id); extern TSConnection *remote_connection_open_nothrow(Oid server_id, Oid user_id, char **errmsg);