Skip to content

Commit

Permalink
Verify configuration before adding data node
Browse files Browse the repository at this point in the history
This change will call a function on a remote database to validate
its configuration before following through with an add_data_node
call.  Right now the check will ensure that the data is able to use
prepared transactions, but further checks can be easily added in
the future.

Since this uses the timescaledb extension to validate the remote
database, it runs at the end of bootstrapping.  We may want to
consider adding code to undo our bootstrapping changes if this check
fails.
  • Loading branch information
Brian Rowe authored and erimatnor committed May 27, 2020
1 parent f18f3d6 commit 31953f0
Show file tree
Hide file tree
Showing 8 changed files with 42 additions and 2 deletions.
3 changes: 3 additions & 0 deletions sql/dist_internal.sql
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,6 @@ RETURNS TABLE (id int,
toast_bytes bigint,
total_bytes bigint)
AS '@MODULE_PATHNAME@', 'ts_dist_remote_hypertable_info' LANGUAGE C VOLATILE STRICT;

CREATE OR REPLACE FUNCTION _timescaledb_internal.validate_as_data_node() RETURNS BOOL
AS '@MODULE_PATHNAME@', 'ts_dist_validate_as_data_node' LANGUAGE C VOLATILE STRICT;
8 changes: 8 additions & 0 deletions src/cross_module_fn.c
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ TS_FUNCTION_INFO_V1(ts_dist_set_id);
TS_FUNCTION_INFO_V1(ts_dist_remove_id);
TS_FUNCTION_INFO_V1(ts_dist_set_peer_id);
TS_FUNCTION_INFO_V1(ts_dist_remote_hypertable_info);
TS_FUNCTION_INFO_V1(ts_dist_validate_as_data_node);

Datum
ts_add_drop_chunks_policy(PG_FUNCTION_ARGS)
Expand Down Expand Up @@ -226,6 +227,12 @@ ts_dist_remote_hypertable_info(PG_FUNCTION_ARGS)
PG_RETURN_DATUM(ts_cm_functions->remote_hypertable_info(fcinfo));
}

Datum
ts_dist_validate_as_data_node(PG_FUNCTION_ARGS)
{
PG_RETURN_BOOL(ts_cm_functions->validate_as_data_node(fcinfo));
}

/*
* stub function to trigger aggregate util functions.
*/
Expand Down Expand Up @@ -631,6 +638,7 @@ TSDLLEXPORT CrossModuleFunctions ts_cm_functions_default = {
.is_frontend_session = error_no_default_fn_bool_void_community,
.remove_from_distributed_db = error_no_default_fn_bool_void_community,
.remote_hypertable_info = error_no_default_fn_pg_community,
.validate_as_data_node = error_no_default_fn_bool_void_community,
};

TSDLLEXPORT CrossModuleFunctions *ts_cm_functions = &ts_cm_functions_default;
1 change: 1 addition & 0 deletions src/cross_module_fn.h
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ typedef struct CrossModuleFunctions
bool (*is_frontend_session)(void);
bool (*remove_from_distributed_db)(void);
PGFunction remote_hypertable_info;
bool (*validate_as_data_node)();
} CrossModuleFunctions;

extern TSDLLEXPORT CrossModuleFunctions *ts_cm_functions;
Expand Down
1 change: 1 addition & 0 deletions src/errors.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
#define ERRCODE_TS_DATA_NODE_ASSIGNMENT_ALREADY_EXISTS MAKE_SQLSTATE('T', 'S', '1', '7', '1')
#define ERRCODE_TS_DATA_NODE_ALREADY_ATTACHED MAKE_SQLSTATE('T', 'S', '1', '7', '2')
#define ERRCODE_TS_DATA_NODE_NOT_ATTACHED MAKE_SQLSTATE('T', 'S', '1', '7', '3')
#define ERRCODE_TS_DATA_NODE_INVALID_CONFIG MAKE_SQLSTATE('T', 'S', '1', '7', '4')

/*
--IO500 - GROUP: internal error
Expand Down
17 changes: 17 additions & 0 deletions tsl/src/data_node.c
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@ data_node_bootstrap_extension(const char *node_name, const char *host, int32 por
const char *schema_name_quoted = quote_identifier(schema_name);
Oid schema_oid = get_namespace_oid(schema_name, true);
bool extension_exists = false;
bool data_node_valid = false;

request = psprintf("SELECT 1 FROM pg_extension WHERE extname = %s",
quote_literal_cstr(EXTENSION_NAME));
Expand All @@ -395,6 +396,8 @@ data_node_bootstrap_extension(const char *node_name, const char *host, int32 por
if (PQntuples(res) > 0)
extension_exists = true;

remote_connection_result_close(res);

if (!extension_exists)
{
if (schema_oid != PG_PUBLIC_NAMESPACE)
Expand All @@ -414,6 +417,20 @@ data_node_bootstrap_extension(const char *node_name, const char *host, int32 por

created = true;
}

request = "SELECT _timescaledb_internal.validate_as_data_node()";
res = remote_connection_query_any_result(conn, request);
data_node_valid =
PQntuples(res) == 1 && PQnfields(res) == 1 && PQgetvalue(res, 0, 0)[0] == 't';
remote_connection_result_close(res);

if (!data_node_valid)
ereport(ERROR,
(errcode(ERRCODE_TS_DATA_NODE_INVALID_CONFIG),
(errmsg("postgres instance for data node is not properly configured for "
"TimescaleDB distributed operations"),
errhint("max_prepared_transactions should be >= max_connections, note that "
"changing these setting requires a postgres restart"))));
}
PG_CATCH();
{
Expand Down
8 changes: 8 additions & 0 deletions tsl/src/dist_util.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
#include <utils/fmgrprotos.h>
#include "remote/dist_commands.h"
#include "funcapi.h"
#include <access/twophase.h>
#include <miscadmin.h>

/*
* When added to a distributed database, this key in the metadata table will be set to match the
Expand Down Expand Up @@ -205,3 +207,9 @@ dist_util_remote_hypertable_info(PG_FUNCTION_ARGS)
ts_dist_cmd_close_response(funcctx->user_fctx);
SRF_RETURN_DONE(funcctx);
}

bool
validate_data_node_settings(void)
{
return MaxConnections > 0 && max_prepared_xacts >= MaxConnections;
}
4 changes: 2 additions & 2 deletions tsl/src/dist_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ const char *dist_util_internal_key_name(void);
void dist_util_set_peer_id(Datum dist_id);
bool dist_util_is_frontend_session(void);

#if !PG96
Datum dist_util_remote_hypertable_info(PG_FUNCTION_ARGS);
#endif

bool validate_data_node_settings(void);

#endif /* TIMESCALEDB_TSL_CHUNK_API_H */
2 changes: 2 additions & 0 deletions tsl/src/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,7 @@ CrossModuleFunctions tsl_cm_functions = {
.is_frontend_session = NULL,
.remove_from_distributed_db = NULL,
.remote_hypertable_info = error_not_supported_default_fn,
.validate_as_data_node = NULL,
#else
.add_data_node = data_node_add,
.delete_data_node = data_node_delete,
Expand Down Expand Up @@ -282,6 +283,7 @@ CrossModuleFunctions tsl_cm_functions = {
.is_frontend_session = dist_util_is_frontend_session,
.remove_from_distributed_db = dist_util_remove_from_db,
.remote_hypertable_info = dist_util_remote_hypertable_info,
.validate_as_data_node = validate_data_node_settings,
#endif
.cache_syscache_invalidate = cache_syscache_invalidate,
};
Expand Down

0 comments on commit 31953f0

Please sign in to comment.