From 9c185374e460cfab452783c86964300e880145b3 Mon Sep 17 00:00:00 2001 From: Mikhail Artemenko Date: Tue, 17 Sep 2024 18:14:47 +0000 Subject: [PATCH] fix level sorting --- src/Coordination/KeeperStorage.h | 8 +++--- src/Coordination/tests/gtest_coordination.cpp | 26 +++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/Coordination/KeeperStorage.h b/src/Coordination/KeeperStorage.h index 6fbc4c2b168..ec502a38a72 100644 --- a/src/Coordination/KeeperStorage.h +++ b/src/Coordination/KeeperStorage.h @@ -613,13 +613,15 @@ public: struct PathCmp { - using is_transparent = std::true_type; - auto operator()(const std::string_view a, const std::string_view b) const { - return a.size() < b.size() || (a.size() == b.size() && a < b); + size_t level_a = std::count(a.begin(), a.end(), '/'); + size_t level_b = std::count(b.begin(), b.end(), '/'); + return level_a < level_b || (level_a == level_b && a < b); } + + using is_transparent = void; // required to make find() work with different type than key_type }; mutable std::map nodes; diff --git a/src/Coordination/tests/gtest_coordination.cpp b/src/Coordination/tests/gtest_coordination.cpp index 15d5f460e94..4272e504da1 100644 --- a/src/Coordination/tests/gtest_coordination.cpp +++ b/src/Coordination/tests/gtest_coordination.cpp @@ -3778,6 +3778,32 @@ TYPED_TEST(CoordinationTest, TestRemoveRecursiveInMultiRequest) ASSERT_FALSE(exists("/A/B")); ASSERT_FALSE(exists("/A/B/D")); } + + { + SCOPED_TRACE("[BUG] Recursive Remove Level Sorting"); + int new_zxid = ++zxid; + + Coordination::Requests ops = { + zkutil::makeCreateRequest("/a", "", zkutil::CreateMode::Persistent), + zkutil::makeCreateRequest("/a/bbbbbb", "", zkutil::CreateMode::Persistent), + zkutil::makeCreateRequest("/A", "", zkutil::CreateMode::Persistent), + zkutil::makeCreateRequest("/A/B", "", zkutil::CreateMode::Persistent), + zkutil::makeCreateRequest("/A/CCCCCCCCCCCC", "", zkutil::CreateMode::Persistent), + zkutil::makeRemoveRecursiveRequest("/A", 3), + }; + auto remove_request = std::make_shared(ops, ACLs{}); + storage.preprocessRequest(remove_request, 1, 0, new_zxid); + auto remove_responses = storage.processRequest(remove_request, 1, new_zxid); + + ASSERT_EQ(remove_responses.size(), 1); + ASSERT_TRUE(is_multi_ok(remove_responses[0].response)); + ASSERT_TRUE(exists("/a")); + ASSERT_TRUE(exists("/a/bbbbbb")); + ASSERT_FALSE(exists("/A")); + ASSERT_FALSE(exists("/A/B")); + ASSERT_FALSE(exists("/A/CCCCCCCCCCCC")); + } + } TYPED_TEST(CoordinationTest, TestRemoveRecursiveWatches)