Replace StringRef with std::string_view in Keeper code

base/base/StringRef.h

@@ -310,6 +310,7 @@ inline size_t hashLessThan16(const char * data, size_t size)
 
 struct CRC32Hash
 {
+    size_t operator()(std::string_view x) const { return (*this)(StringRef{x}); }
     size_t operator() (StringRef x) const
     {
         const char * pos = x.data;
@@ -349,6 +350,8 @@ struct CRC32Hash
 
 struct StringRefHash : CRC32Hash {};
 
+using StringViewHash = StringRefHash;
+
 #else
 
 struct CRC32Hash

programs/keeper-data-dumper/KeeperDataDumper.cpp

@@ -47,9 +47,9 @@ void dumpMachine(std::shared_ptr<KeeperStateMachine<DB::KeeperMemoryStorage>> ma
         for (const auto & child : value.getChildren())
         {
             if (key == "/")
-                keys.push(key + child.toString());
+                keys.push(key + std::string{child});
             else
-                keys.push(key + "/" + child.toString());
+                keys.push(key + "/" + std::string{child});
         }
     }
     std::cout << std::flush;

programs/keeper-utils/KeeperUtils.cpp

@@ -159,13 +159,13 @@ void analyzeSnapshot(const std::string & snapshot_path, bool full_storage, bool
                     if (with_node_stats)
                     {
                         std::cout << "Finding biggest subtrees... " << std::endl;
-                        std::unordered_map<StringRef, size_t> subtree_sizes;
+                        std::unordered_map<std::string_view, size_t> subtree_sizes;
                         for (const auto & path : result.paths)
                         {
                             if (path == "/")
                                 continue;
 
-                            StringRef current_path = path;
+                            std::string_view current_path = path;
                             while (true)
                             {
                                 auto parent = parentNodePath(current_path);
@@ -475,9 +475,9 @@ void dumpNodes(const DB::KeeperMemoryStorage & storage, const std::string & outp
             for (const auto & child : value.getChildren())
             {
                 if (key == "/")
-                    keys.push(key + child.toString());
+                    keys.push(fmt::format("/{}", child));
                 else
-                    keys.push(key + "/" + child.toString());
+                    keys.push(fmt::format("{}/{}", key, child));
             }
         }
     };

src/Common/HashTable/Hash.h

@@ -541,3 +541,6 @@ struct IntHash32
 
 template <>
 struct DefaultHash<StringRef> : public StringRefHash {};
+
+template <>
+struct DefaultHash<std::string_view> : public StringViewHash {};

src/Common/ZooKeeper/ZooKeeperCommon.cpp

@@ -1273,36 +1273,36 @@ PathMatchResult matchPath(std::string_view path, std::string_view match_to)
 
 namespace
 {
-size_t findLastSlash(StringRef path)
+size_t findLastSlash(std::string_view path)
 {
-    if (path.size == 0)
+    if (path.empty())
         return std::string::npos;
 
-    for (size_t i = path.size - 1; i > 0; --i)
+    for (size_t i = path.size() - 1; i > 0; --i)
     {
-        if (path.data[i] == '/')
+        if (path[i] == '/')
             return i;
     }
 
-    if (path.data[0] == '/')
+    if (path[0] == '/')
         return 0;
 
     return std::string::npos;
 }
 }
 
-StringRef parentNodePath(StringRef path)
+std::string_view parentNodePath(std::string_view path)
 {
     auto rslash_pos = findLastSlash(path);
     if (rslash_pos > 0)
-        return StringRef{path.data, rslash_pos};
+        return path.substr(0, rslash_pos);
     return "/";
 }
 
-StringRef getBaseNodeName(StringRef path)
+std::string_view getBaseNodeName(std::string_view path)
 {
     size_t basename_start = findLastSlash(path);
-    return StringRef{path.data + basename_start + 1, path.size - basename_start - 1};
+    return path.substr(basename_start + 1, path.size() - basename_start - 1);
 }
 
 }

src/Common/ZooKeeper/ZooKeeperCommon.h

@@ -693,9 +693,9 @@ enum class PathMatchResult : uint8_t
 
 PathMatchResult matchPath(std::string_view path, std::string_view match_to);
 
-StringRef parentNodePath(StringRef path);
+std::string_view parentNodePath(std::string_view path);
 
-StringRef getBaseNodeName(StringRef path);
+std::string_view getBaseNodeName(std::string_view path);
 
 
 }

src/Coordination/KeeperCommon.h

@@ -1,6 +1,5 @@
 #pragma once
 
-#include <base/StringRef.h>
 #include <Common/Logger.h>
 
 #include <functional>

src/Coordination/KeeperSnapshotManager.cpp

@@ -256,7 +256,7 @@ void KeeperStorageSnapshot<Storage>::serialize(const KeeperStorageSnapshot<Stora
         const auto & path = it->key;
 
         // write only the root system path because of digest
-        if (Coordination::matchPath(path.toView(), keeper_system_path) == Coordination::PathMatchResult::IS_CHILD)
+        if (Coordination::matchPath(path, keeper_system_path) == Coordination::PathMatchResult::IS_CHILD)
         {
             if (counter == snapshot.snapshot_container_size - 1)
                 break;
@@ -511,7 +511,7 @@ void KeeperStorageSnapshot<Storage>::deserialize(SnapshotDeserializationResult<S
                         " is different from actual children size {} for node {}",
                         itr.value.stats.numChildren(),
                         itr.value.getChildren().size(),
-                        itr.key.toView());
+                        itr.key);
 #else
                     throw Exception(
                         ErrorCodes::LOGICAL_ERROR,

src/Coordination/KeeperStorage.cpp

@@ -7,6 +7,7 @@
 #include <Poco/SHA1Engine.h>
 
 #include <Common/Base64.h>
+#include <Common/Exception.h>
 #include <Common/FailPoint.h>
 #include <Common/ZooKeeper/ZooKeeperCommon.h>
 #include <Common/SipHash.h>
@@ -176,12 +177,12 @@ KeeperResponsesForSessions processWatchesImpl(
     Strings paths_to_check_for_list_watches;
     if (event_type == Coordination::Event::CREATED)
     {
-        paths_to_check_for_list_watches.push_back(parent_path.toString()); /// Trigger list watches for parent
+        paths_to_check_for_list_watches.push_back(std::string{parent_path}); /// Trigger list watches for parent
     }
     else if (event_type == Coordination::Event::DELETED)
     {
         paths_to_check_for_list_watches.push_back(path); /// Trigger both list watches for this path
-        paths_to_check_for_list_watches.push_back(parent_path.toString()); /// And for parent path
+        paths_to_check_for_list_watches.push_back(std::string{parent_path}); /// And for parent path
     }
     /// CHANGED event never trigger list watches
 
@@ -346,7 +347,6 @@ KeeperMemNode & KeeperMemNode::operator=(const KeeperMemNode & other)
     }
 
     children = other.children;
-
     return *this;
 }
 
@@ -405,7 +405,7 @@ void KeeperMemNode::setResponseStat(Coordination::Stat & response_stat) const
 
 uint64_t KeeperMemNode::sizeInBytes() const
 {
-    return sizeof(KeeperMemNode) + children.size() * sizeof(StringRef) + stats.data_size;
+    return sizeof(KeeperMemNode) + children.size() * sizeof(std::string_view) + stats.data_size;
 }
 
 void KeeperMemNode::setData(const String & new_data)
@@ -418,12 +418,12 @@ void KeeperMemNode::setData(const String & new_data)
     }
 }
 
-void KeeperMemNode::addChild(StringRef child_path)
+void KeeperMemNode::addChild(std::string_view child_path)
 {
     children.insert(child_path);
 }
 
-void KeeperMemNode::removeChild(StringRef child_path)
+void KeeperMemNode::removeChild(std::string_view child_path)
 {
     children.erase(child_path);
 }
@@ -585,7 +585,7 @@ KeeperStorageBase::KeeperStorageBase(int64_t tick_time_ms, const KeeperContextPt
 template <typename Container>
 KeeperStorage<Container>::KeeperStorage(
     int64_t tick_time_ms, const String & superdigest_, const KeeperContextPtr & keeper_context_, const bool initialize_system_nodes)
-    : KeeperStorageBase(tick_time_ms, keeper_context_, superdigest_)
+    try : KeeperStorageBase(tick_time_ms, keeper_context_, superdigest_)
 {
     if constexpr (use_rocksdb)
         container.initialize(keeper_context);
@@ -597,6 +597,10 @@ KeeperStorage<Container>::KeeperStorage(
     if (initialize_system_nodes)
         initializeSystemNodes();
 }
+catch (...)
+{
+    tryLogCurrentException(getLogger("HLEP"));
+}
 
 template<typename Container>
 void KeeperStorage<Container>::initializeSystemNodes()
@@ -649,7 +653,7 @@ void KeeperStorage<Container>::initializeSystemNodes()
             /// Take child path from key owned by map.
             auto child_path = Coordination::getBaseNodeName(map_key->getKey());
             container.updateValue(
-                Coordination::parentNodePath(StringRef(path)),
+                Coordination::parentNodePath(path),
                 [child_path](auto & parent)
                 {
                     // don't update stats so digest is okay
@@ -675,7 +679,7 @@ template <class... Ts>
 Overloaded(Ts...) -> Overloaded<Ts...>;  /// NOLINT(misc-use-internal-linkage)
 
 template<typename Container>
-std::shared_ptr<typename Container::Node> KeeperStorage<Container>::UncommittedState::tryGetNodeFromStorage(StringRef path, bool should_lock_storage) const
+std::shared_ptr<typename Container::Node> KeeperStorage<Container>::UncommittedState::tryGetNodeFromStorage(std::string_view path, bool should_lock_storage) const
 {
     std::shared_lock lock(storage.storage_mutex, std::defer_lock);
     if (should_lock_storage)
@@ -1063,9 +1067,9 @@ void KeeperStorage<Container>::UncommittedState::rollback(std::list<Delta> rollb
 }
 
 template<typename Container>
-std::shared_ptr<typename Container::Node> KeeperStorage<Container>::UncommittedState::getNode(StringRef path, bool should_lock_storage) const
+std::shared_ptr<typename Container::Node> KeeperStorage<Container>::UncommittedState::getNode(std::string_view path, bool should_lock_storage) const
 {
-    if (auto node_it = nodes.find(path.toView()); node_it != nodes.end())
+    if (auto node_it = nodes.find(path); node_it != nodes.end())
         return node_it->second.node;
 
     std::shared_ptr<KeeperStorage::Node> node = tryGetNodeFromStorage(path, should_lock_storage);
@@ -1080,18 +1084,9 @@ std::shared_ptr<typename Container::Node> KeeperStorage<Container>::UncommittedS
 }
 
 template<typename Container>
-const typename Container::Node * KeeperStorage<Container>::UncommittedState::getActualNodeView(StringRef path, const Node & storage_node) const
+Coordination::ACLs KeeperStorage<Container>::UncommittedState::getACLs(std::string_view path) const
 {
-    if (auto node_it = nodes.find(path.toView()); node_it != nodes.end())
-        return node_it->second.node.get();
-
-    return &storage_node;
-}
-
-template<typename Container>
-Coordination::ACLs KeeperStorage<Container>::UncommittedState::getACLs(StringRef path) const
-{
-    auto node_it = nodes.find(path.toView());
+    auto node_it = nodes.find(path);
     if (node_it == nodes.end())
     {
         std::shared_ptr<KeeperStorage::Node> node = tryGetNodeFromStorage(path);
@@ -1356,7 +1351,7 @@ bool KeeperStorage<Container>::createNode(
         );
 
         if (update_digest)
-            addDigest(map_key->getMapped()->value, map_key->getKey().toView());
+            addDigest(map_key->getMapped()->value, map_key->getKey());
     }
 
     if (stat.ephemeralOwner != 0)
@@ -1461,7 +1456,7 @@ namespace
 {
 
 template<typename Storage>
-Coordination::ACLs getNodeACLs(Storage & storage, StringRef path, bool is_local)
+Coordination::ACLs getNodeACLs(Storage & storage, std::string_view path, bool is_local)
 {
     if (is_local)
     {
@@ -1491,7 +1486,7 @@ void handleSystemNodeModification(const KeeperContext & keeper_context, std::str
 }
 
 template<typename Container>
-bool KeeperStorage<Container>::checkACL(StringRef path, int32_t permission, int64_t session_id, bool is_local)
+bool KeeperStorage<Container>::checkACL(std::string_view path, int32_t permission, int64_t session_id, bool is_local)
 {
     const auto node_acls = getNodeACLs(*this, path, is_local);
     if (node_acls.empty())
@@ -1652,7 +1647,7 @@ std::list<KeeperStorageBase::Delta> preprocess(
         return {KeeperStorageBase::Delta{zxid, Coordination::Error::ZNODEEXISTS}};
     }
 
-    if (Coordination::getBaseNodeName(path_created).size == 0)
+    if (Coordination::getBaseNodeName(path_created).empty())
         return {KeeperStorageBase::Delta{zxid, Coordination::Error::ZBADARGUMENTS}};
 
     Coordination::ACLs node_acls;
@@ -1978,7 +1973,7 @@ public:
     {
     }
 
-    CollectStatus collect(StringRef root_path, const Storage::Node & root_node)
+    CollectStatus collect(std::string_view root_path, const Storage::Node & root_node)
     {
         if (checkLimits(root_node))
             return CollectStatus::LimitExceeded;
@@ -2018,14 +2013,14 @@ public:
     }
 
 private:
-    CollectStatus visitRocksDBNode(StringRef current_path, const UncommittedChildren & uncommitted_children) requires Storage::use_rocksdb
+    CollectStatus visitRocksDBNode(std::string_view current_path, const UncommittedChildren & uncommitted_children) requires Storage::use_rocksdb
     {
-        std::filesystem::path current_path_fs(current_path.toString());
+        std::filesystem::path current_path_fs(current_path);
 
         std::vector<std::pair<std::string, typename Storage::Node>> children;
         {
             std::lock_guard lock(storage.storage_mutex);
-            children = storage.container.getChildren(current_path.toString(), /*read_meta*/ true, /*read_data=*/true);
+            children = storage.container.getChildren(current_path, /*read_meta*/ true, /*read_data=*/true);
         }
 
         for (auto && [child_name, child_node] : children)
@@ -2044,19 +2039,19 @@ private:
         return CollectStatus::Ok;
     }
 
-    CollectStatus visitMemNode(StringRef current_path, const UncommittedChildren & uncommitted_children) requires (!Storage::use_rocksdb)
+    CollectStatus visitMemNode(std::string_view current_path, const UncommittedChildren & uncommitted_children) requires (!Storage::use_rocksdb)
     {
         std::lock_guard lock(storage.storage_mutex);
         auto node_it = storage.container.find(current_path);
         if (node_it == storage.container.end())
             return CollectStatus::Ok;
 
-        std::filesystem::path current_path_fs(current_path.toString());
+        std::filesystem::path current_path_fs(current_path);
         const auto & children = node_it->value.getChildren();
 
         for (const auto & child_name : children)
         {
-            auto child_path = (current_path_fs / child_name.toView()).generic_string();
+            auto child_path = (current_path_fs / child_name).generic_string();
 
             if (uncommitted_children.contains(child_path))
                 continue;
@@ -2100,7 +2095,7 @@ private:
         return CollectStatus::Ok;
     }
 
-    void addDelta(StringRef path, const NodeStats & stats, Coordination::ACLs acls, std::string data)
+    void addDelta(std::string_view path, const NodeStats & stats, Coordination::ACLs acls, std::string data)
     {
         deltas.emplace_front(std::string{path}, zxid, RemoveNodeDelta{/*version=*/-1, stats, std::move(acls), std::move(data)});
     }
@@ -2505,7 +2500,7 @@ Coordination::ZooKeeperResponsePtr processImpl(const Coordination::ZooKeeperList
             bool is_ephemeral;
             if constexpr (!Storage::use_rocksdb)
             {
-                auto child_path = (std::filesystem::path(zk_request.path) / child.toView()).generic_string();
+                auto child_path = (std::filesystem::path(zk_request.path) / child).generic_string();
                 auto child_it = container.find(child_path);
                 if (child_it == container.end())
                     onStorageInconsistency("Failed to find a child");
@@ -2526,7 +2521,7 @@ Coordination::ZooKeeperResponsePtr processImpl(const Coordination::ZooKeeperList
                 if constexpr (Storage::use_rocksdb)
                     response->names.push_back(child.first);
                 else
-                    response->names.push_back(child.toString());
+                    response->names.push_back(std::string{child});
             }
         }
 
@@ -3181,12 +3176,12 @@ KeeperDigest KeeperStorage<Container>::preprocessRequest(
                     if (!node || node->stats.ephemeralOwner() != session_id)
                         continue;
 
-                    auto parent_node_path = Coordination::parentNodePath(ephemeral_path).toView();
+                    auto parent_node_path = Coordination::parentNodePath(ephemeral_path);
 
                     auto parent_update_it = parent_updates.find(parent_node_path);
                     if (parent_update_it == parent_updates.end())
                     {
-                        auto parent_node = uncommitted_state.getNode(StringRef{parent_node_path}, /*should_lock_storage=*/false);
+                        auto parent_node = uncommitted_state.getNode(parent_node_path, /*should_lock_storage=*/false);
                         std::tie(parent_update_it, std::ignore) = parent_updates.emplace(parent_node_path, *parent_node);
                     }
 

src/Coordination/KeeperStorage.h

@@ -27,7 +27,7 @@ class KeeperContext;
 using KeeperContextPtr = std::shared_ptr<KeeperContext>;
 
 using ResponseCallback = std::function<void(const Coordination::ZooKeeperResponsePtr &)>;
-using ChildrenSet = absl::flat_hash_set<StringRef, StringRefHash>;
+using ChildrenSet = absl::flat_hash_set<std::string_view, StringViewHash>;
 
 struct NodeStats
 {
@@ -149,7 +149,7 @@ struct KeeperRocksNodeInfo
     uint64_t acl_id = 0; /// 0 -- no ACL by default
 
     /// dummy interface for test
-    void addChild(StringRef) {}
+    void addChild(std::string_view) {}
     auto getChildren() const
     {
         return std::vector<int>(stats.numChildren());
@@ -247,9 +247,9 @@ struct KeeperMemNode
 
     std::string_view getData() const noexcept { return {data.get(), stats.data_size}; }
 
-    void addChild(StringRef child_path);
+    void addChild(std::string_view child_path);
 
-    void removeChild(StringRef child_path);
+    void removeChild(std::string_view child_path);
 
     template <typename Self>
     auto & getChildren(this Self & self)
@@ -500,10 +500,9 @@ public:
         void rollback(int64_t rollback_zxid);
         void rollback(std::list<Delta> rollback_deltas);
 
-        std::shared_ptr<Node> getNode(StringRef path, bool should_lock_storage = true) const;
-        const Node * getActualNodeView(StringRef path, const Node & storage_node) const;
+        std::shared_ptr<Node> getNode(std::string_view path, bool should_lock_storage = true) const;
 
-        Coordination::ACLs getACLs(StringRef path) const;
+        Coordination::ACLs getACLs(std::string_view path) const;
 
         void applyDeltas(const std::list<Delta> & new_deltas, uint64_t * digest);
         void applyDelta(const Delta & delta, uint64_t * digest);
@@ -516,7 +515,7 @@ public:
 
         void forEachAuthInSession(int64_t session_id, std::function<void(const AuthID &)> func) const;
 
-        std::shared_ptr<Node> tryGetNodeFromStorage(StringRef path, bool should_lock_storage = true) const;
+        std::shared_ptr<Node> tryGetNodeFromStorage(std::string_view path, bool should_lock_storage = true) const;
 
         std::unordered_map<int64_t, std::unordered_set<int64_t>> closed_sessions_to_zxids;
 
@@ -580,7 +579,7 @@ public:
     // We don't care about the exact failure because we should've caught it during preprocessing
     bool removeNode(const std::string & path, int32_t version, bool update_digest);
 
-    bool checkACL(StringRef path, int32_t permissions, int64_t session_id, bool is_local);
+    bool checkACL(std::string_view path, int32_t permissions, int64_t session_id, bool is_local);
 
     KeeperStorage(int64_t tick_time_ms, const String & superdigest_, const KeeperContextPtr & keeper_context_, bool initialize_system_nodes = true);
 

src/Coordination/RocksDBContainer.h

@@ -1,5 +1,4 @@
 #pragma once
-#include <base/StringRef.h>
 #include <Coordination/KeeperContext.h>
 #include <Disks/DiskLocal.h>
 #include <IO/WriteBufferFromString.h>
@@ -51,7 +50,7 @@ private:
 
     struct KVPair
     {
-        StringRef key;
+        std::string_view key;
         Node value;
     };
 
@@ -96,7 +95,7 @@ public:
                 return true;
             if (pair == nullptr || other == nullptr)
                 return false;
-            return pair->key.toView() == other->key.toView() && iter == other.iter;
+            return pair->key == other->key && iter == other.iter;
         }
 
         bool operator == (std::nullptr_t) const
@@ -127,7 +126,7 @@ public:
             if (iter && iter->Valid())
             {
                 auto new_pair = std::make_shared<KVPair>();
-                new_pair->key = StringRef(iter->key().ToStringView());
+                new_pair->key = iter->key().ToStringView();
                 ReadBufferFromOwnString buffer(iter->value().ToStringView());
                 typename Node::Meta & meta = new_pair->value;
                 readPODBinary(meta, buffer);
@@ -188,12 +187,12 @@ public:
         }
     }
 
-    std::vector<std::pair<std::string, Node>> getChildren(const std::string & key_prefix, bool read_meta = true, bool read_data = false)
+    std::vector<std::pair<std::string, Node>> getChildren(std::string_view key_prefix, bool read_meta = true, bool read_data = false)
     {
         rocksdb::ReadOptions read_options;
         read_options.total_order_seek = true;
 
-        std::string key = key_prefix;
+        std::string key{key_prefix};
         if (!key.ends_with('/'))
             key += '/';
         size_t len = key.size();
@@ -257,11 +256,11 @@ public:
         return true;
     }
 
-    const_iterator find(StringRef key)
+    const_iterator find(std::string_view key)
     {
         /// rocksdb::PinnableSlice slice;
         std::string buffer_str;
-        rocksdb::Status status = rocksdb_ptr->Get(rocksdb::ReadOptions(), key.toView(), &buffer_str);
+        rocksdb::Status status = rocksdb_ptr->Get(rocksdb::ReadOptions(), key, &buffer_str);
         if (status.IsNotFound())
             return end();
         if (!status.ok())
@@ -281,24 +280,24 @@ public:
         return const_iterator(kv);
     }
 
-    MockNode getValue(StringRef key)
+    MockNode getValue(std::string_view key)
     {
         auto it = find(key);
         chassert(it != end());
         return MockNode(it->value.stats.numChildren(), it->value.getData(), it->value.acl_id);
     }
 
-    const_iterator updateValue(StringRef key, ValueUpdater updater)
+    const_iterator updateValue(std::string_view key, ValueUpdater updater)
     {
         std::string buffer_str;
-        rocksdb::Status status = rocksdb_ptr->Get(rocksdb::ReadOptions(), key.toView(), &buffer_str);
+        rocksdb::Status status = rocksdb_ptr->Get(rocksdb::ReadOptions(), key, &buffer_str);
         if (!status.ok())
             throw Exception(ErrorCodes::ROCKSDB_ERROR, "Got rocksdb error during find. The error message is {}.", status.ToString());
         auto kv = std::make_shared<KVPair>();
         kv->key = key;
         kv->value.decodeFromString(buffer_str);
         updater(kv->value);
-        insertOrReplace(key.toString(), kv->value, /*update=*/true);
+        insertOrReplace(key, kv->value, /*update=*/true);
         return const_iterator(kv);
     }
 
@@ -387,7 +386,7 @@ public:
             throw Exception(ErrorCodes::ROCKSDB_ERROR, "Got rocksdb error during insert. The error message is {}.", status.ToString());
     }
 
-    void insertOrReplace(const std::string & key, Node & value, bool update = false)
+    void insertOrReplace(const std::string_view key, Node & value, bool update = false)
     {
         if (write_batch)
         {

src/Coordination/SnapshotableHashTable.h

@@ -1,8 +1,6 @@
 #pragma once
-#include <base/StringRef.h>
 #include <Common/HashTable/HashMap.h>
 #include <Common/ArenaUtils.h>
-
 #include <list>
 
 namespace DB
@@ -16,7 +14,7 @@ extern const int LOGICAL_ERROR;
 template<typename V>
 struct ListNode
 {
-    StringRef key;
+    std::string_view key;
     V value;
 
     struct
@@ -87,7 +85,7 @@ private:
     using ListElem = ListNode<V>;
     using List = std::list<ListElem>;
     using Mapped = typename List::iterator;
-    using IndexMap = HashMap<StringRef, Mapped>;
+    using IndexMap = HashMap<std::string_view, Mapped>;
 
     List list;
     IndexMap map;
@@ -98,7 +96,7 @@ private:
 
     /// Arena used for keys
     /// we don't use std::string because it uses 24 bytes (because of SSO)
-    /// we want to always allocate the key on heap and use StringRef to it
+    /// we want to always allocate the key on heap and use std::string_view to it
     GlobalArena arena;
 
     /// Collect invalid iterators to avoid traversing the whole list
@@ -155,7 +153,7 @@ private:
         }
     }
 
-    void insertOrReplace(StringRef key, V value, bool owns_key)
+    void insertOrReplace(std::string_view key, V value, bool owns_key)
     {
         size_t hash_value = map.hash(key);
         auto new_value_size = value.sizeInBytes();
@@ -164,7 +162,7 @@ private:
 
         if (it == map.end())
         {
-            auto list_key = owns_key ? key : copyStringInArena(arena, key);
+            auto list_key = owns_key ? key : copyStringInArena(arena, StringRef{key}).toView();
             ListElem elem{list_key, std::move(value)};
             elem.setVersion(current_version);
             auto itr = list.insert(list.end(), std::move(elem));
@@ -177,7 +175,7 @@ private:
         else
         {
             if (owns_key)
-                arena.free(key.data, key.size);
+                arena.free(key.data(), key.size());
 
             auto list_itr = it->getMapped();
             if (snapshot_mode)
@@ -194,7 +192,7 @@ private:
                 list_itr->value = std::move(value);
             }
         }
-        updateDataSize(INSERT_OR_REPLACE, key.size, new_value_size, old_value_size, !snapshot_mode);
+        updateDataSize(INSERT_OR_REPLACE, key.size(), new_value_size, old_value_size, !snapshot_mode);
     }
 
 public:
@@ -217,7 +215,7 @@ public:
         if (!it)
         {
             auto value_size = value.sizeInBytes();
-            ListElem elem{copyStringInArena(arena, key), std::move(value)};
+            ListElem elem{copyStringInArena(arena, key).toView(), std::move(value)};
             elem.setVersion(current_version);
             auto itr = list.insert(list.end(), std::move(elem));
             bool inserted;
@@ -261,7 +259,7 @@ public:
 
     void insertOrReplace(KeyPtr key_data, size_t key_size, V value)
     {
-        StringRef key{key_data.release(), key_size};
+        std::string_view key{key_data.release(), key_size};
         insertOrReplace(key, std::move(value), /*owns_key*/ true);
     }
 
@@ -283,7 +281,7 @@ public:
         else
         {
             map.erase(it->getKey());
-            arena.free(const_cast<char *>(list_itr->key.data), list_itr->key.size);
+            arena.free(const_cast<char *>(list_itr->key.data()), list_itr->key.size());
             list.erase(list_itr);
         }
 
@@ -296,12 +294,12 @@ public:
         return map.find(key) != map.end();
     }
 
-    const_iterator updateValue(StringRef key, ValueUpdater updater)
+    const_iterator updateValue(std::string_view key, ValueUpdater updater)
     {
         size_t hash_value = map.hash(key);
         auto it = map.find(key, hash_value);
         if (it == map.end())
-            throw Exception(ErrorCodes::LOGICAL_ERROR, "Could not find key: '{}'", key.toView());
+            throw Exception(ErrorCodes::LOGICAL_ERROR, "Could not find key: '{}'", key);
 
         auto list_itr = it->getMapped();
         uint64_t old_value_size = list_itr->value.sizeInBytes();
@@ -317,7 +315,7 @@ public:
             if (list_itr->getVersion() <= snapshot_up_to_version)
             {
                 auto elem_copy = list_itr->copyFromSnapshotNode();
-                updateDataSize(UPDATE, key.size, list_itr->value.sizeInBytes(), old_value_size, /*remove_old=*/true);
+                updateDataSize(UPDATE, key.size(), list_itr->value.sizeInBytes(), old_value_size, /*remove_old=*/true);
                 list_itr->setInactiveInMap();
                 snapshot_invalid_iters.push_back(list_itr);
                 updater(elem_copy.value);
@@ -341,11 +339,11 @@ public:
             ret = list_itr;
         }
 
-        updateDataSize(UPDATE, key.size, ret->value.sizeInBytes(), old_value_size, remove_old_size);
+        updateDataSize(UPDATE, key.size(), ret->value.sizeInBytes(), old_value_size, remove_old_size);
         return ret;
     }
 
-    const_iterator find(StringRef key) const
+    const_iterator find(std::string_view key) const
     {
         auto map_it = map.find(key);
         if (map_it != map.end())
@@ -355,11 +353,11 @@ public:
     }
 
 
-    const V & getValue(StringRef key) const
+    const V & getValue(std::string_view key) const
     {
         auto it = map.find(key);
         if (it == map.end())
-            throw Exception(ErrorCodes::LOGICAL_ERROR, "Could not find key: '{}'", key.toView());
+            throw Exception(ErrorCodes::LOGICAL_ERROR, "Could not find key: '{}'", key);
         return it->getMapped()->value;
     }
 
@@ -368,10 +366,10 @@ public:
         for (auto & itr : snapshot_invalid_iters)
         {
             if (itr->isActiveInMap())
-                throw Exception(ErrorCodes::LOGICAL_ERROR, "{} is not active in map", itr->key.toView());
-            updateDataSize(ERASE, itr->key.size, 0, itr->value.sizeInBytes(), /*remove_old=*/true);
+                throw Exception(ErrorCodes::LOGICAL_ERROR, "{} is not active in map", itr->key);
+            updateDataSize(ERASE, itr->key.size(), 0, itr->value.sizeInBytes(), /*remove_old=*/true);
             if (itr->getFreeKey())
-                arena.free(const_cast<char *>(itr->key.data), itr->key.size);
+                arena.free(const_cast<char *>(itr->key.data()), itr->key.size());
             list.erase(itr);
         }
         snapshot_invalid_iters.clear();
@@ -382,7 +380,7 @@ public:
         clearOutdatedNodes();
         map.clear();
         for (auto itr = list.begin(); itr != list.end(); ++itr)
-            arena.free(const_cast<char *>(itr->key.data), itr->key.size);
+            arena.free(const_cast<char *>(itr->key.data()), itr->key.size());
         list.clear();
         updateDataSize(CLEAR, 0, 0, 0);
     }
@@ -419,7 +417,7 @@ public:
         approximate_data_size = 0;
         for (auto & node : list)
         {
-            approximate_data_size += node.key.size;
+            approximate_data_size += node.key.size();
             approximate_data_size += node.value.sizeInBytes();
         }
     }

src/Coordination/tests/gtest_coordination_common.h

@@ -122,7 +122,7 @@ void addNode(Storage & storage, const std::string & path, const std::string & da
     auto child_it = storage.container.find(path);
     auto child_path = Coordination::getBaseNodeName(child_it->key);
     storage.container.updateValue(
-        Coordination::parentNodePath(StringRef{path}),
+        Coordination::parentNodePath(path),
         [&](auto & parent)
         {
             parent.addChild(child_path);

src/Coordination/tests/gtest_coordination_snapshot.cpp

@@ -539,7 +539,6 @@ TYPED_TEST(CoordinationTest, TestStorageSnapshotEqual)
 
 TYPED_TEST(CoordinationTest, TestStorageSnapshotBlockACL)
 {
-
     ChangelogDirTest test("./snapshots");
     this->setSnapshotDirectory("./snapshots");
 
@@ -551,7 +550,7 @@ TYPED_TEST(CoordinationTest, TestStorageSnapshotBlockACL)
     DB::KeeperSnapshotManager<Storage> manager(3, this->keeper_context, this->enable_compression);
 
     Storage storage(500, "", this->keeper_context);
-    static constexpr StringRef path = "/hello";
+    static constexpr std::string_view path = "/hello";
     static constexpr uint64_t acl_id = 42;
     addNode(storage, std::string{path}, "world", /*ephemeral_owner=*/0, acl_id);
     DB::KeeperStorageSnapshot<Storage> snapshot(&storage, 50);

src/Coordination/tests/gtest_coordination_storage.cpp

@@ -794,13 +794,14 @@ TYPED_TEST(CoordinationTest, TestListRequestTypes)
         return create_response.path_created;
     };
 
-    create_path(parentNodePath(StringRef{test_path}).toString(), false, false);
+    create_path(std::string{parentNodePath(test_path)}, false, false);
 
     static constexpr size_t persistent_num = 5;
     std::unordered_set<std::string> expected_persistent_children;
     for (size_t i = 0; i < persistent_num; ++i)
     {
-        expected_persistent_children.insert(getBaseNodeName(create_path(test_path, false)).toString());
+        auto created_path = create_path(test_path, false);
+        expected_persistent_children.insert(std::string{getBaseNodeName(created_path)});
     }
     ASSERT_EQ(expected_persistent_children.size(), persistent_num);
 
@@ -808,7 +809,8 @@ TYPED_TEST(CoordinationTest, TestListRequestTypes)
     std::unordered_set<std::string> expected_ephemeral_children;
     for (size_t i = 0; i < ephemeral_num; ++i)
     {
-        expected_ephemeral_children.insert(getBaseNodeName(create_path(test_path, true)).toString());
+        auto created_path = create_path(test_path, true);
+        expected_ephemeral_children.insert(std::string{getBaseNodeName(created_path)});
     }
     ASSERT_EQ(expected_ephemeral_children.size(), ephemeral_num);
 
@@ -816,7 +818,7 @@ TYPED_TEST(CoordinationTest, TestListRequestTypes)
     {
         const auto list_request = std::make_shared<ZooKeeperFilteredListRequest>();
         int new_zxid = ++zxid;
-        list_request->path = parentNodePath(StringRef{test_path}).toString();
+        list_request->path = std::string{parentNodePath(test_path)};
         list_request->list_request_type = list_request_type;
         storage.preprocessRequest(list_request, 1, 0, new_zxid);
         auto responses = storage.processRequest(list_request, 1, new_zxid);
@@ -991,11 +993,11 @@ TYPED_TEST(CoordinationTest, TestBlockACL)
     static constexpr int64_t session_id = 42;
     storage.committed_session_and_auth[session_id].push_back(KeeperStorageBase::AuthID{.scheme = "digest", .id = std::string{digest}});
     {
-        static constexpr StringRef path = "/test";
+        static constexpr std::string_view path = "/test";
 
         auto req_zxid = zxid++;
         const auto create_request = std::make_shared<ZooKeeperCreateRequest>();
-        create_request->path = path.toString();
+        create_request->path = path;
         create_request->acls = {Coordination::ACL{.permissions = Coordination::ACL::All, .scheme = "digest", .id = std::string{digest}}};
         storage.preprocessRequest(create_request, session_id, 0, req_zxid);
         auto acls = storage.uncommitted_state.getACLs(path);
@@ -1006,7 +1008,7 @@ TYPED_TEST(CoordinationTest, TestBlockACL)
 
         req_zxid = zxid++;
         const auto set_acl_request = std::make_shared<ZooKeeperSetACLRequest>();
-        set_acl_request->path = path.toString();
+        set_acl_request->path = path;
         set_acl_request->acls = {Coordination::ACL{.permissions = Coordination::ACL::All, .scheme = "digest", .id = std::string{new_digest}}};
         storage.preprocessRequest(set_acl_request, session_id, 0, req_zxid);
         acls = storage.uncommitted_state.getACLs(path);
@@ -1017,12 +1019,12 @@ TYPED_TEST(CoordinationTest, TestBlockACL)
     }
 
     {
-        static constexpr StringRef path = "/test_blocked_acl";
+        static constexpr std::string_view path = "/test_blocked_acl";
         this->keeper_context->setBlockACL(true);
 
         auto req_zxid = zxid++;
         const auto create_request = std::make_shared<ZooKeeperCreateRequest>();
-        create_request->path = path.toString();
+        create_request->path = path;
         create_request->acls = {Coordination::ACL{.permissions = Coordination::ACL::All, .scheme = "digest", .id = std::string{digest}}};
         storage.preprocessRequest(create_request, session_id, 0, req_zxid);
         auto acls = storage.uncommitted_state.getACLs(path);
@@ -1032,7 +1034,7 @@ TYPED_TEST(CoordinationTest, TestBlockACL)
 
         req_zxid = zxid++;
         const auto set_acl_request = std::make_shared<ZooKeeperSetACLRequest>();
-        set_acl_request->path = path.toString();
+        set_acl_request->path = path;
         set_acl_request->acls = {Coordination::ACL{.permissions = Coordination::ACL::All, .scheme = "digest", .id = std::string{new_digest}}};
         storage.preprocessRequest(set_acl_request, session_id, 0, req_zxid);
         acls = storage.uncommitted_state.getACLs(path);