From 0b6139bb788109ecf5092e569206da1588abeb5a Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Fri, 27 May 2022 12:19:26 +0800 Subject: [PATCH 1/8] Optimize blobstore gc when stat totalsize is 0 --- dbms/src/Storages/Page/V3/BlobStore.cpp | 11 +++++++++-- dbms/src/Storages/Page/V3/BlobStore.h | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/dbms/src/Storages/Page/V3/BlobStore.cpp b/dbms/src/Storages/Page/V3/BlobStore.cpp index dc5ed536f9e..ce237616b3b 100644 --- a/dbms/src/Storages/Page/V3/BlobStore.cpp +++ b/dbms/src/Storages/Page/V3/BlobStore.cpp @@ -914,7 +914,14 @@ std::vector BlobStore::getGCStats() // Avoid divide by zero if (right_margin == 0) { - LOG_FMT_TRACE(log, "Current blob is empty [blob_id={}, total size(all invalid)={}].", stat->id, stat->sm_total_size); + assert(stat->sm_valid_rate == 0); + LOG_FMT_TRACE(log, "Current blob is empty [blob_id={}, total size(all invalid)={}] [valid_rate={}].", stat->id, stat->sm_total_size, stat->sm_valid_rate); + // If current blob empty, the size of in disk blob may not empty + // So we need truncate current blob, and let it be reused. + auto blobfile = getBlobFile(stat->id); + LOG_FMT_TRACE(log, "Truncate empty blob file [blob_id={}] to 0.", stat->id, stat->sm_total_size, right_margin); + blobfile->truncate(0); + blobstore_gc_info.appendToTruncatedBlob(stat->id, stat->sm_valid_rate); continue; } @@ -1468,7 +1475,7 @@ void BlobStore::BlobStats::BlobStat::recalculateSpaceMap() const auto & [total_size, valid_size] = smap->getSizes(); sm_total_size = total_size; sm_valid_size = valid_size; - sm_valid_rate = valid_size * 1.0 / total_size; + sm_valid_rate = total_size == 0 ? 0 : valid_size * 1.0 / total_size; recalculateCapacity(); } diff --git a/dbms/src/Storages/Page/V3/BlobStore.h b/dbms/src/Storages/Page/V3/BlobStore.h index e527eb0f3bf..b83c1407994 100644 --- a/dbms/src/Storages/Page/V3/BlobStore.h +++ b/dbms/src/Storages/Page/V3/BlobStore.h @@ -107,7 +107,7 @@ class BlobStore : private Allocator UInt64 sm_max_caps = 0; UInt64 sm_total_size = 0; UInt64 sm_valid_size = 0; - double sm_valid_rate = 1.0; + double sm_valid_rate = 0; public: BlobStat(BlobFileId id_, SpaceMap::SpaceMapType sm_type, UInt64 sm_max_caps_, BlobStatType type_ = BlobStatType::NORMAL) From 2a1d161dc3ff7f67c6bc9515dbe50596e91c2ff3 Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Fri, 27 May 2022 14:05:21 +0800 Subject: [PATCH 2/8] Update dbms/src/Storages/Page/V3/BlobStore.cpp Co-authored-by: JaySon --- dbms/src/Storages/Page/V3/BlobStore.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/Storages/Page/V3/BlobStore.cpp b/dbms/src/Storages/Page/V3/BlobStore.cpp index ce237616b3b..fe074323ca1 100644 --- a/dbms/src/Storages/Page/V3/BlobStore.cpp +++ b/dbms/src/Storages/Page/V3/BlobStore.cpp @@ -920,7 +920,7 @@ std::vector BlobStore::getGCStats() // So we need truncate current blob, and let it be reused. auto blobfile = getBlobFile(stat->id); LOG_FMT_TRACE(log, "Truncate empty blob file [blob_id={}] to 0.", stat->id, stat->sm_total_size, right_margin); - blobfile->truncate(0); + blobfile->truncate(right_margin); blobstore_gc_info.appendToTruncatedBlob(stat->id, stat->sm_valid_rate); continue; } From 51401f15cec140ce41881b62df50f2ca605ef446 Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Fri, 27 May 2022 14:05:30 +0800 Subject: [PATCH 3/8] Update dbms/src/Storages/Page/V3/BlobStore.cpp Co-authored-by: JaySon --- dbms/src/Storages/Page/V3/BlobStore.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/Storages/Page/V3/BlobStore.cpp b/dbms/src/Storages/Page/V3/BlobStore.cpp index fe074323ca1..45e024838bd 100644 --- a/dbms/src/Storages/Page/V3/BlobStore.cpp +++ b/dbms/src/Storages/Page/V3/BlobStore.cpp @@ -919,7 +919,7 @@ std::vector BlobStore::getGCStats() // If current blob empty, the size of in disk blob may not empty // So we need truncate current blob, and let it be reused. auto blobfile = getBlobFile(stat->id); - LOG_FMT_TRACE(log, "Truncate empty blob file [blob_id={}] to 0.", stat->id, stat->sm_total_size, right_margin); + LOG_FMT_TRACE(log, "Truncate empty blob file [blob_id={}] to 0.", stat->id); blobfile->truncate(right_margin); blobstore_gc_info.appendToTruncatedBlob(stat->id, stat->sm_valid_rate); continue; From 581d09f31f839932ad0bc888785e5b736e42dcbd Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Fri, 27 May 2022 14:05:45 +0800 Subject: [PATCH 4/8] Update dbms/src/Storages/Page/V3/BlobStore.h Co-authored-by: JaySon --- dbms/src/Storages/Page/V3/BlobStore.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/Storages/Page/V3/BlobStore.h b/dbms/src/Storages/Page/V3/BlobStore.h index b83c1407994..16c775d0667 100644 --- a/dbms/src/Storages/Page/V3/BlobStore.h +++ b/dbms/src/Storages/Page/V3/BlobStore.h @@ -107,7 +107,7 @@ class BlobStore : private Allocator UInt64 sm_max_caps = 0; UInt64 sm_total_size = 0; UInt64 sm_valid_size = 0; - double sm_valid_rate = 0; + double sm_valid_rate = 0.0; public: BlobStat(BlobFileId id_, SpaceMap::SpaceMapType sm_type, UInt64 sm_max_caps_, BlobStatType type_ = BlobStatType::NORMAL) From 325519a99982ada21411a77a9d95af5c07da069e Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Fri, 27 May 2022 14:05:52 +0800 Subject: [PATCH 5/8] Update dbms/src/Storages/Page/V3/BlobStore.cpp Co-authored-by: JaySon --- dbms/src/Storages/Page/V3/BlobStore.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/Storages/Page/V3/BlobStore.cpp b/dbms/src/Storages/Page/V3/BlobStore.cpp index 45e024838bd..bc7f225b6f9 100644 --- a/dbms/src/Storages/Page/V3/BlobStore.cpp +++ b/dbms/src/Storages/Page/V3/BlobStore.cpp @@ -1475,7 +1475,7 @@ void BlobStore::BlobStats::BlobStat::recalculateSpaceMap() const auto & [total_size, valid_size] = smap->getSizes(); sm_total_size = total_size; sm_valid_size = valid_size; - sm_valid_rate = total_size == 0 ? 0 : valid_size * 1.0 / total_size; + sm_valid_rate = total_size == 0 ? 0.0 : valid_size * 1.0 / total_size; recalculateCapacity(); } From d8fcb781eb2b1ce96d23110e1d3843c73013e7ac Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Fri, 27 May 2022 15:59:01 +0800 Subject: [PATCH 6/8] change assert to exception --- dbms/src/Storages/Page/V3/BlobStore.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/dbms/src/Storages/Page/V3/BlobStore.cpp b/dbms/src/Storages/Page/V3/BlobStore.cpp index bc7f225b6f9..3c9dcf8f7b3 100644 --- a/dbms/src/Storages/Page/V3/BlobStore.cpp +++ b/dbms/src/Storages/Page/V3/BlobStore.cpp @@ -914,8 +914,16 @@ std::vector BlobStore::getGCStats() // Avoid divide by zero if (right_margin == 0) { - assert(stat->sm_valid_rate == 0); + if (stat->sm_valid_rate == 0) + { + throw Exception(fmt::format("Current blob is empty, but valid rate is not 0. [blob_id={}][valid_size={}][valid_rate={}]", + stat->id, + stat->sm_valid_size, + stat->sm_valid_rate)); + } + LOG_FMT_TRACE(log, "Current blob is empty [blob_id={}, total size(all invalid)={}] [valid_rate={}].", stat->id, stat->sm_total_size, stat->sm_valid_rate); + // If current blob empty, the size of in disk blob may not empty // So we need truncate current blob, and let it be reused. auto blobfile = getBlobFile(stat->id); From 214687906f7cfc565832cf9c3ec0209db9e6f0c1 Mon Sep 17 00:00:00 2001 From: Flowyi Date: Fri, 27 May 2022 16:22:06 +0800 Subject: [PATCH 7/8] Update dbms/src/Storages/Page/V3/BlobStore.cpp --- dbms/src/Storages/Page/V3/BlobStore.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/Storages/Page/V3/BlobStore.cpp b/dbms/src/Storages/Page/V3/BlobStore.cpp index 3c9dcf8f7b3..e5ca2905c47 100644 --- a/dbms/src/Storages/Page/V3/BlobStore.cpp +++ b/dbms/src/Storages/Page/V3/BlobStore.cpp @@ -914,7 +914,7 @@ std::vector BlobStore::getGCStats() // Avoid divide by zero if (right_margin == 0) { - if (stat->sm_valid_rate == 0) + if (unlikely(stat->sm_valid_rate == 0)) { throw Exception(fmt::format("Current blob is empty, but valid rate is not 0. [blob_id={}][valid_size={}][valid_rate={}]", stat->id, From 2b55f533b0a7c37336ec19d42635124c65519488 Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Fri, 27 May 2022 17:11:41 +0800 Subject: [PATCH 8/8] Update dbms/src/Storages/Page/V3/BlobStore.cpp Co-authored-by: Flowyi --- dbms/src/Storages/Page/V3/BlobStore.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/Storages/Page/V3/BlobStore.cpp b/dbms/src/Storages/Page/V3/BlobStore.cpp index e5ca2905c47..f03bc5bcf73 100644 --- a/dbms/src/Storages/Page/V3/BlobStore.cpp +++ b/dbms/src/Storages/Page/V3/BlobStore.cpp @@ -914,7 +914,7 @@ std::vector BlobStore::getGCStats() // Avoid divide by zero if (right_margin == 0) { - if (unlikely(stat->sm_valid_rate == 0)) + if (unlikely(stat->sm_valid_rate != 0)) { throw Exception(fmt::format("Current blob is empty, but valid rate is not 0. [blob_id={}][valid_size={}][valid_rate={}]", stat->id,