From 220e4dd25afe4b73acaa59dba30f3b895f00dab8 Mon Sep 17 00:00:00 2001 From: zhanglistar Date: Thu, 10 Feb 2022 17:13:46 +0800 Subject: [PATCH 1/2] Keeper: add version to SnapshotableHashTable --- src/Coordination/KeeperSnapshotManager.cpp | 10 +++--- src/Coordination/KeeperStorage.h | 5 +-- src/Coordination/SnapshotableHashTable.h | 34 ++++++++++--------- src/Coordination/tests/gtest_coordination.cpp | 13 +++---- 4 files changed, 34 insertions(+), 28 deletions(-) diff --git a/src/Coordination/KeeperSnapshotManager.cpp b/src/Coordination/KeeperSnapshotManager.cpp index 8d5df7c35e9..3d44d73cc3a 100644 --- a/src/Coordination/KeeperSnapshotManager.cpp +++ b/src/Coordination/KeeperSnapshotManager.cpp @@ -337,8 +337,9 @@ KeeperStorageSnapshot::KeeperStorageSnapshot(KeeperStorage * storage_, uint64_t , session_id(storage->session_id_counter) , cluster_config(cluster_config_) { - snapshot_container_size = storage->container.snapshotSize(); - storage->enableSnapshotMode(snapshot_container_size); + auto [size, ver] = storage->container.snapshotSizeWithVersion(); + snapshot_container_size = size; + storage->enableSnapshotMode(ver); begin = storage->getSnapshotIteratorBegin(); session_and_timeout = storage->getActiveSessions(); acl_map = storage->acl_map.getMapping(); @@ -351,8 +352,9 @@ KeeperStorageSnapshot::KeeperStorageSnapshot(KeeperStorage * storage_, const Sna , session_id(storage->session_id_counter) , cluster_config(cluster_config_) { - snapshot_container_size = storage->container.snapshotSize(); - storage->enableSnapshotMode(snapshot_container_size); + auto [size, ver] = storage->container.snapshotSizeWithVersion(); + snapshot_container_size = size; + storage->enableSnapshotMode(ver); begin = storage->getSnapshotIteratorBegin(); session_and_timeout = storage->getActiveSessions(); acl_map = storage->acl_map.getMapping(); diff --git a/src/Coordination/KeeperStorage.h b/src/Coordination/KeeperStorage.h index cbf33be61a0..ff62d64ac64 100644 --- a/src/Coordination/KeeperStorage.h +++ b/src/Coordination/KeeperStorage.h @@ -160,9 +160,10 @@ public: /// Set of methods for creating snapshots /// Turn on snapshot mode, so data inside Container is not deleted, but replaced with new version. - void enableSnapshotMode(size_t up_to_size) + void enableSnapshotMode(size_t up_to_version) { - container.enableSnapshotMode(up_to_size); + container.enableSnapshotMode(up_to_version); + } /// Turn off snapshot mode. diff --git a/src/Coordination/SnapshotableHashTable.h b/src/Coordination/SnapshotableHashTable.h index b1d72578530..507291780b3 100644 --- a/src/Coordination/SnapshotableHashTable.h +++ b/src/Coordination/SnapshotableHashTable.h @@ -16,6 +16,8 @@ struct ListNode StringRef key; V value; + /// Monotonically increasing verison info for snapshot + size_t version{0}; bool active_in_map{true}; bool free_key{false}; }; @@ -34,7 +36,8 @@ private: IndexMap map; bool snapshot_mode{false}; /// Allows to avoid additional copies in updateValue function - size_t snapshot_up_to_size = 0; + size_t current_version{0}; + size_t snapshot_up_to_version{0}; ArenaWithFreeLists arena; uint64_t approximate_data_size{0}; @@ -137,8 +140,8 @@ public: if (!it) { - ListElem elem{copyStringInArena(key), value, true}; - auto itr = list.insert(list.end(), elem); + ListElem elem{copyStringInArena(key), value, current_version}; + auto itr = list.insert(list.end(), std::move(elem)); bool inserted; map.emplace(itr->key, it, inserted, hash_value); assert(inserted); @@ -159,8 +162,8 @@ public: if (it == map.end()) { - ListElem elem{copyStringInArena(key), value, true}; - auto itr = list.insert(list.end(), elem); + ListElem elem{copyStringInArena(key), value, current_version}; + auto itr = list.insert(list.end(), std::move(elem)); bool inserted; map.emplace(itr->key, it, inserted, hash_value); assert(inserted); @@ -171,9 +174,9 @@ public: auto list_itr = it->getMapped(); if (snapshot_mode) { - ListElem elem{list_itr->key, value, true}; + ListElem elem{list_itr->key, value, current_version}; list_itr->active_in_map = false; - auto new_list_itr = list.insert(list.end(), elem); + auto new_list_itr = list.insert(list.end(), std::move(elem)); it->getMapped() = new_list_itr; } else @@ -230,13 +233,13 @@ public: /// We in snapshot mode but updating some node which is already more /// fresh than snapshot distance. So it will not participate in /// snapshot and we don't need to copy it. - size_t distance = std::distance(list.begin(), list_itr); - if (distance < snapshot_up_to_size) + if (snapshot_mode && list_itr->version <= snapshot_up_to_version) { auto elem_copy = *(list_itr); list_itr->active_in_map = false; updater(elem_copy.value); - auto itr = list.insert(list.end(), elem_copy); + elem_copy.version = current_version; + auto itr = list.insert(list.end(), std::move(elem_copy)); it->getMapped() = itr; ret = itr; } @@ -302,17 +305,16 @@ public: updateDataSize(CLEAR, 0, 0, 0); } - void enableSnapshotMode(size_t up_to_size) + void enableSnapshotMode(size_t version) { snapshot_mode = true; - snapshot_up_to_size = up_to_size; + snapshot_up_to_version = version; + ++current_version; } void disableSnapshotMode() { - snapshot_mode = false; - snapshot_up_to_size = 0; } size_t size() const @@ -320,9 +322,9 @@ public: return map.size(); } - size_t snapshotSize() const + std::pair snapshotSizeWithVersion() const { - return list.size(); + return std::make_pair(list.size(), current_version); } uint64_t getApproximateDataSize() const diff --git a/src/Coordination/tests/gtest_coordination.cpp b/src/Coordination/tests/gtest_coordination.cpp index 9c434ebb653..70a6d1cf2c5 100644 --- a/src/Coordination/tests/gtest_coordination.cpp +++ b/src/Coordination/tests/gtest_coordination.cpp @@ -865,7 +865,7 @@ TEST_P(CoordinationTest, SnapshotableHashMapTrySnapshot) EXPECT_FALSE(map_snp.insert("/hello", 145).second); map_snp.updateValue("/hello", [](IntNode & value) { value = 554; }); EXPECT_EQ(map_snp.getValue("/hello"), 554); - EXPECT_EQ(map_snp.snapshotSize(), 2); + EXPECT_EQ(map_snp.snapshotSizeWithVersion().first, 2); EXPECT_EQ(map_snp.size(), 1); auto itr = map_snp.begin(); @@ -884,7 +884,7 @@ TEST_P(CoordinationTest, SnapshotableHashMapTrySnapshot) } EXPECT_EQ(map_snp.getValue("/hello3"), 3); - EXPECT_EQ(map_snp.snapshotSize(), 7); + EXPECT_EQ(map_snp.snapshotSizeWithVersion().first, 7); EXPECT_EQ(map_snp.size(), 6); itr = std::next(map_snp.begin(), 2); for (size_t i = 0; i < 5; ++i) @@ -898,7 +898,7 @@ TEST_P(CoordinationTest, SnapshotableHashMapTrySnapshot) EXPECT_TRUE(map_snp.erase("/hello3")); EXPECT_TRUE(map_snp.erase("/hello2")); - EXPECT_EQ(map_snp.snapshotSize(), 7); + EXPECT_EQ(map_snp.snapshotSizeWithVersion().first, 7); EXPECT_EQ(map_snp.size(), 4); itr = std::next(map_snp.begin(), 2); for (size_t i = 0; i < 5; ++i) @@ -910,7 +910,7 @@ TEST_P(CoordinationTest, SnapshotableHashMapTrySnapshot) } map_snp.clearOutdatedNodes(); - EXPECT_EQ(map_snp.snapshotSize(), 4); + EXPECT_EQ(map_snp.snapshotSizeWithVersion().first, 4); EXPECT_EQ(map_snp.size(), 4); itr = map_snp.begin(); EXPECT_EQ(itr->key, "/hello"); @@ -1164,14 +1164,15 @@ TEST_P(CoordinationTest, TestStorageSnapshotMode) storage.container.erase("/hello_" + std::to_string(i)); } EXPECT_EQ(storage.container.size(), 26); - EXPECT_EQ(storage.container.snapshotSize(), 101); + EXPECT_EQ(storage.container.snapshotSizeWithVersion().first, 101); + EXPECT_EQ(storage.container.snapshotSizeWithVersion().second, 1); auto buf = manager.serializeSnapshotToBuffer(snapshot); manager.serializeSnapshotBufferToDisk(*buf, 50); } EXPECT_TRUE(fs::exists("./snapshots/snapshot_50.bin" + params.extension)); EXPECT_EQ(storage.container.size(), 26); storage.clearGarbageAfterSnapshot(); - EXPECT_EQ(storage.container.snapshotSize(), 26); + EXPECT_EQ(storage.container.snapshotSizeWithVersion().first, 26); for (size_t i = 0; i < 50; ++i) { if (i % 2 != 0) From baeea7c635e2426f2c21d06f5f8a2d693d97484a Mon Sep 17 00:00:00 2001 From: zhanglistar Date: Thu, 10 Feb 2022 17:37:49 +0800 Subject: [PATCH 2/2] Fix typo in SnapshotableHashTable.h --- src/Coordination/SnapshotableHashTable.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Coordination/SnapshotableHashTable.h b/src/Coordination/SnapshotableHashTable.h index 507291780b3..1c543d28a6d 100644 --- a/src/Coordination/SnapshotableHashTable.h +++ b/src/Coordination/SnapshotableHashTable.h @@ -16,7 +16,7 @@ struct ListNode StringRef key; V value; - /// Monotonically increasing verison info for snapshot + /// Monotonically increasing version info for snapshot size_t version{0}; bool active_in_map{true}; bool free_key{false};