Merge a23534a0a3 into 0fd196771e

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);

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")