Skip to content

Commit

Permalink
Add error detail when adding a data node fails
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
erimatnor committed Dec 18, 2020
1 parent 15e1edb commit eede24b
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 19 deletions.
22 changes: 14 additions & 8 deletions tsl/src/data_node.c
Expand Up @@ -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;
}

/*
Expand Down Expand Up @@ -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);
Expand Down
45 changes: 35 additions & 10 deletions tsl/src/remote/connection.c
Expand Up @@ -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
Expand All @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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;
}

Expand All @@ -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:
Expand Down Expand Up @@ -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();
}
Expand All @@ -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;
}
Expand All @@ -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.
Expand Down Expand Up @@ -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;
}
Expand Down
3 changes: 2 additions & 1 deletion tsl/src/remote/connection.h
Expand Up @@ -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);
Expand Down

0 comments on commit eede24b

Please sign in to comment.