diff --git a/sql/ddl_api.sql b/sql/ddl_api.sql index 18ab735b7..5b66cb714 100644 --- a/sql/ddl_api.sql +++ b/sql/ddl_api.sql @@ -164,7 +164,8 @@ CREATE OR REPLACE FUNCTION delete_data_node( node_name NAME, if_exists BOOLEAN = FALSE, force BOOLEAN = FALSE, - repartition BOOLEAN = TRUE + repartition BOOLEAN = TRUE, + drop_database BOOLEAN = FALSE ) RETURNS BOOLEAN AS '@MODULE_PATHNAME@', 'ts_data_node_delete' LANGUAGE C VOLATILE; -- Attach a data node to a distributed hypertable diff --git a/sql/updates/latest-dev.sql b/sql/updates/latest-dev.sql index 5991af61b..56060d9da 100644 --- a/sql/updates/latest-dev.sql +++ b/sql/updates/latest-dev.sql @@ -1,5 +1,5 @@ DROP FUNCTION IF EXISTS recompress_chunk; - +DROP FUNCTION IF EXISTS delete_data_node; -- Also see the comments for ContinuousAggsBucketFunction structure. CREATE TABLE IF NOT EXISTS _timescaledb_catalog.continuous_aggs_bucket_function( diff --git a/sql/updates/reverse-dev.sql b/sql/updates/reverse-dev.sql index d2482efca..cfd50731e 100644 --- a/sql/updates/reverse-dev.sql +++ b/sql/updates/reverse-dev.sql @@ -1,6 +1,7 @@ DROP PROCEDURE IF EXISTS recompress_chunk; DROP FUNCTION IF EXISTS _timescaledb_internal.chunk_status; +DROP FUNCTION IF EXISTS delete_data_node; DO $$ DECLARE diff --git a/tsl/src/data_node.c b/tsl/src/data_node.c index deccafa36..2f3f4272d 100644 --- a/tsl/src/data_node.c +++ b/tsl/src/data_node.c @@ -1320,6 +1320,132 @@ data_node_detach(PG_FUNCTION_ARGS) PG_RETURN_INT32(removed); } +/* + * Drop a data node's database. + * + * To drop the database on the data node, a connection must be made to another + * database since one cannot drop the database currently connected + * to. Therefore, we bypass the connection cache and use a "raw" connection to + * a standard database (e.g., template0 or postgres), similar to how + * bootstrapping does it. + * + * Note that no password is provided on the command line as is done when + * bootstrapping. Instead, it is assumed that the current user already has a + * method to authenticate with the remote data node (e.g., via a password + * file, certificate, or user mapping). This should normally be the case or + * otherwise the user wouldn't have been able to use the data node. + * + * Note that the user that deletes a data node also must be the database owner + * on the data node. The database will only be dropped if there are no other + * concurrent connections so all connections must be closed before being able + * to drop the database. + */ +static void +drop_data_node_database(const ForeignServer *server) +{ + ListCell *lc; + TSConnection *conn; + Oid userid = GetUserId(); + TSConnectionId connid = { + .server_id = server->serverid, + .user_id = userid, + }; + /* Make a copy of the node name since the server pointer will be + * updated */ + char *nodename = pstrdup(server->servername); + char *dbname = NULL; + char *err = NULL; + int i; + + /* Figure out the name of the database that should be dropped */ + foreach (lc, server->options) + { + DefElem *d = lfirst(lc); + + if (strcmp(d->defname, "dbname") == 0) + { + dbname = defGetString(d); + break; + } + } + + if (NULL == dbname) + { + /* This should not happen unless the configuration is messed up */ + ereport(ERROR, + (errcode(ERRCODE_TS_DATA_NODE_INVALID_CONFIG), + errmsg("could not drop the database on data node \"%s\"", nodename), + errdetail("The data node configuration lacks the \"dbname\" option."))); + pg_unreachable(); + return; + } + + /* Clear potentially cached connection to the data node for the current + * session so that it won't block dropping the database */ + remote_connection_cache_remove(connid); + + /* Cannot connect to the database that is being dropped, so try to connect + * to a "standard" bootstrap database that we expect to exist on the data + * node */ + for (i = 0; i < lengthof(bootstrap_databases); i++) + { + List *conn_options; + DefElem dbname_elem = { + .type = T_DefElem, + .defaction = DEFELEM_SET, + .defname = "dbname", + .arg = (Node *) makeString(pstrdup(bootstrap_databases[i])), + }; + AlterForeignServerStmt stmt = { + .type = T_AlterForeignServerStmt, + .servername = nodename, + .has_version = false, + .options = list_make1(&dbname_elem), + }; + + /* + * We assume that the user already has credentials configured to + * connect to the data node, e.g., via a user mapping, password file, + * or certificate. But in order to easily make use of those + * authentication methods, we need to establish the connection using + * the standard connection functions to pick up the foreign server + * options and associated user mapping (if such a mapping + * exists). However, the foreign server configuration references the + * database we are trying to drop, so we first need to update the + * foreign server definition to use the bootstrap database. */ + AlterForeignServer(&stmt); + + /* Make changes to foreign server database visible */ + CommandCounterIncrement(); + + /* Get the updated server definition */ + server = data_node_get_foreign_server(nodename, ACL_USAGE, true, false); + /* Open a connection to the bootstrap database using the new server options */ + conn_options = remote_connection_prepare_auth_options(server, userid); + conn = remote_connection_open_with_options_nothrow(nodename, conn_options, &err); + + if (NULL != conn) + break; + } + + if (NULL != conn) + { + /* Do not include FORCE or IF EXISTS options when dropping the + * database. Instead, we expect the database to exist, or the user + * has to rerun the command without drop_database=>true set. We + * don't force removal if there are other connections to the + * database out of caution. If the user wants to forcefully remove + * the database, they can do it manually. */ + remote_connection_cmdf_ok(conn, "DROP DATABASE %s", quote_identifier(dbname)); + remote_connection_close(conn); + } + else + ereport(ERROR, + (errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION), + errmsg("could not connect to data node \"%s\"", nodename), + err == NULL ? 0 : errdetail("%s", err))); +} + Datum data_node_delete(PG_FUNCTION_ARGS) { @@ -1327,6 +1453,7 @@ data_node_delete(PG_FUNCTION_ARGS) bool if_exists = PG_ARGISNULL(1) ? false : PG_GETARG_BOOL(1); bool force = PG_ARGISNULL(2) ? false : PG_GETARG_BOOL(2); bool repartition = PG_ARGISNULL(3) ? false : PG_GETARG_BOOL(3); + bool drop_database = PG_ARGISNULL(4) ? false : PG_GETARG_BOOL(4); List *hypertable_data_nodes = NIL; DropStmt stmt; ObjectAddress address; @@ -1353,6 +1480,12 @@ data_node_delete(PG_FUNCTION_ARGS) PG_RETURN_BOOL(false); } + if (drop_database) + { + TS_PREVENT_IN_TRANSACTION_BLOCK(true); + drop_data_node_database(server); + } + /* close any pending connections */ remote_connection_id_set(&cid, server->serverid, GetUserId()); remote_connection_cache_remove(cid); diff --git a/tsl/src/remote/connection.c b/tsl/src/remote/connection.c index 4820d6bac..13f009f76 100644 --- a/tsl/src/remote/connection.c +++ b/tsl/src/remote/connection.c @@ -1461,11 +1461,11 @@ options_contain(List *options, const char *key) } /* - * Add user info (username and optionally password) to the connection + * Add athentication info (username and optionally password) to the connection * options). */ -static List * -add_userinfo_to_server_options(ForeignServer *server, Oid user_id) +List * +remote_connection_prepare_auth_options(const ForeignServer *server, Oid user_id) { const UserMapping *um = get_user_mapping(user_id, server->serverid); List *options = list_copy(server->options); @@ -1544,7 +1544,7 @@ remote_connection_get_connstr(const char *node_name) int i; server = data_node_get_foreign_server(node_name, ACL_NO_CHECK, false, false); - connection_options = add_userinfo_to_server_options(server, GetUserId()); + connection_options = remote_connection_prepare_auth_options(server, GetUserId()); setup_full_connection_options(connection_options, &keywords, &values); /* Cycle through the options and create the connection string */ @@ -1574,7 +1574,7 @@ TSConnection * remote_connection_open_by_id(TSConnectionId id) { ForeignServer *server = GetForeignServer(id.server_id); - List *connection_options = add_userinfo_to_server_options(server, id.user_id); + List *connection_options = remote_connection_prepare_auth_options(server, id.user_id); return remote_connection_open_with_options(server->servername, connection_options, true); } @@ -1607,7 +1607,7 @@ remote_connection_open_nothrow(Oid server_id, Oid user_id, char **errmsg) return NULL; } - connection_options = add_userinfo_to_server_options(server, user_id); + connection_options = remote_connection_prepare_auth_options(server, user_id); conn = remote_connection_open_with_options_nothrow(server->servername, connection_options, errmsg); diff --git a/tsl/src/remote/connection.h b/tsl/src/remote/connection.h index cb89311e3..59894356e 100644 --- a/tsl/src/remote/connection.h +++ b/tsl/src/remote/connection.h @@ -66,6 +66,7 @@ extern TSConnection *remote_connection_open_with_options_nothrow(const char *nod 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); +extern List *remote_connection_prepare_auth_options(const ForeignServer *server, Oid user_id); extern bool remote_connection_set_autoclose(TSConnection *conn, bool autoclose); extern int remote_connection_xact_depth_get(const TSConnection *conn); extern int remote_connection_xact_depth_inc(TSConnection *conn); diff --git a/tsl/test/expected/data_node_bootstrap.out b/tsl/test/expected/data_node_bootstrap.out index b70a9a796..40f2d2772 100644 --- a/tsl/test/expected/data_node_bootstrap.out +++ b/tsl/test/expected/data_node_bootstrap.out @@ -121,13 +121,66 @@ SELECT * FROM add_data_node('bootstrap_test', host => 'localhost', bootstrap_test | localhost | 55432 | bootstrap_test | t | f | f (1 row) +\set ON_ERROR_STOP 0 +-- Dropping the database with delete_data_node should not work in a +-- transaction block since it is non-transactional. +BEGIN; +SELECT * FROM delete_data_node('bootstrap_test', drop_database => true); +ERROR: delete_data_node() cannot run inside a transaction block +ROLLBACK; +\set ON_ERROR_STOP 1 +-- Using the drop_database option when there are active connections to +-- the data node should fail. But any connections in the current +-- session should be cleared when dropping the database. To test that +-- the connection is cleared, first create a connection in the +-- connection cache by inserting some data +CREATE TABLE conditions (time timestamptz, device int, temp float); +SELECT create_distributed_hypertable('conditions', 'time', 'device'); +WARNING: only one data node was assigned to the hypertable +NOTICE: adding not-null constraint to column "time" + create_distributed_hypertable +------------------------------- + (1,public,conditions,t) +(1 row) + +INSERT INTO conditions VALUES ('2021-12-01 10:30', 1, 20.3); +DROP TABLE conditions; +-- Now drop the data node and it should clear the connection from the +-- cache first +SELECT * FROM delete_data_node('bootstrap_test', drop_database => true); + delete_data_node +------------------ + t +(1 row) + +\set ON_ERROR_STOP 0 +-- Dropping the database now should fail since it no longer exists +DROP DATABASE bootstrap_test; +ERROR: database "bootstrap_test" does not exist +\set ON_ERROR_STOP 1 +-- Adding the data node again should work +SELECT * FROM add_data_node('bootstrap_test', host => 'localhost', + database => 'bootstrap_test', bootstrap => true); + node_name | host | port | database | node_created | database_created | extension_created +----------------+-----------+-------+----------------+--------------+------------------+------------------- + bootstrap_test | localhost | 55432 | bootstrap_test | t | t | t +(1 row) + +-- Now drop the database manually before using the drop_database option +DROP DATABASE bootstrap_test; +\set ON_ERROR_STOP 0 +-- Expect an error since the database does not exist. +SELECT * FROM delete_data_node('bootstrap_test', drop_database => true); +ERROR: [bootstrap_test]: database "bootstrap_test" does not exist +\set ON_ERROR_STOP 1 +-- Delete it without the drop_database option set since the database +-- was manually deleted. SELECT * FROM delete_data_node('bootstrap_test'); delete_data_node ------------------ t (1 row) -DROP DATABASE bootstrap_test; ---------------------------------------------------------------------- -- Do a manual bootstrap of the data node and check that it can be -- added. diff --git a/tsl/test/sql/data_node_bootstrap.sql b/tsl/test/sql/data_node_bootstrap.sql index b1175eea3..42871b6de 100644 --- a/tsl/test/sql/data_node_bootstrap.sql +++ b/tsl/test/sql/data_node_bootstrap.sql @@ -79,8 +79,46 @@ DELETE FROM _timescaledb_catalog.metadata WHERE key = 'dist_uuid'; SELECT * FROM add_data_node('bootstrap_test', host => 'localhost', database => 'bootstrap_test', bootstrap => false); -SELECT * FROM delete_data_node('bootstrap_test'); +\set ON_ERROR_STOP 0 +-- Dropping the database with delete_data_node should not work in a +-- transaction block since it is non-transactional. +BEGIN; +SELECT * FROM delete_data_node('bootstrap_test', drop_database => true); +ROLLBACK; +\set ON_ERROR_STOP 1 + +-- Using the drop_database option when there are active connections to +-- the data node should fail. But any connections in the current +-- session should be cleared when dropping the database. To test that +-- the connection is cleared, first create a connection in the +-- connection cache by inserting some data +CREATE TABLE conditions (time timestamptz, device int, temp float); +SELECT create_distributed_hypertable('conditions', 'time', 'device'); +INSERT INTO conditions VALUES ('2021-12-01 10:30', 1, 20.3); +DROP TABLE conditions; + +-- Now drop the data node and it should clear the connection from the +-- cache first +SELECT * FROM delete_data_node('bootstrap_test', drop_database => true); + +\set ON_ERROR_STOP 0 +-- Dropping the database now should fail since it no longer exists DROP DATABASE bootstrap_test; +\set ON_ERROR_STOP 1 + +-- Adding the data node again should work +SELECT * FROM add_data_node('bootstrap_test', host => 'localhost', + database => 'bootstrap_test', bootstrap => true); +-- Now drop the database manually before using the drop_database option +DROP DATABASE bootstrap_test; +\set ON_ERROR_STOP 0 +-- Expect an error since the database does not exist. +SELECT * FROM delete_data_node('bootstrap_test', drop_database => true); +\set ON_ERROR_STOP 1 + +-- Delete it without the drop_database option set since the database +-- was manually deleted. +SELECT * FROM delete_data_node('bootstrap_test'); ---------------------------------------------------------------------- -- Do a manual bootstrap of the data node and check that it can be