From 1376fe6f11c8ee8bcff167022266061487a3f9a9 Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Thu, 17 Mar 2022 14:54:39 +0800 Subject: [PATCH 1/8] Fix max_caps may not been update after GC Signed-off-by: jiaqizho --- dbms/src/Storages/Page/V3/BlobStore.cpp | 43 ++++++++++++++++--- dbms/src/Storages/Page/V3/BlobStore.h | 10 ++++- dbms/src/Storages/Page/V3/spacemap/SpaceMap.h | 4 ++ 3 files changed, 49 insertions(+), 8 deletions(-) diff --git a/dbms/src/Storages/Page/V3/BlobStore.cpp b/dbms/src/Storages/Page/V3/BlobStore.cpp index 5145cc03a33..bbb7ed1717d 100644 --- a/dbms/src/Storages/Page/V3/BlobStore.cpp +++ b/dbms/src/Storages/Page/V3/BlobStore.cpp @@ -192,18 +192,36 @@ PageEntriesEdit BlobStore::write(DB::WriteBatch & wb, const WriteLimiterPtr & wr void BlobStore::remove(const PageEntriesV3 & del_entries) { + std::set blob_updated; for (const auto & entry : del_entries) { + blob_updated.insert(entry.file_id); // External page size is 0 if (entry.size == 0) { continue; - // throw Exception(fmt::format("Invaild entry. entry size 0. [id={}] [offset={}]", - // entry.file_id, - // entry.offset)); } removePosFromStats(entry.file_id, entry.offset, entry.size); } + + // After we remove postion of blob, we need recalculate the blob. + for (const auto & blob_id : blob_updated) + { + const auto & stat = blob_stats.blobIdToStat(blob_id, + /*restore_if_not_exist*/ false, + /*ignore_not_exist*/ true); + + // Some of blob may been removed. + // So if we can't use id find blob, just ignore it. + if (stat) + { + { + auto lock = stat->lock(); + stat->recalculateCapability(); + } + LOG_FMT_TRACE(log, "Blob begin to recalculate capability [blob_id={}]", blob_id); + } + } } std::pair BlobStore::getPosFromStats(size_t size) @@ -747,6 +765,7 @@ void BlobStore::BlobStats::restore() for (const auto & stat : stats_map) { stat->recalculateSpaceMap(); + stat->recalculateCapability(); max_restored_file_id = std::max(stat->id, max_restored_file_id); existing_file_ids.insert(stat->id); } @@ -863,7 +882,7 @@ std::pair BlobStore::BlobStats::chooseStat(size_t buf_s return std::make_pair(stat_ptr, INVALID_BLOBFILE_ID); } -BlobStatPtr BlobStore::BlobStats::blobIdToStat(BlobFileId file_id, bool restore_if_not_exist) +BlobStatPtr BlobStore::BlobStats::blobIdToStat(BlobFileId file_id, bool restore_if_not_exist, bool ignore_not_exist) { auto guard = lock(); for (auto & stat : stats_map) @@ -880,9 +899,14 @@ BlobStatPtr BlobStore::BlobStats::blobIdToStat(BlobFileId file_id, bool restore_ return createStatNotCheckingRoll(file_id, guard); } - throw Exception(fmt::format("Can't find BlobStat with [blob_id={}]", - file_id), - ErrorCodes::LOGICAL_ERROR); + if (!ignore_not_exist) + { + throw Exception(fmt::format("Can't find BlobStat with [blob_id={}]", + file_id), + ErrorCodes::LOGICAL_ERROR); + } + + return nullptr; } /********************* @@ -962,5 +986,10 @@ void BlobStore::BlobStats::BlobStat::recalculateSpaceMap() sm_valid_rate = valid_size * 1.0 / total_size; } +void BlobStore::BlobStats::BlobStat::recalculateCapability() +{ + sm_max_caps = smap->getMaxCapability(); +} + } // namespace PS::V3 } // namespace DB diff --git a/dbms/src/Storages/Page/V3/BlobStore.h b/dbms/src/Storages/Page/V3/BlobStore.h index 39fd3344f4f..37faaa5a314 100644 --- a/dbms/src/Storages/Page/V3/BlobStore.h +++ b/dbms/src/Storages/Page/V3/BlobStore.h @@ -110,6 +110,14 @@ class BlobStore : private Allocator * We still need to recalculate a `sm_total_size`/`sm_valid_size`/`sm_valid_rate`. */ void recalculateSpaceMap(); + + /** + * After GC we removed out of date data. + * But we can't calculate the `sm_max_cap`.It can only be updated through spacemap. + * + * Also after restore, we need call this method to recalculate the `sm_max_cap`. + */ + void recalculateCapability(); }; using BlobStatPtr = std::shared_ptr; @@ -143,7 +151,7 @@ class BlobStore : private Allocator */ std::pair chooseStat(size_t buf_size, UInt64 file_limit_size, const std::lock_guard &); - BlobStatPtr blobIdToStat(BlobFileId file_id, bool restore_if_not_exist = false); + BlobStatPtr blobIdToStat(BlobFileId file_id, bool restore_if_not_exist = false, bool ignore_not_exist = false); std::list getStats() const { diff --git a/dbms/src/Storages/Page/V3/spacemap/SpaceMap.h b/dbms/src/Storages/Page/V3/spacemap/SpaceMap.h index ac9d1db85b3..4b708f44a78 100644 --- a/dbms/src/Storages/Page/V3/spacemap/SpaceMap.h +++ b/dbms/src/Storages/Page/V3/spacemap/SpaceMap.h @@ -82,6 +82,10 @@ class SpaceMap */ virtual UInt64 getRightMargin() = 0; + /** + * Get the max of capability of space map. + */ + virtual UInt64 getMaxCapability() = 0; /** * Return the size of file and the size contains valid data. From a2bec1fcd242ca34808ddfd7d201f15775e512d2 Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Thu, 17 Mar 2022 14:57:56 +0800 Subject: [PATCH 2/8] missing files --- .../Page/V3/spacemap/SpaceMapRBTree.cpp | 21 +++++++++++++++++++ .../Page/V3/spacemap/SpaceMapRBTree.h | 2 ++ .../Page/V3/spacemap/SpaceMapSTDMap.h | 11 ++++++++++ .../Storages/Page/V3/tests/gtest_free_map.cpp | 19 +++++++++++++++++ 4 files changed, 53 insertions(+) diff --git a/dbms/src/Storages/Page/V3/spacemap/SpaceMapRBTree.cpp b/dbms/src/Storages/Page/V3/spacemap/SpaceMapRBTree.cpp index 7a40244cb1d..cdcba192230 100644 --- a/dbms/src/Storages/Page/V3/spacemap/SpaceMapRBTree.cpp +++ b/dbms/src/Storages/Page/V3/spacemap/SpaceMapRBTree.cpp @@ -650,6 +650,27 @@ std::pair RBTreeSpaceMap::searchInsertOffset(size_t size) return std::make_pair(offset, max_cap); } +UInt64 RBTreeSpaceMap::getMaxCapability() +{ + struct rb_node * node = nullptr; + struct SmapRbEntry * entry; + UInt64 max_cap = 0; + + node = rb_tree_first(&rb_tree->root); + if (node == nullptr) + { + return max_cap; + } + + for (; node != nullptr; node = rb_tree_next(node)) + { + entry = node_to_entry(node); + max_cap = std::max(max_cap, entry->count); + } + + return max_cap; +} + std::pair RBTreeSpaceMap::getSizes() const { struct rb_node * node = rb_tree_last(&rb_tree->root); diff --git a/dbms/src/Storages/Page/V3/spacemap/SpaceMapRBTree.h b/dbms/src/Storages/Page/V3/spacemap/SpaceMapRBTree.h index c3174892be2..9237867ac6b 100644 --- a/dbms/src/Storages/Page/V3/spacemap/SpaceMapRBTree.h +++ b/dbms/src/Storages/Page/V3/spacemap/SpaceMapRBTree.h @@ -28,6 +28,8 @@ class RBTreeSpaceMap std::pair searchInsertOffset(size_t size) override; + UInt64 getMaxCapability() override; + std::pair getSizes() const override; UInt64 getRightMargin() override; diff --git a/dbms/src/Storages/Page/V3/spacemap/SpaceMapSTDMap.h b/dbms/src/Storages/Page/V3/spacemap/SpaceMapSTDMap.h index 06598a8e1a3..0ce349978ea 100644 --- a/dbms/src/Storages/Page/V3/spacemap/SpaceMapSTDMap.h +++ b/dbms/src/Storages/Page/V3/spacemap/SpaceMapSTDMap.h @@ -243,6 +243,17 @@ class STDMapSpaceMap return std::make_pair(offset, hint_biggest_cap); } + UInt64 getMaxCapability() override + { + UInt64 max_cap = 0; + for (const auto & [start, size] : free_map) + { + (void)start; + max_cap = std::max(max_cap, size); + } + return max_cap; + } + bool markFreeImpl(UInt64 offset, size_t length) override { auto it = free_map.find(offset); diff --git a/dbms/src/Storages/Page/V3/tests/gtest_free_map.cpp b/dbms/src/Storages/Page/V3/tests/gtest_free_map.cpp index 4a18db234e7..0d9700ade7d 100644 --- a/dbms/src/Storages/Page/V3/tests/gtest_free_map.cpp +++ b/dbms/src/Storages/Page/V3/tests/gtest_free_map.cpp @@ -356,6 +356,25 @@ TEST_P(SpaceMapTest, TestGetSizes) } } + +TEST_P(SpaceMapTest, TestGetMaxCap) +{ + { + auto smap = SpaceMap::createSpaceMap(test_type, 0, 100); + ASSERT_TRUE(smap->markUsed(50, 10)); + ASSERT_TRUE(smap->markUsed(80, 10)); + + ASSERT_EQ(smap->getMaxCapability(), 50); + } + + { + auto smap = SpaceMap::createSpaceMap(test_type, 0, 100); + ASSERT_TRUE(smap->markUsed(0, 100)); + + ASSERT_EQ(smap->getMaxCapability(), 0); + } +} + INSTANTIATE_TEST_CASE_P( Type, SpaceMapTest, From 789b503cce0ceae853eff41dc2b85916815d4ba6 Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Thu, 17 Mar 2022 16:53:07 +0800 Subject: [PATCH 3/8] update Signed-off-by: jiaqizho --- dbms/src/Storages/Page/V3/BlobStore.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dbms/src/Storages/Page/V3/BlobStore.cpp b/dbms/src/Storages/Page/V3/BlobStore.cpp index bbb7ed1717d..8c1b376c9ac 100644 --- a/dbms/src/Storages/Page/V3/BlobStore.cpp +++ b/dbms/src/Storages/Page/V3/BlobStore.cpp @@ -866,10 +866,10 @@ std::pair BlobStore::BlobStats::chooseStat(size_t buf_s { if (!stat->isReadOnly() && stat->sm_max_caps >= buf_size - && stat->sm_total_size + buf_size < file_limit_size + // && stat->sm_total_size + buf_size < file_limit_size && stat->sm_valid_rate < smallest_valid_rate) { - smallest_valid_rate = stat->sm_valid_size; + smallest_valid_rate = stat->sm_valid_rate; stat_ptr = stat; } } From 986e9882dc25161c8ab0f976826986cb32bd46f8 Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Thu, 17 Mar 2022 18:23:12 +0800 Subject: [PATCH 4/8] update --- dbms/src/Storages/Page/V3/BlobStore.cpp | 4 ++-- dbms/src/Storages/Page/V3/spacemap/SpaceMap.h | 2 +- .../Storages/Page/V3/spacemap/SpaceMapRBTree.cpp | 11 +++++++++-- .../src/Storages/Page/V3/spacemap/SpaceMapRBTree.h | 2 +- .../src/Storages/Page/V3/spacemap/SpaceMapSTDMap.h | 14 +++++++++++--- 5 files changed, 24 insertions(+), 9 deletions(-) diff --git a/dbms/src/Storages/Page/V3/BlobStore.cpp b/dbms/src/Storages/Page/V3/BlobStore.cpp index 8c1b376c9ac..6ca44513578 100644 --- a/dbms/src/Storages/Page/V3/BlobStore.cpp +++ b/dbms/src/Storages/Page/V3/BlobStore.cpp @@ -851,7 +851,7 @@ void BlobStore::BlobStats::eraseStat(BlobFileId blob_file_id, const std::lock_gu eraseStat(std::move(stat), lock); } -std::pair BlobStore::BlobStats::chooseStat(size_t buf_size, UInt64 file_limit_size, const std::lock_guard &) +std::pair BlobStore::BlobStats::chooseStat(size_t buf_size, UInt64 /*file_limit_size*/, const std::lock_guard &) { BlobStatPtr stat_ptr = nullptr; double smallest_valid_rate = 2; @@ -988,7 +988,7 @@ void BlobStore::BlobStats::BlobStat::recalculateSpaceMap() void BlobStore::BlobStats::BlobStat::recalculateCapability() { - sm_max_caps = smap->getMaxCapability(); + sm_max_caps = smap->updateAccurateMaxCapacity(); } } // namespace PS::V3 diff --git a/dbms/src/Storages/Page/V3/spacemap/SpaceMap.h b/dbms/src/Storages/Page/V3/spacemap/SpaceMap.h index 4b708f44a78..914724e04b8 100644 --- a/dbms/src/Storages/Page/V3/spacemap/SpaceMap.h +++ b/dbms/src/Storages/Page/V3/spacemap/SpaceMap.h @@ -85,7 +85,7 @@ class SpaceMap /** * Get the max of capability of space map. */ - virtual UInt64 getMaxCapability() = 0; + virtual UInt64 updateAccurateMaxCapacity() = 0; /** * Return the size of file and the size contains valid data. diff --git a/dbms/src/Storages/Page/V3/spacemap/SpaceMapRBTree.cpp b/dbms/src/Storages/Page/V3/spacemap/SpaceMapRBTree.cpp index cdcba192230..6dc135065d4 100644 --- a/dbms/src/Storages/Page/V3/spacemap/SpaceMapRBTree.cpp +++ b/dbms/src/Storages/Page/V3/spacemap/SpaceMapRBTree.cpp @@ -650,10 +650,11 @@ std::pair RBTreeSpaceMap::searchInsertOffset(size_t size) return std::make_pair(offset, max_cap); } -UInt64 RBTreeSpaceMap::getMaxCapability() +UInt64 RBTreeSpaceMap::updateAccurateMaxCapacity() { struct rb_node * node = nullptr; struct SmapRbEntry * entry; + UInt64 max_offset = 0; UInt64 max_cap = 0; node = rb_tree_first(&rb_tree->root); @@ -665,9 +666,15 @@ UInt64 RBTreeSpaceMap::getMaxCapability() for (; node != nullptr; node = rb_tree_next(node)) { entry = node_to_entry(node); - max_cap = std::max(max_cap, entry->count); + if (entry->count > max_cap) + { + max_offset = entry->start; + max_cap = entry->count; + } } + biggest_range = max_offset; + biggest_cap = max_cap; return max_cap; } diff --git a/dbms/src/Storages/Page/V3/spacemap/SpaceMapRBTree.h b/dbms/src/Storages/Page/V3/spacemap/SpaceMapRBTree.h index 9237867ac6b..271b488d5c2 100644 --- a/dbms/src/Storages/Page/V3/spacemap/SpaceMapRBTree.h +++ b/dbms/src/Storages/Page/V3/spacemap/SpaceMapRBTree.h @@ -28,7 +28,7 @@ class RBTreeSpaceMap std::pair searchInsertOffset(size_t size) override; - UInt64 getMaxCapability() override; + UInt64 updateAccurateMaxCapacity() override; std::pair getSizes() const override; diff --git a/dbms/src/Storages/Page/V3/spacemap/SpaceMapSTDMap.h b/dbms/src/Storages/Page/V3/spacemap/SpaceMapSTDMap.h index 0ce349978ea..be5dfde0504 100644 --- a/dbms/src/Storages/Page/V3/spacemap/SpaceMapSTDMap.h +++ b/dbms/src/Storages/Page/V3/spacemap/SpaceMapSTDMap.h @@ -243,14 +243,22 @@ class STDMapSpaceMap return std::make_pair(offset, hint_biggest_cap); } - UInt64 getMaxCapability() override + UInt64 updateAccurateMaxCapacity() override { + UInt64 max_offset = 0; UInt64 max_cap = 0; + for (const auto & [start, size] : free_map) { - (void)start; - max_cap = std::max(max_cap, size); + if (size > max_cap) + { + max_cap = size; + max_offset = start; + } } + hint_biggest_offset = max_offset; + hint_biggest_cap = max_cap; + return max_cap; } From 6740397ebb65620f26737ad358cacd527c97a347 Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Fri, 18 Mar 2022 13:21:20 +0800 Subject: [PATCH 5/8] update Signed-off-by: jiaqizho --- dbms/src/Storages/Page/V3/BlobStore.cpp | 11 +++++------ dbms/src/Storages/Page/V3/BlobStore.h | 10 ++++------ dbms/src/Storages/Page/V3/spacemap/SpaceMap.h | 2 +- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/dbms/src/Storages/Page/V3/BlobStore.cpp b/dbms/src/Storages/Page/V3/BlobStore.cpp index 6ca44513578..cfababa4a5d 100644 --- a/dbms/src/Storages/Page/V3/BlobStore.cpp +++ b/dbms/src/Storages/Page/V3/BlobStore.cpp @@ -217,7 +217,7 @@ void BlobStore::remove(const PageEntriesV3 & del_entries) { { auto lock = stat->lock(); - stat->recalculateCapability(); + stat->recalculateCapacity(); } LOG_FMT_TRACE(log, "Blob begin to recalculate capability [blob_id={}]", blob_id); } @@ -231,7 +231,7 @@ std::pair BlobStore::getPosFromStats(size_t size) auto lock_stat = [size, this, &stat]() -> std::lock_guard { auto lock_stats = blob_stats.lock(); BlobFileId blob_file_id = INVALID_BLOBFILE_ID; - std::tie(stat, blob_file_id) = blob_stats.chooseStat(size, config.file_limit_size, lock_stats); + std::tie(stat, blob_file_id) = blob_stats.chooseStat(size, lock_stats); // No valid stat for puting data with `size`, create a new one if (stat == nullptr) @@ -765,7 +765,6 @@ void BlobStore::BlobStats::restore() for (const auto & stat : stats_map) { stat->recalculateSpaceMap(); - stat->recalculateCapability(); max_restored_file_id = std::max(stat->id, max_restored_file_id); existing_file_ids.insert(stat->id); } @@ -851,7 +850,7 @@ void BlobStore::BlobStats::eraseStat(BlobFileId blob_file_id, const std::lock_gu eraseStat(std::move(stat), lock); } -std::pair BlobStore::BlobStats::chooseStat(size_t buf_size, UInt64 /*file_limit_size*/, const std::lock_guard &) +std::pair BlobStore::BlobStats::chooseStat(size_t buf_size, const std::lock_guard &) { BlobStatPtr stat_ptr = nullptr; double smallest_valid_rate = 2; @@ -866,7 +865,6 @@ std::pair BlobStore::BlobStats::chooseStat(size_t buf_s { if (!stat->isReadOnly() && stat->sm_max_caps >= buf_size - // && stat->sm_total_size + buf_size < file_limit_size && stat->sm_valid_rate < smallest_valid_rate) { smallest_valid_rate = stat->sm_valid_rate; @@ -984,9 +982,10 @@ void BlobStore::BlobStats::BlobStat::recalculateSpaceMap() sm_total_size = total_size; sm_valid_size = valid_size; sm_valid_rate = valid_size * 1.0 / total_size; + recalculateCapacity(); } -void BlobStore::BlobStats::BlobStat::recalculateCapability() +void BlobStore::BlobStats::BlobStat::recalculateCapacity() { sm_max_caps = smap->updateAccurateMaxCapacity(); } diff --git a/dbms/src/Storages/Page/V3/BlobStore.h b/dbms/src/Storages/Page/V3/BlobStore.h index 37faaa5a314..166cfec71b1 100644 --- a/dbms/src/Storages/Page/V3/BlobStore.h +++ b/dbms/src/Storages/Page/V3/BlobStore.h @@ -112,12 +112,10 @@ class BlobStore : private Allocator void recalculateSpaceMap(); /** - * After GC we removed out of date data. - * But we can't calculate the `sm_max_cap`.It can only be updated through spacemap. - * - * Also after restore, we need call this method to recalculate the `sm_max_cap`. + * The `sm_max_cap` is not accurate after GC removes out-of-date data, or after restoring from disk. + * Caller should call this function to update the `sm_max_cap` so that we can reuse the space in this BlobStat. */ - void recalculateCapability(); + void recalculateCapacity(); }; using BlobStatPtr = std::shared_ptr; @@ -149,7 +147,7 @@ class BlobStore : private Allocator * The `INVALID_BLOBFILE_ID` means that you don't need create a new `BlobFile`. * */ - std::pair chooseStat(size_t buf_size, UInt64 file_limit_size, const std::lock_guard &); + std::pair chooseStat(size_t buf_size, const std::lock_guard &); BlobStatPtr blobIdToStat(BlobFileId file_id, bool restore_if_not_exist = false, bool ignore_not_exist = false); diff --git a/dbms/src/Storages/Page/V3/spacemap/SpaceMap.h b/dbms/src/Storages/Page/V3/spacemap/SpaceMap.h index 914724e04b8..f246ad126d2 100644 --- a/dbms/src/Storages/Page/V3/spacemap/SpaceMap.h +++ b/dbms/src/Storages/Page/V3/spacemap/SpaceMap.h @@ -83,7 +83,7 @@ class SpaceMap virtual UInt64 getRightMargin() = 0; /** - * Get the max of capability of space map. + * Get the accurate max capacity of the space map. */ virtual UInt64 updateAccurateMaxCapacity() = 0; From a7f9baba2b899f8e3b505d5f0535b3c0481d5db3 Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Fri, 18 Mar 2022 13:27:23 +0800 Subject: [PATCH 6/8] miss Signed-off-by: jiaqizho --- dbms/src/Storages/Page/V3/tests/gtest_blob_store.cpp | 10 +++++----- dbms/src/Storages/Page/V3/tests/gtest_free_map.cpp | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/dbms/src/Storages/Page/V3/tests/gtest_blob_store.cpp b/dbms/src/Storages/Page/V3/tests/gtest_blob_store.cpp index 842c05610e7..fa56eb8816a 100644 --- a/dbms/src/Storages/Page/V3/tests/gtest_blob_store.cpp +++ b/dbms/src/Storages/Page/V3/tests/gtest_blob_store.cpp @@ -123,17 +123,17 @@ TEST_F(BlobStoreStatsTest, testStat) BlobStats stats(logger, config); - std::tie(stat, blob_file_id) = stats.chooseStat(10, BLOBFILE_LIMIT_SIZE, stats.lock()); + std::tie(stat, blob_file_id) = stats.chooseStat(10, stats.lock()); ASSERT_EQ(blob_file_id, 1); ASSERT_FALSE(stat); // still 0 - std::tie(stat, blob_file_id) = stats.chooseStat(10, BLOBFILE_LIMIT_SIZE, stats.lock()); + std::tie(stat, blob_file_id) = stats.chooseStat(10, stats.lock()); ASSERT_EQ(blob_file_id, 1); ASSERT_FALSE(stat); stats.createStat(0, stats.lock()); - std::tie(stat, blob_file_id) = stats.chooseStat(10, BLOBFILE_LIMIT_SIZE, stats.lock()); + std::tie(stat, blob_file_id) = stats.chooseStat(10, stats.lock()); ASSERT_EQ(blob_file_id, INVALID_BLOBFILE_ID); ASSERT_TRUE(stat); @@ -210,7 +210,7 @@ TEST_F(BlobStoreStatsTest, testFullStats) ASSERT_LE(stat->sm_valid_rate, 1); // Won't choose full one - std::tie(stat, blob_file_id) = stats.chooseStat(100, BLOBFILE_LIMIT_SIZE, stats.lock()); + std::tie(stat, blob_file_id) = stats.chooseStat(100, stats.lock()); ASSERT_EQ(blob_file_id, 2); ASSERT_FALSE(stat); @@ -228,7 +228,7 @@ TEST_F(BlobStoreStatsTest, testFullStats) // Then choose stat , it should return the stat id 3 // Stat which id is 2 is full. - std::tie(stat, blob_file_id) = stats.chooseStat(100, BLOBFILE_LIMIT_SIZE, stats.lock()); + std::tie(stat, blob_file_id) = stats.chooseStat(100, stats.lock()); ASSERT_EQ(blob_file_id, 3); ASSERT_FALSE(stat); } diff --git a/dbms/src/Storages/Page/V3/tests/gtest_free_map.cpp b/dbms/src/Storages/Page/V3/tests/gtest_free_map.cpp index 0d9700ade7d..a518d7cf6f8 100644 --- a/dbms/src/Storages/Page/V3/tests/gtest_free_map.cpp +++ b/dbms/src/Storages/Page/V3/tests/gtest_free_map.cpp @@ -364,14 +364,14 @@ TEST_P(SpaceMapTest, TestGetMaxCap) ASSERT_TRUE(smap->markUsed(50, 10)); ASSERT_TRUE(smap->markUsed(80, 10)); - ASSERT_EQ(smap->getMaxCapability(), 50); + ASSERT_EQ(smap->updateAccurateMaxCapacity(), 50); } { auto smap = SpaceMap::createSpaceMap(test_type, 0, 100); ASSERT_TRUE(smap->markUsed(0, 100)); - ASSERT_EQ(smap->getMaxCapability(), 0); + ASSERT_EQ(smap->updateAccurateMaxCapacity(), 0); } } From 64005c6d8a6802713ccd46bc5d3e3d39f0b13d71 Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Mon, 21 Mar 2022 11:27:08 +0800 Subject: [PATCH 7/8] update --- dbms/src/Storages/Page/V3/BlobStore.cpp | 5 ++- .../Page/V3/spacemap/SpaceMapRBTree.cpp | 34 ++++++++++++++----- .../Page/V3/spacemap/SpaceMapSTDMap.h | 30 ++++++++++++---- .../Page/V3/tests/gtest_blob_store.cpp | 22 ++++++++++-- 4 files changed, 69 insertions(+), 22 deletions(-) diff --git a/dbms/src/Storages/Page/V3/BlobStore.cpp b/dbms/src/Storages/Page/V3/BlobStore.cpp index cfababa4a5d..2661c871618 100644 --- a/dbms/src/Storages/Page/V3/BlobStore.cpp +++ b/dbms/src/Storages/Page/V3/BlobStore.cpp @@ -215,11 +215,11 @@ void BlobStore::remove(const PageEntriesV3 & del_entries) // So if we can't use id find blob, just ignore it. if (stat) { + LOG_FMT_TRACE(log, "Blob begin to recalculate capability [blob_id={}]", blob_id); { auto lock = stat->lock(); stat->recalculateCapacity(); } - LOG_FMT_TRACE(log, "Blob begin to recalculate capability [blob_id={}]", blob_id); } } } @@ -761,13 +761,12 @@ void BlobStore::BlobStats::restoreByEntry(const PageEntryV3 & entry) void BlobStore::BlobStats::restore() { BlobFileId max_restored_file_id = 0; - std::set existing_file_ids; for (const auto & stat : stats_map) { stat->recalculateSpaceMap(); max_restored_file_id = std::max(stat->id, max_restored_file_id); - existing_file_ids.insert(stat->id); } + // restore `roll_id` roll_id = max_restored_file_id + 1; } diff --git a/dbms/src/Storages/Page/V3/spacemap/SpaceMapRBTree.cpp b/dbms/src/Storages/Page/V3/spacemap/SpaceMapRBTree.cpp index 6dc135065d4..d8a7475b920 100644 --- a/dbms/src/Storages/Page/V3/spacemap/SpaceMapRBTree.cpp +++ b/dbms/src/Storages/Page/V3/spacemap/SpaceMapRBTree.cpp @@ -688,18 +688,34 @@ std::pair RBTreeSpaceMap::getSizes() const } auto * entry = node_to_entry(node); - UInt64 total_size = entry->start - start; - UInt64 last_node_size = entry->count; - UInt64 valid_size = 0; - - for (node = rb_tree_first(&rb_tree->root); node != nullptr; node = rb_tree_next(node)) + if (entry->start + entry->count != end) { - entry = node_to_entry(node); - valid_size += entry->count; + UInt64 total_size = end; + UInt64 valid_size = 0; + for (node = rb_tree_first(&rb_tree->root); node != nullptr; node = rb_tree_next(node)) + { + entry = node_to_entry(node); + valid_size += entry->count; + } + + valid_size = total_size - valid_size; + return std::make_pair(total_size, valid_size); } - valid_size = total_size - (valid_size - last_node_size); + else + { + UInt64 total_size = entry->start - start; + UInt64 last_node_size = entry->count; + UInt64 valid_size = 0; - return std::make_pair(total_size, valid_size); + for (node = rb_tree_first(&rb_tree->root); node != nullptr; node = rb_tree_next(node)) + { + entry = node_to_entry(node); + valid_size += entry->count; + } + valid_size = total_size - (valid_size - last_node_size); + + return std::make_pair(total_size, valid_size); + } } UInt64 RBTreeSpaceMap::getRightMargin() diff --git a/dbms/src/Storages/Page/V3/spacemap/SpaceMapSTDMap.h b/dbms/src/Storages/Page/V3/spacemap/SpaceMapSTDMap.h index be5dfde0504..23cd4d4d071 100644 --- a/dbms/src/Storages/Page/V3/spacemap/SpaceMapSTDMap.h +++ b/dbms/src/Storages/Page/V3/spacemap/SpaceMapSTDMap.h @@ -64,17 +64,33 @@ class STDMapSpaceMap } const auto & last_free_block = free_map.rbegin(); - UInt64 total_size = last_free_block->first - start; - UInt64 last_free_block_size = last_free_block->second; - UInt64 valid_size = 0; - for (const auto & free_block : free_map) + if (last_free_block->first + last_free_block->second != end) { - valid_size += free_block.second; + UInt64 total_size = end; + UInt64 valid_size = 0; + for (const auto & free_block : free_map) + { + valid_size += free_block.second; + } + + valid_size = total_size - valid_size; + return std::make_pair(total_size, valid_size); } - valid_size = total_size - (valid_size - last_free_block_size); + else + { + UInt64 total_size = last_free_block->first - start; + UInt64 last_free_block_size = last_free_block->second; - return std::make_pair(total_size, valid_size); + UInt64 valid_size = 0; + for (const auto & free_block : free_map) + { + valid_size += free_block.second; + } + valid_size = total_size - (valid_size - last_free_block_size); + + return std::make_pair(total_size, valid_size); + } } UInt64 getRightMargin() override diff --git a/dbms/src/Storages/Page/V3/tests/gtest_blob_store.cpp b/dbms/src/Storages/Page/V3/tests/gtest_blob_store.cpp index fa56eb8816a..8af3b7e0052 100644 --- a/dbms/src/Storages/Page/V3/tests/gtest_blob_store.cpp +++ b/dbms/src/Storages/Page/V3/tests/gtest_blob_store.cpp @@ -257,6 +257,7 @@ TEST_F(BlobStoreTest, Restore) try { const auto file_provider = DB::tests::TiFlashTestEnv::getContext().getFileProvider(); + config.file_limit_size = 2560; auto blob_store = BlobStore(file_provider, path, config); BlobFileId file_id1 = 10; @@ -287,9 +288,24 @@ try blob_store.blob_stats.restore(); } - auto blob_need_gc = blob_store.getGCStats(); - ASSERT_EQ(blob_need_gc.size(), 1); - EXPECT_EQ(blob_need_gc[0], 12); + // check spacemap updated + { + for (const auto & stat : blob_store.blob_stats.getStats()) + { + if (stat->id == file_id1) + { + ASSERT_EQ(stat->sm_total_size, 2560); + ASSERT_EQ(stat->sm_valid_size, 640); + ASSERT_EQ(stat->sm_max_caps, 1024); + } + else if (stat->id == file_id2) + { + ASSERT_EQ(stat->sm_total_size, 2560); + ASSERT_EQ(stat->sm_valid_size, 512); + ASSERT_EQ(stat->sm_max_caps, 2048); + } + } + } } CATCH From 7ba4e7b8082584218850581c2469e70792e3f9b1 Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Mon, 21 Mar 2022 12:11:03 +0800 Subject: [PATCH 8/8] update --- dbms/src/Storages/Page/V3/spacemap/SpaceMapRBTree.cpp | 7 +++---- dbms/src/Storages/Page/V3/spacemap/SpaceMapSTDMap.h | 7 +++---- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/dbms/src/Storages/Page/V3/spacemap/SpaceMapRBTree.cpp b/dbms/src/Storages/Page/V3/spacemap/SpaceMapRBTree.cpp index d8a7475b920..fb151bd4c7a 100644 --- a/dbms/src/Storages/Page/V3/spacemap/SpaceMapRBTree.cpp +++ b/dbms/src/Storages/Page/V3/spacemap/SpaceMapRBTree.cpp @@ -690,15 +690,14 @@ std::pair RBTreeSpaceMap::getSizes() const auto * entry = node_to_entry(node); if (entry->start + entry->count != end) { - UInt64 total_size = end; - UInt64 valid_size = 0; + UInt64 total_size = end - start; + UInt64 valid_size = total_size; for (node = rb_tree_first(&rb_tree->root); node != nullptr; node = rb_tree_next(node)) { entry = node_to_entry(node); - valid_size += entry->count; + valid_size -= entry->count; } - valid_size = total_size - valid_size; return std::make_pair(total_size, valid_size); } else diff --git a/dbms/src/Storages/Page/V3/spacemap/SpaceMapSTDMap.h b/dbms/src/Storages/Page/V3/spacemap/SpaceMapSTDMap.h index 23cd4d4d071..7038eb7c36a 100644 --- a/dbms/src/Storages/Page/V3/spacemap/SpaceMapSTDMap.h +++ b/dbms/src/Storages/Page/V3/spacemap/SpaceMapSTDMap.h @@ -67,14 +67,13 @@ class STDMapSpaceMap if (last_free_block->first + last_free_block->second != end) { - UInt64 total_size = end; - UInt64 valid_size = 0; + UInt64 total_size = end - start; + UInt64 valid_size = total_size; for (const auto & free_block : free_map) { - valid_size += free_block.second; + valid_size -= free_block.second; } - valid_size = total_size - valid_size; return std::make_pair(total_size, valid_size); } else