From c224bc7994b0a40a3bffd306bb9af8425ec2bb97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Nordstr=C3=B6m?= Date: Tue, 22 Dec 2020 14:26:43 +0100 Subject: [PATCH] Always validate existing database and extension This change ensures the database and extension is validated whenever these objects aren't created, instead of only doing validation when `bootstrap=>false` is passed when adding a data node. This fixes a corner case where a data node could be added and removed several times, even though the data node's database was already marked as having been part of a multi-node setup. A new test checks that a data node cannot be re-added after deleting it on the access node, irrespective of whether one bootstraps the data node or not when it is added. --- tsl/src/data_node.c | 9 ++++++--- tsl/test/expected/data_node_bootstrap.out | 16 +++++++++++++--- tsl/test/expected/dist_util.out | 12 ++++++------ tsl/test/sql/data_node_bootstrap.sql | 11 +++++++++-- 4 files changed, 34 insertions(+), 14 deletions(-) diff --git a/tsl/src/data_node.c b/tsl/src/data_node.c index badf515cf..6544ba4d6 100644 --- a/tsl/src/data_node.c +++ b/tsl/src/data_node.c @@ -454,7 +454,7 @@ data_node_validate_as_data_node(TSConnection *conn) if (PQresultStatus(res) != PGRES_TUPLES_OK) ereport(ERROR, (errcode(ERRCODE_TS_DATA_NODE_INVALID_CONFIG), - (errmsg("%s is not valid as data node", remote_connection_node_name(conn)), + (errmsg("cannot add \"%s\" as a data node", remote_connection_node_name(conn)), errdetail("%s", PQresultErrorMessage(res))))); remote_result_close(res); @@ -762,13 +762,16 @@ data_node_add_internal(PG_FUNCTION_ARGS, bool set_distid) if (bootstrap) extension_created = data_node_bootstrap_extension(conn); - else + + if (!database_created) { data_node_validate_database(conn, &database); - data_node_validate_extension(conn); data_node_validate_as_data_node(conn); } + if (!extension_created) + data_node_validate_extension(conn); + /* After the node is verified or bootstrapped, we set the `dist_uuid` * using the same connection. We skip this if clustering checks are * disabled, which means that the `dist_uuid` is neither set nor diff --git a/tsl/test/expected/data_node_bootstrap.out b/tsl/test/expected/data_node_bootstrap.out index 02e6cc1d3..5f389c45f 100644 --- a/tsl/test/expected/data_node_bootstrap.out +++ b/tsl/test/expected/data_node_bootstrap.out @@ -47,7 +47,8 @@ SELECT PG_ENCODING_TO_CHAR(encoding) = :enc (1 row) \c :TEST_DBNAME :ROLE_CLUSTER_SUPERUSER; --- After delete database and extension should still be there +-- After delete_data_node, the database and extension should still +-- exist on the data node SELECT * FROM delete_data_node('bootstrap_test'); delete_data_node ------------------ @@ -70,6 +71,16 @@ AND e.extname = 'timescaledb'; (1 row) \c :TEST_DBNAME :ROLE_CLUSTER_SUPERUSER; +\set ON_ERROR_STOP 0 +-- Trying to add the data node again should fail, with or without +-- bootstrapping. +SELECT * FROM add_data_node('bootstrap_test', host => 'localhost', database => 'bootstrap_test', bootstrap=>false); +ERROR: cannot add "bootstrap_test" as a data node +SELECT * FROM add_data_node('bootstrap_test', host => 'localhost', database => 'bootstrap_test'); +NOTICE: database "bootstrap_test" already exists on data node, skipping +NOTICE: extension "timescaledb" already exists on data node, skipping +ERROR: cannot add "bootstrap_test" as a data node +\set ON_ERROR_STOP 0 DROP DATABASE bootstrap_test; ---------------------------------------------------------------------- -- Bootstrap the database and check that calling it without @@ -88,7 +99,6 @@ SELECT * FROM show_data_nodes(); bootstrap_test | localhost | 55432 | bootstrap_test (1 row) -\c :TEST_DBNAME :ROLE_CLUSTER_SUPERUSER; SELECT * FROM delete_data_node('bootstrap_test'); delete_data_node ------------------ @@ -327,7 +337,7 @@ SELECT extname FROM pg_extension WHERE extname = 'timescaledb'; \c :TEST_DBNAME :ROLE_CLUSTER_SUPERUSER; SELECT * FROM add_data_node('bootstrap_test', host => 'localhost', database => 'bootstrap_test', bootstrap => false); -ERROR: database does not have TimescaleDB extension loaded +ERROR: cannot add "bootstrap_test" as a data node \set ON_ERROR_STOP 1 DROP DATABASE bootstrap_test; ----------------------------------------------------------------------- diff --git a/tsl/test/expected/dist_util.out b/tsl/test/expected/dist_util.out index c45be0d3a..729c7b343 100644 --- a/tsl/test/expected/dist_util.out +++ b/tsl/test/expected/dist_util.out @@ -131,18 +131,18 @@ SET client_min_messages TO NOTICE; SELECT * FROM add_data_node('invalid_data_node', host => 'localhost', database => 'frontend_2', bootstrap => true); NOTICE: database "frontend_2" already exists on data node, skipping NOTICE: extension "timescaledb" already exists on data node, skipping -ERROR: [invalid_data_node]: database is already a member of a distributed database +ERROR: cannot add "invalid_data_node" as a data node SELECT * FROM add_data_node('invalid_data_node', host => 'localhost', database => 'frontend_2', bootstrap => false); -ERROR: invalid_data_node is not valid as data node +ERROR: cannot add "invalid_data_node" as a data node ---------------------------------------------------------------- -- Adding backend from a different group as a backend should fail \c frontend_1 :ROLE_CLUSTER_SUPERUSER SELECT * FROM add_data_node('invalid_data_node', host => 'localhost', database => 'backend_2_1', bootstrap => true); NOTICE: database "backend_2_1" already exists on data node, skipping NOTICE: extension "timescaledb" already exists on data node, skipping -ERROR: [invalid_data_node]: database is already a member of a distributed database +ERROR: cannot add "invalid_data_node" as a data node SELECT * FROM add_data_node('invalid_data_node', host => 'localhost', database => 'backend_2_1', bootstrap => false); -ERROR: invalid_data_node is not valid as data node +ERROR: cannot add "invalid_data_node" as a data node ---------------------------------------------------------------- -- Adding a valid backend target but to an existing backend should fail \c backend_1_1 :ROLE_CLUSTER_SUPERUSER @@ -156,9 +156,9 @@ ERROR: unable to assign data nodes from an existing distributed database SELECT * FROM add_data_node('invalid_data_node', host => 'localhost', database => 'frontend_1', bootstrap => true); NOTICE: database "frontend_1" already exists on data node, skipping NOTICE: extension "timescaledb" already exists on data node, skipping -ERROR: [invalid_data_node]: database is already a member of a distributed database +ERROR: cannot add "invalid_data_node" as a data node SELECT * FROM add_data_node('invalid_data_node', host => 'localhost', database => 'frontend_1', bootstrap => false); -ERROR: invalid_data_node is not valid as data node +ERROR: cannot add "invalid_data_node" as a data node \set ON_ERROR_STOP 1 ---------------------------------------------------------------- -- Test that a data node can be moved to a different frontend if it is diff --git a/tsl/test/sql/data_node_bootstrap.sql b/tsl/test/sql/data_node_bootstrap.sql index 3c0ad0e14..487ac7f8f 100644 --- a/tsl/test/sql/data_node_bootstrap.sql +++ b/tsl/test/sql/data_node_bootstrap.sql @@ -39,7 +39,8 @@ SELECT PG_ENCODING_TO_CHAR(encoding) = :enc WHERE datname = current_database(); \c :TEST_DBNAME :ROLE_CLUSTER_SUPERUSER; --- After delete database and extension should still be there +-- After delete_data_node, the database and extension should still +-- exist on the data node SELECT * FROM delete_data_node('bootstrap_test'); SELECT * FROM show_data_nodes(); @@ -51,6 +52,13 @@ WHERE e.extnamespace = n.oid AND e.extname = 'timescaledb'; \c :TEST_DBNAME :ROLE_CLUSTER_SUPERUSER; +\set ON_ERROR_STOP 0 +-- Trying to add the data node again should fail, with or without +-- bootstrapping. +SELECT * FROM add_data_node('bootstrap_test', host => 'localhost', database => 'bootstrap_test', bootstrap=>false); +SELECT * FROM add_data_node('bootstrap_test', host => 'localhost', database => 'bootstrap_test'); +\set ON_ERROR_STOP 0 + DROP DATABASE bootstrap_test; ---------------------------------------------------------------------- @@ -61,7 +69,6 @@ SELECT * FROM add_data_node('bootstrap_test', host => 'localhost', database => 'bootstrap_test', bootstrap => true); SELECT * FROM show_data_nodes(); -\c :TEST_DBNAME :ROLE_CLUSTER_SUPERUSER; SELECT * FROM delete_data_node('bootstrap_test'); \c bootstrap_test :ROLE_CLUSTER_SUPERUSER;