diff --git a/src/Access/ReplicatedAccessStorage.cpp b/src/Access/ReplicatedAccessStorage.cpp index f0cb8a5c9f0..c7a513d23fc 100644 --- a/src/Access/ReplicatedAccessStorage.cpp +++ b/src/Access/ReplicatedAccessStorage.cpp @@ -287,7 +287,7 @@ bool ReplicatedAccessStorage::removeImpl(const UUID & id, bool throw_if_not_exis } -bool ReplicatedAccessStorage::removeZooKeeper(const zkutil::ZooKeeperPtr & zookeeper, const UUID & id, bool throw_if_not_exists) +bool ReplicatedAccessStorage::removeZooKeeper(const zkutil::ZooKeeperPtr & zookeeper, const UUID & id, const bool throw_if_not_exists) { const String entity_uuid = toString(id); const String entity_path = zookeeper_path + "/uuid/" + entity_uuid; @@ -310,9 +310,22 @@ bool ReplicatedAccessStorage::removeZooKeeper(const zkutil::ZooKeeperPtr & zooke const String entity_name_path = zookeeper_path + "/" + type_info.unique_char + "/" + escapeForFileName(name); + String entity_name_definition; + Coordination::Stat entity_name_stat; + const bool entity_name_exists = zookeeper->tryGet(entity_name_path, entity_name_definition, &entity_name_stat); + Coordination::Requests ops; ops.emplace_back(zkutil::makeRemoveRequest(entity_path, entity_stat.version)); - ops.emplace_back(zkutil::makeRemoveRequest(entity_name_path, -1)); + if (entity_name_exists) + { + ops.emplace_back(zkutil::makeRemoveRequest(entity_name_path, entity_name_stat.version)); + } + else + { + LOG_WARNING( + getLogger(), + "Although uuid entity was founded, name entity doesn't exist. Data partially removed. Need to remove uuid entity."); + } /// If this fails, then we'll just retry from the start. zookeeper->multi(ops); diff --git a/tests/integration/test_replicated_users/test.py b/tests/integration/test_replicated_users/test.py index ed524ce0118..a4c3cb0e5b5 100644 --- a/tests/integration/test_replicated_users/test.py +++ b/tests/integration/test_replicated_users/test.py @@ -175,6 +175,39 @@ def test_rename_replicated(started_cluster, entity): node1.query(f"DROP {entity.keyword} {entity.name}2 {entity.options}") +def test_create_broken_and_delete_replicated(started_cluster): + def name_user_zk_path(name: str) -> str: + return f"/clickhouse/access/U/{name}/" + + def uuid_user_zk_path(uuid: str) -> str: + return f"/clickhouse/access/uuid/{uuid}/" + + zk = cluster.get_kazoo_client("zoo1") + + user_name = "theuser_drop_name" + node1.query(f"CREATE USER {user_name}") + uuid = node1.query(f"SELECT id FROM system.users WHERE name='{user_name}' AND storage='replicated'").strip() + name_zk_path = name_user_zk_path(user_name) + uuid_zk_path = uuid_user_zk_path(uuid) + assert zk.exists(name_zk_path) + assert zk.exists(uuid_zk_path) + zk.delete(name_zk_path) + node1.query_with_retry(f"DROP USER {user_name}") + assert not zk.exists(name_zk_path) + assert not zk.exists(uuid_zk_path) + + user_name = "theuser_drop_uuid" + node1.query(f"CREATE USER {user_name}") + uuid = node1.query(f"SELECT id FROM system.users WHERE name='{user_name}' AND storage='replicated'").strip() + name_zk_path = name_user_zk_path(user_name) + uuid_zk_path = uuid_user_zk_path(uuid) + assert zk.exists(name_zk_path) + assert zk.exists(uuid_zk_path) + zk.delete(uuid_zk_path) + assert f"There is no user `{user_name}` in user directories" in node1.query_and_get_error(f"DROP USER {user_name}") + assert zk.exists(name_zk_path) + + # ReplicatedAccessStorage must be able to continue working after reloading ZooKeeper. def test_reload_zookeeper(started_cluster): node1.query("CREATE USER u1")