From 8bcae3f7ce8b31e1dadfef7fb4d13a5017e0e1a1 Mon Sep 17 00:00:00 2001 From: Dmitry Simonenko Date: Thu, 8 Oct 2020 15:26:49 +0300 Subject: [PATCH] Add if_attached argument to detach_data_node() This change makes detach_data_node() function consistent with other data node management functions by adding missing if_attach argument. The function will not show an error in case if data node is not attached and if_attached is set to true. Issue: #2506 --- sql/ddl_api.sql | 1 + sql/updates/latest-dev.sql | 2 ++ tsl/src/data_node.c | 39 +++++++++++++++++++++++---------- tsl/test/expected/data_node.out | 12 +++++++++- tsl/test/sql/data_node.sql | 6 ++++- 5 files changed, 46 insertions(+), 14 deletions(-) diff --git a/sql/ddl_api.sql b/sql/ddl_api.sql index 21fa11d9eba..90125fda31b 100644 --- a/sql/ddl_api.sql +++ b/sql/ddl_api.sql @@ -179,6 +179,7 @@ AS '@MODULE_PATHNAME@', 'ts_data_node_attach' LANGUAGE C VOLATILE; CREATE OR REPLACE FUNCTION detach_data_node( node_name NAME, hypertable REGCLASS = NULL, + if_attached BOOLEAN = FALSE, force BOOLEAN = FALSE, repartition BOOLEAN = TRUE ) RETURNS INTEGER diff --git a/sql/updates/latest-dev.sql b/sql/updates/latest-dev.sql index e69de29bb2d..76a4e2a58b7 100644 --- a/sql/updates/latest-dev.sql +++ b/sql/updates/latest-dev.sql @@ -0,0 +1,2 @@ + +DROP FUNCTION IF EXISTS detach_data_node(name,regclass,boolean,boolean); diff --git a/tsl/src/data_node.c b/tsl/src/data_node.c index 2a6adece2af..bb6caf92076 100644 --- a/tsl/src/data_node.c +++ b/tsl/src/data_node.c @@ -1133,14 +1133,14 @@ data_node_detach_hypertable_data_nodes(const char *node_name, List *hypertable_d } static HypertableDataNode * -get_hypertable_data_node(Oid table_id, const char *node_name, bool ownercheck) +get_hypertable_data_node(Oid table_id, const char *node_name, bool owner_check, bool attach_check) { HypertableDataNode *hdn = NULL; Cache *hcache = ts_hypertable_cache_pin(); Hypertable *ht = ts_hypertable_cache_get_entry(hcache, table_id, CACHE_FLAG_NONE); ListCell *lc; - if (ownercheck) + if (owner_check) ts_hypertable_permissions_check(table_id, GetUserId()); foreach (lc, ht->data_nodes) @@ -1153,11 +1153,21 @@ get_hypertable_data_node(Oid table_id, const char *node_name, bool ownercheck) } if (hdn == NULL) - ereport(ERROR, - (errcode(ERRCODE_TS_DATA_NODE_NOT_ATTACHED), - errmsg("data node \"%s\" is not attached to hypertable \"%s\"", - node_name, - get_rel_name(table_id)))); + { + if (attach_check) + ereport(ERROR, + (errcode(ERRCODE_TS_DATA_NODE_NOT_ATTACHED), + errmsg("data node \"%s\" is not attached to hypertable \"%s\"", + node_name, + get_rel_name(table_id)))); + else + ereport(NOTICE, + (errcode(ERRCODE_TS_DATA_NODE_NOT_ATTACHED), + errmsg("data node \"%s\" is not attached to hypertable \"%s\", " + "skipping", + node_name, + get_rel_name(table_id)))); + } ts_cache_release(hcache); @@ -1180,7 +1190,7 @@ data_node_block_or_allow_new_chunks(const char *node_name, Oid const table_id, b /* Early abort on missing hypertable permissions */ ts_hypertable_permissions_check(table_id, GetUserId()); hypertable_data_nodes = - list_make1(get_hypertable_data_node(table_id, server->servername, true)); + list_make1(get_hypertable_data_node(table_id, server->servername, true, true)); } else { @@ -1226,8 +1236,9 @@ data_node_detach(PG_FUNCTION_ARGS) const char *node_name = PG_ARGISNULL(0) ? NULL : NameStr(*PG_GETARG_NAME(0)); Oid table_id = PG_ARGISNULL(1) ? InvalidOid : PG_GETARG_OID(1); bool all_hypertables = PG_ARGISNULL(1); - bool force = PG_ARGISNULL(2) ? InvalidOid : PG_GETARG_OID(2); - bool repartition = PG_ARGISNULL(3) ? false : PG_GETARG_BOOL(3); + bool if_attached = PG_ARGISNULL(2) ? false : PG_GETARG_BOOL(2); + bool force = PG_ARGISNULL(3) ? InvalidOid : PG_GETARG_OID(3); + bool repartition = PG_ARGISNULL(4) ? false : PG_GETARG_BOOL(4); int removed = 0; List *hypertable_data_nodes = NIL; ForeignServer *server; @@ -1239,10 +1250,14 @@ data_node_detach(PG_FUNCTION_ARGS) if (OidIsValid(table_id)) { + HypertableDataNode *node; + /* Early abort on missing hypertable permissions */ ts_hypertable_permissions_check(table_id, GetUserId()); - hypertable_data_nodes = - list_make1(get_hypertable_data_node(table_id, server->servername, true)); + + node = get_hypertable_data_node(table_id, server->servername, true, !if_attached); + if (node) + hypertable_data_nodes = list_make1(node); } else { diff --git a/tsl/test/expected/data_node.out b/tsl/test/expected/data_node.out index 8189af7aaf4..ef2eee4e140 100644 --- a/tsl/test/expected/data_node.out +++ b/tsl/test/expected/data_node.out @@ -1026,6 +1026,8 @@ ERROR: detaching data node "data_node_1" failed because it contains chunks for -- can't detach already detached data node SELECT * FROM detach_data_node('data_node_2', 'disttable_2'); ERROR: data node "data_node_2" is not attached to hypertable "disttable_2" +SELECT * FROM detach_data_node('data_node_2', 'disttable_2', if_attached => false); +ERROR: data node "data_node_2" is not attached to hypertable "disttable_2" -- can't detach b/c of replication factor for disttable_2 SELECT * FROM detach_data_node('data_node_3', 'disttable_2'); ERROR: detaching data node "data_node_3" risks making new data for hypertable "disttable_2" under-replicated @@ -1033,6 +1035,14 @@ ERROR: detaching data node "data_node_3" risks making new data for hypertable " SELECT * FROM detach_data_node('data_node_3', 'devices'); ERROR: table "devices" is not a hypertable \set ON_ERROR_STOP 1 +-- do nothing if node is not attached +SELECT * FROM detach_data_node('data_node_2', 'disttable_2', if_attached => true); +NOTICE: data node "data_node_2" is not attached to hypertable "disttable_2", skipping + detach_data_node +------------------ + 0 +(1 row) + -- force detach data node to become under-replicated for new data SELECT * FROM detach_data_node('data_node_3', 'disttable_2', force => true); WARNING: new data for hypertable "disttable_2" will be under-replicated due to detaching data node "data_node_3" @@ -1127,7 +1137,7 @@ ORDER BY foreign_table_name; --------------------+--------------------- (0 rows) -SELECT * FROM detach_data_node('data_node_2', 'disttable', true); +SELECT * FROM detach_data_node('data_node_2', 'disttable', force => true); WARNING: new data for hypertable "disttable" will be under-replicated due to detaching data node "data_node_2" NOTICE: the number of partitions in dimension "device" was decreased to 1 detach_data_node diff --git a/tsl/test/sql/data_node.sql b/tsl/test/sql/data_node.sql index 3366e1f7ec4..4c16ed65529 100644 --- a/tsl/test/sql/data_node.sql +++ b/tsl/test/sql/data_node.sql @@ -508,12 +508,16 @@ SELECT * FROM detach_data_node(NULL, 'disttable'); SELECT * FROM detach_data_node('data_node_1'); -- can't detach already detached data node SELECT * FROM detach_data_node('data_node_2', 'disttable_2'); +SELECT * FROM detach_data_node('data_node_2', 'disttable_2', if_attached => false); -- can't detach b/c of replication factor for disttable_2 SELECT * FROM detach_data_node('data_node_3', 'disttable_2'); -- can't detach non hypertable SELECT * FROM detach_data_node('data_node_3', 'devices'); \set ON_ERROR_STOP 1 +-- do nothing if node is not attached +SELECT * FROM detach_data_node('data_node_2', 'disttable_2', if_attached => true); + -- force detach data node to become under-replicated for new data SELECT * FROM detach_data_node('data_node_3', 'disttable_2', force => true); @@ -547,7 +551,7 @@ SELECT foreign_table_name, foreign_server_name FROM information_schema.foreign_tables ORDER BY foreign_table_name; -SELECT * FROM detach_data_node('data_node_2', 'disttable', true); +SELECT * FROM detach_data_node('data_node_2', 'disttable', force => true); -- Let's add more data nodes SET ROLE :ROLE_CLUSTER_SUPERUSER;