From fb83cf50c90ed5c8e83654d41c2b234144dee3f3 Mon Sep 17 00:00:00 2001 From: MikhailBurdukov Date: Thu, 5 Oct 2023 13:31:40 +0000 Subject: [PATCH] Enable cleanup for replicated databases and do some refactoring. --- ch_tools/chadmin/internal/zookeeper.py | 93 ++++++++++++++++++------ tests/features/chadmin_zookeeper.feature | 24 +++++- 2 files changed, 91 insertions(+), 26 deletions(-) diff --git a/ch_tools/chadmin/internal/zookeeper.py b/ch_tools/chadmin/internal/zookeeper.py index dac92639..1790e08f 100644 --- a/ch_tools/chadmin/internal/zookeeper.py +++ b/ch_tools/chadmin/internal/zookeeper.py @@ -165,18 +165,53 @@ def _try_delete_zk_node(zk, node): # Do nothing. print("Node {node} is not empty, skipped".format(node=node)) - def _find_parents(zk, root_path, nodes, parent_name): + def _find_tables(zk, root_path, nodes): + """ + Find nodes mentions in zk for table management. + """ + hosts_metions = _find_paths(zk, root_path, [".*/" + node for node in nodes]) excluded_paths = [ ".*clickhouse/task_queue", ".*clickhouse/zero_copy", ] - included_paths = [".*/" + parent_name + "/" + node for node in nodes] - paths = _find_paths(zk, root_path, included_paths, excluded_paths) + included_paths = [".*/replicas/" + node for node in nodes] + + included_regexp = re.compile("|".join(included_paths)) + excluded_regexp = re.compile("|".join(excluded_paths)) + table_paths = list( + filter( + lambda path: not re.match(excluded_regexp, path) + and re.match(included_regexp, path), + hosts_metions, + ) + ) + # Paths will be like */shard1/replicas/hostname. But we need */shard1. # Go up for 2 directories. - paths = [os.sep.join(path.split(os.sep)[:-2]) for path in paths] + table_paths = [os.sep.join(path.split(os.sep)[:-2]) for path in table_paths] # One path might be in list several times. Make list unique. - return list(set(paths)) + return (list(set(table_paths)), hosts_metions) + + def _find_databases(zk, root_path, nodes): + """ + Databases contains replicas in zk in cases when engine is Replicated with + pattern: | + """ + hosts_mentions = _find_paths( + zk, root_path, [".*/.*[|]" + node for node in nodes] + ) + + included_paths = [".*/replicas/.*[|]" + node for node in nodes] + included_regexp = re.compile("|".join(included_paths)) + + databases_paths = list( + filter(lambda path: re.match(included_regexp, path), hosts_mentions) + ) + databases_paths = [ + os.sep.join(path.split(os.sep)[:-2]) for path in hosts_mentions + ] + + return (list(set(databases_paths)), hosts_mentions) def _set_replicas_is_lost(zk, table_paths, nodes): """ @@ -210,31 +245,45 @@ def _remove_replicas_queues(zk, paths, replica_names): if zk.exists(queue_path): _try_delete_zk_node(zk, queue_path) - def _nodes_absent(zk, zk_root_path, nodes): - included_paths = [".*/" + node for node in nodes] - paths_to_delete = _find_paths(zk, zk_root_path, included_paths) - for node in paths_to_delete: + def _remove_hosts_nodes(zk, paths): + for node in paths: _try_delete_zk_node(zk, node) - def _absent_if_empty_child(zk, path, child): + def _remove_if_no_hosts_in_replicas(zk, paths): """ Remove node if subnode is empty """ - child_full_path = os.path.join(path, child) - if ( - not zk.exists(child_full_path) - or len(_get_children(zk, child_full_path)) == 0 - ): - _try_delete_zk_node(zk, path) + for path in paths: + child_full_path = os.path.join(path, "replicas") + if ( + not zk.exists(child_full_path) + or len(_get_children(zk, child_full_path)) == 0 + ): + _try_delete_zk_node(zk, path) + + def tables_cleanup(zk, zk_root_path): + """ + Do cleanup for tables which contains nodes. + """ + (table_paths, hosts_paths) = _find_tables(zk, zk_root_path, nodes) + _set_replicas_is_lost(zk, table_paths, nodes) + _remove_replicas_queues(zk, table_paths, nodes) + _remove_hosts_nodes(zk, hosts_paths) + _remove_if_no_hosts_in_replicas(zk, table_paths) + + def databases_cleanups(zk, zk_root_path): + """ + Do cleanup for databases which contains nodes. + """ + + (databases_paths, hosts_paths) = _find_databases(zk, zk_root_path, nodes) + _remove_hosts_nodes(zk, hosts_paths) + _remove_if_no_hosts_in_replicas(zk, databases_paths) zk_root_path = _format_path(ctx, "/") with zk_client(ctx) as zk: - table_paths = _find_parents(zk, zk_root_path, nodes, "replicas") - _set_replicas_is_lost(zk, table_paths, nodes) - _remove_replicas_queues(zk, table_paths, nodes) - _nodes_absent(zk, zk_root_path, nodes) - for path in table_paths: - _absent_if_empty_child(zk, path, "replicas") + tables_cleanup(zk, zk_root_path) + databases_cleanups(zk, zk_root_path) @contextmanager diff --git a/tests/features/chadmin_zookeeper.feature b/tests/features/chadmin_zookeeper.feature index a60faeeb..f93df3f0 100644 --- a/tests/features/chadmin_zookeeper.feature +++ b/tests/features/chadmin_zookeeper.feature @@ -43,13 +43,29 @@ Feature: keeper-monitoring tool """ - Scenario: Cleanup from Replicated database. + Scenario: Remove single host from Replicated database. # Imitate that we got nodes in zk from replicated database. When we execute chadmin create zk nodes on zookeeper01 """ - '/test/clickhouse/databases/test/shard1/replicas/shard1|hos1.net' - '/test/clickhouse/databases/test/shard1/replicas/shard1|hos2.net' + '/test/clickhouse/databases/test/shard1/replicas/shard1|host1.net' + '/test/clickhouse/databases/test/shard1/replicas/shard1|host2.net' '/test/clickhouse/databases/test/shard1/counter' """ And we do hosts cleanup on zookeeper01 with fqdn host1.net and zk root /test - Then asdasda + Then the list of children on zookeeper01 for zk node /test/clickhouse/databases/test/shard1/replicas/ are equal to + """ + /test/clickhouse/databases/test/shard1/replicas/shard1|host2.net + """ + + Scenario: Remove all host from Replicated database. + # Imitate that we got nodes in zk from replicated database. + When we execute chadmin create zk nodes on zookeeper01 + """ + '/test/clickhouse/databases/test/shard1/replicas/shard1|host1.net' + '/test/clickhouse/databases/test/shard1/replicas/shard1|host2.net' + '/test/clickhouse/databases/test/shard1/counter' + """ + And we do hosts cleanup on zookeeper01 with fqdn host1.net,host2.net and zk root /test + Then the list of children on zookeeper01 for zk node /test/clickhouse/databases/test/ are equal to + """ + """