From c300f5e68fa130626d66f00d5c7a47f36d8b3617 Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Sun, 12 Mar 2023 13:48:16 +0000 Subject: [PATCH 01/18] Cleanup: max_size --> max_size_in_bytes To disambiguate the threshold from the maximum number of cache entries. --- src/Common/CacheBase.h | 10 +++++----- src/Common/LRUCachePolicy.h | 31 ++++++++++++++++--------------- src/Common/SLRUCachePolicy.h | 30 +++++++++++++++--------------- 3 files changed, 36 insertions(+), 35 deletions(-) diff --git a/src/Common/CacheBase.h b/src/Common/CacheBase.h index 8145bdf95b5..fe0f2408f05 100644 --- a/src/Common/CacheBase.h +++ b/src/Common/CacheBase.h @@ -27,7 +27,7 @@ namespace ErrorCodes /// (default policy evicts entries which are not used for a long time). /// WeightFunction is a functor that takes Mapped as a parameter and returns "weight" (approximate size) /// of that value. -/// Cache starts to evict entries when their total weight exceeds max_size. +/// Cache starts to evict entries when their total weight exceeds max_size_in_bytes. /// Value weight should not change after insertion. template , typename WeightFunction = TrivialWeightFunction> class CacheBase @@ -37,7 +37,7 @@ public: using Mapped = TMapped; using MappedPtr = std::shared_ptr; - explicit CacheBase(size_t max_size, size_t max_elements_size = 0, String cache_policy_name = "", double size_ratio = 0.5) + explicit CacheBase(size_t max_size_in_bytes, size_t max_elements_size = 0, String cache_policy_name = "", double size_ratio = 0.5) { auto on_weight_loss_function = [&](size_t weight_loss) { onRemoveOverflowWeightLoss(weight_loss); }; @@ -47,12 +47,12 @@ public: if (cache_policy_name == "LRU") { using LRUPolicy = LRUCachePolicy; - cache_policy = std::make_unique(max_size, max_elements_size, on_weight_loss_function); + cache_policy = std::make_unique(max_size_in_bytes, max_elements_size, on_weight_loss_function); } else if (cache_policy_name == "SLRU") { using SLRUPolicy = SLRUCachePolicy; - cache_policy = std::make_unique(max_size, max_elements_size, size_ratio, on_weight_loss_function); + cache_policy = std::make_unique(max_size_in_bytes, max_elements_size, size_ratio, on_weight_loss_function); } else { @@ -175,7 +175,7 @@ public: } size_t maxSize() const - TSA_NO_THREAD_SAFETY_ANALYSIS // disabled because max_size of cache_policy is a constant parameter + TSA_NO_THREAD_SAFETY_ANALYSIS // disabled because max_size_in_bytes of cache_policy is a constant parameter { return cache_policy->maxSize(); } diff --git a/src/Common/LRUCachePolicy.h b/src/Common/LRUCachePolicy.h index 3c069eb276b..0202d0ae509 100644 --- a/src/Common/LRUCachePolicy.h +++ b/src/Common/LRUCachePolicy.h @@ -12,7 +12,7 @@ namespace DB /// Cache policy LRU evicts entries which are not used for a long time. /// WeightFunction is a functor that takes Mapped as a parameter and returns "weight" (approximate size) /// of that value. -/// Cache starts to evict entries when their total weight exceeds max_size. +/// Cache starts to evict entries when their total weight exceeds max_size_in_bytes. /// Value weight should not change after insertion. /// To work with the thread-safe implementation of this class use a class "CacheBase" with first parameter "LRU" /// and next parameters in the same order as in the constructor of the current class. @@ -27,18 +27,19 @@ public: using Base = ICachePolicy; using typename Base::OnWeightLossFunction; - /** Initialize LRUCachePolicy with max_size and max_elements_size. + /** Initialize LRUCachePolicy with max_size_in_bytes and max_elements_size. * max_elements_size == 0 means no elements size restrictions. */ - explicit LRUCachePolicy(size_t max_size_, size_t max_elements_size_ = 0, OnWeightLossFunction on_weight_loss_function_ = {}) - : max_size(std::max(static_cast(1), max_size_)), max_elements_size(max_elements_size_) + explicit LRUCachePolicy(size_t max_size_in_bytes_, size_t max_elements_size_ = 0, OnWeightLossFunction on_weight_loss_function_ = {}) + : max_size_in_bytes(std::max(static_cast(1), max_size_in_bytes_)) + , max_elements_size(max_elements_size_) { Base::on_weight_loss_function = on_weight_loss_function_; } size_t weight(std::lock_guard & /* cache_lock */) const override { - return current_size; + return current_size_in_bytes; } size_t count(std::lock_guard & /* cache_lock */) const override @@ -48,14 +49,14 @@ public: size_t maxSize() const override { - return max_size; + return max_size_in_bytes; } void reset(std::lock_guard & /* cache_lock */) override { queue.clear(); cells.clear(); - current_size = 0; + current_size_in_bytes = 0; } void remove(const Key & key, std::lock_guard & /* cache_lock */) override @@ -64,7 +65,7 @@ public: if (it == cells.end()) return; auto & cell = it->second; - current_size -= cell.size; + current_size_in_bytes -= cell.size; queue.erase(cell.queue_iterator); cells.erase(it); } @@ -107,13 +108,13 @@ public: } else { - current_size -= cell.size; + current_size_in_bytes -= cell.size; queue.splice(queue.end(), queue, cell.queue_iterator); } cell.value = mapped; cell.size = cell.value ? weight_function(*cell.value) : 0; - current_size += cell.size; + current_size_in_bytes += cell.size; removeOverflow(); } @@ -136,8 +137,8 @@ protected: Cells cells; /// Total weight of values. - size_t current_size = 0; - const size_t max_size; + size_t current_size_in_bytes = 0; + const size_t max_size_in_bytes; const size_t max_elements_size; WeightFunction weight_function; @@ -147,7 +148,7 @@ protected: size_t current_weight_lost = 0; size_t queue_size = cells.size(); - while ((current_size > max_size || (max_elements_size != 0 && queue_size > max_elements_size)) && (queue_size > 0)) + while ((current_size_in_bytes > max_size_in_bytes || (max_elements_size != 0 && queue_size > max_elements_size)) && (queue_size > 0)) { const Key & key = queue.front(); @@ -160,7 +161,7 @@ protected: const auto & cell = it->second; - current_size -= cell.size; + current_size_in_bytes -= cell.size; current_weight_lost += cell.size; cells.erase(it); @@ -170,7 +171,7 @@ protected: Base::on_weight_loss_function(current_weight_lost); - if (current_size > (1ull << 63)) + if (current_size_in_bytes > (1ull << 63)) { LOG_ERROR(&Poco::Logger::get("LRUCache"), "LRUCache became inconsistent. There must be a bug in it."); abort(); diff --git a/src/Common/SLRUCachePolicy.h b/src/Common/SLRUCachePolicy.h index e1d72aa630a..da268708729 100644 --- a/src/Common/SLRUCachePolicy.h +++ b/src/Common/SLRUCachePolicy.h @@ -14,7 +14,7 @@ namespace DB /// this policy protects entries which were used more then once from a sequential scan. /// WeightFunction is a functor that takes Mapped as a parameter and returns "weight" (approximate size) /// of that value. -/// Cache starts to evict entries when their total weight exceeds max_size. +/// Cache starts to evict entries when their total weight exceeds max_size_in_bytes. /// Value weight should not change after insertion. /// To work with the thread-safe implementation of this class use a class "CacheBase" with first parameter "SLRU" /// and next parameters in the same order as in the constructor of the current class. @@ -29,14 +29,14 @@ public: using Base = ICachePolicy; using typename Base::OnWeightLossFunction; - /** Initialize SLRUCachePolicy with max_size and max_protected_size. + /** Initialize SLRUCachePolicy with max_size_in_bytes and max_protected_size. * max_protected_size shows how many of the most frequently used entries will not be evicted after a sequential scan. * max_protected_size == 0 means that the default protected size is equal to half of the total max size. */ /// TODO: construct from special struct with cache policy parameters (also with max_protected_size). - SLRUCachePolicy(size_t max_size_, size_t max_elements_size_ = 0, double size_ratio = 0.5, OnWeightLossFunction on_weight_loss_function_ = {}) - : max_protected_size(static_cast(max_size_ * std::min(1.0, size_ratio))) - , max_size(max_size_) + SLRUCachePolicy(size_t max_size_in_bytes_, size_t max_elements_size_ = 0, double size_ratio = 0.5, OnWeightLossFunction on_weight_loss_function_ = {}) + : max_protected_size(static_cast(max_size_in_bytes_ * std::min(1.0, size_ratio))) + , max_size_in_bytes(max_size_in_bytes_) , max_elements_size(max_elements_size_) { Base::on_weight_loss_function = on_weight_loss_function_; @@ -44,7 +44,7 @@ public: size_t weight(std::lock_guard & /* cache_lock */) const override { - return current_size; + return current_size_in_bytes; } size_t count(std::lock_guard & /* cache_lock */) const override @@ -54,7 +54,7 @@ public: size_t maxSize() const override { - return max_size; + return max_size_in_bytes; } void reset(std::lock_guard & /* cache_lock */) override @@ -62,7 +62,7 @@ public: cells.clear(); probationary_queue.clear(); protected_queue.clear(); - current_size = 0; + current_size_in_bytes = 0; current_protected_size = 0; } @@ -72,7 +72,7 @@ public: if (it == cells.end()) return; auto & cell = it->second; - current_size -= cell.size; + current_size_in_bytes -= cell.size; if (cell.is_protected) { current_protected_size -= cell.size; @@ -129,7 +129,7 @@ public: } else { - current_size -= cell.size; + current_size_in_bytes -= cell.size; if (cell.is_protected) { current_protected_size -= cell.size; @@ -144,11 +144,11 @@ public: cell.value = mapped; cell.size = cell.value ? weight_function(*cell.value) : 0; - current_size += cell.size; + current_size_in_bytes += cell.size; current_protected_size += cell.is_protected ? cell.size : 0; removeOverflow(protected_queue, max_protected_size, current_protected_size, /*is_protected=*/true); - removeOverflow(probationary_queue, max_size, current_size, /*is_protected=*/false); + removeOverflow(probationary_queue, max_size_in_bytes, current_size_in_bytes, /*is_protected=*/false); } protected: @@ -171,9 +171,9 @@ protected: Cells cells; size_t current_protected_size = 0; - size_t current_size = 0; + size_t current_size_in_bytes = 0; const size_t max_protected_size; - const size_t max_size; + const size_t max_size_in_bytes; const size_t max_elements_size; WeightFunction weight_function; @@ -240,7 +240,7 @@ protected: Base::on_weight_loss_function(current_weight_lost); } - if (current_size > (1ull << 63)) + if (current_size_in_bytes > (1ull << 63)) { LOG_ERROR(&Poco::Logger::get("SLRUCache"), "SLRUCache became inconsistent. There must be a bug in it."); abort(); From 7d5bc0d8c9aadbf8e6a291cb013a2d0820b887e5 Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Sun, 12 Mar 2023 13:50:48 +0000 Subject: [PATCH 02/18] Cleanup: TrivialWeightFunction --> EqualWeightFunction A true "trival" weight function would return .size() of the cache entry. --- src/Common/CacheBase.h | 2 +- src/Common/ICachePolicy.h | 5 +++-- src/Common/LRUCachePolicy.h | 2 +- src/Common/SLRUCachePolicy.h | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/Common/CacheBase.h b/src/Common/CacheBase.h index fe0f2408f05..ba8609ae0d0 100644 --- a/src/Common/CacheBase.h +++ b/src/Common/CacheBase.h @@ -29,7 +29,7 @@ namespace ErrorCodes /// of that value. /// Cache starts to evict entries when their total weight exceeds max_size_in_bytes. /// Value weight should not change after insertion. -template , typename WeightFunction = TrivialWeightFunction> +template , typename WeightFunction = EqualWeightFunction> class CacheBase { public: diff --git a/src/Common/ICachePolicy.h b/src/Common/ICachePolicy.h index 4e5916f125e..56e27cf39bd 100644 --- a/src/Common/ICachePolicy.h +++ b/src/Common/ICachePolicy.h @@ -6,8 +6,9 @@ namespace DB { + template -struct TrivialWeightFunction +struct EqualWeightFunction { size_t operator()(const T &) const { @@ -15,7 +16,7 @@ struct TrivialWeightFunction } }; -template , typename WeightFunction = TrivialWeightFunction> +template , typename WeightFunction = EqualWeightFunction> class ICachePolicy { public: diff --git a/src/Common/LRUCachePolicy.h b/src/Common/LRUCachePolicy.h index 0202d0ae509..60587294b51 100644 --- a/src/Common/LRUCachePolicy.h +++ b/src/Common/LRUCachePolicy.h @@ -16,7 +16,7 @@ namespace DB /// Value weight should not change after insertion. /// To work with the thread-safe implementation of this class use a class "CacheBase" with first parameter "LRU" /// and next parameters in the same order as in the constructor of the current class. -template , typename WeightFunction = TrivialWeightFunction> +template , typename WeightFunction = EqualWeightFunction> class LRUCachePolicy : public ICachePolicy { public: diff --git a/src/Common/SLRUCachePolicy.h b/src/Common/SLRUCachePolicy.h index da268708729..debac3824bf 100644 --- a/src/Common/SLRUCachePolicy.h +++ b/src/Common/SLRUCachePolicy.h @@ -18,7 +18,7 @@ namespace DB /// Value weight should not change after insertion. /// To work with the thread-safe implementation of this class use a class "CacheBase" with first parameter "SLRU" /// and next parameters in the same order as in the constructor of the current class. -template , typename WeightFunction = TrivialWeightFunction> +template , typename WeightFunction = EqualWeightFunction> class SLRUCachePolicy : public ICachePolicy { public: From 39898fbe64ad235a952993881b9e10013def3f29 Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Sun, 12 Mar 2023 13:57:10 +0000 Subject: [PATCH 03/18] Cleanup: max_elements_size --> max_entries To disambiguate the maximum number of entries from the maximum byte size of an entry. --- src/Common/CacheBase.h | 8 +++----- src/Common/LRUCachePolicy.h | 12 ++++++------ src/Common/SLRUCachePolicy.h | 12 ++++++------ 3 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/Common/CacheBase.h b/src/Common/CacheBase.h index ba8609ae0d0..237bf664f6d 100644 --- a/src/Common/CacheBase.h +++ b/src/Common/CacheBase.h @@ -37,7 +37,7 @@ public: using Mapped = TMapped; using MappedPtr = std::shared_ptr; - explicit CacheBase(size_t max_size_in_bytes, size_t max_elements_size = 0, String cache_policy_name = "", double size_ratio = 0.5) + explicit CacheBase(size_t max_size_in_bytes, size_t max_entries = 0, String cache_policy_name = "", double size_ratio = 0.5) { auto on_weight_loss_function = [&](size_t weight_loss) { onRemoveOverflowWeightLoss(weight_loss); }; @@ -47,17 +47,15 @@ public: if (cache_policy_name == "LRU") { using LRUPolicy = LRUCachePolicy; - cache_policy = std::make_unique(max_size_in_bytes, max_elements_size, on_weight_loss_function); + cache_policy = std::make_unique(max_size_in_bytes, max_entries, on_weight_loss_function); } else if (cache_policy_name == "SLRU") { using SLRUPolicy = SLRUCachePolicy; - cache_policy = std::make_unique(max_size_in_bytes, max_elements_size, size_ratio, on_weight_loss_function); + cache_policy = std::make_unique(max_size_in_bytes, max_entries, size_ratio, on_weight_loss_function); } else - { throw Exception(ErrorCodes::BAD_ARGUMENTS, "Undeclared cache policy name: {}", cache_policy_name); - } } MappedPtr get(const Key & key) diff --git a/src/Common/LRUCachePolicy.h b/src/Common/LRUCachePolicy.h index 60587294b51..be473310445 100644 --- a/src/Common/LRUCachePolicy.h +++ b/src/Common/LRUCachePolicy.h @@ -27,12 +27,12 @@ public: using Base = ICachePolicy; using typename Base::OnWeightLossFunction; - /** Initialize LRUCachePolicy with max_size_in_bytes and max_elements_size. - * max_elements_size == 0 means no elements size restrictions. + /** Initialize LRUCachePolicy with max_size_in_bytes and max_entries. + * max_entries == 0 means no elements size restrictions. */ - explicit LRUCachePolicy(size_t max_size_in_bytes_, size_t max_elements_size_ = 0, OnWeightLossFunction on_weight_loss_function_ = {}) + explicit LRUCachePolicy(size_t max_size_in_bytes_, size_t max_entries_ = 0, OnWeightLossFunction on_weight_loss_function_ = {}) : max_size_in_bytes(std::max(static_cast(1), max_size_in_bytes_)) - , max_elements_size(max_elements_size_) + , max_entries(max_entries_) { Base::on_weight_loss_function = on_weight_loss_function_; } @@ -139,7 +139,7 @@ protected: /// Total weight of values. size_t current_size_in_bytes = 0; const size_t max_size_in_bytes; - const size_t max_elements_size; + const size_t max_entries; WeightFunction weight_function; @@ -148,7 +148,7 @@ protected: size_t current_weight_lost = 0; size_t queue_size = cells.size(); - while ((current_size_in_bytes > max_size_in_bytes || (max_elements_size != 0 && queue_size > max_elements_size)) && (queue_size > 0)) + while ((current_size_in_bytes > max_size_in_bytes || (max_entries != 0 && queue_size > max_entries)) && (queue_size > 0)) { const Key & key = queue.front(); diff --git a/src/Common/SLRUCachePolicy.h b/src/Common/SLRUCachePolicy.h index debac3824bf..cd97a39af06 100644 --- a/src/Common/SLRUCachePolicy.h +++ b/src/Common/SLRUCachePolicy.h @@ -34,10 +34,10 @@ public: * max_protected_size == 0 means that the default protected size is equal to half of the total max size. */ /// TODO: construct from special struct with cache policy parameters (also with max_protected_size). - SLRUCachePolicy(size_t max_size_in_bytes_, size_t max_elements_size_ = 0, double size_ratio = 0.5, OnWeightLossFunction on_weight_loss_function_ = {}) + explicit SLRUCachePolicy(size_t max_size_in_bytes_, size_t max_entries_ = 0, double size_ratio = 0.5, OnWeightLossFunction on_weight_loss_function_ = {}) : max_protected_size(static_cast(max_size_in_bytes_ * std::min(1.0, size_ratio))) , max_size_in_bytes(max_size_in_bytes_) - , max_elements_size(max_elements_size_) + , max_entries(max_entries_) { Base::on_weight_loss_function = on_weight_loss_function_; } @@ -174,7 +174,7 @@ protected: size_t current_size_in_bytes = 0; const size_t max_protected_size; const size_t max_size_in_bytes; - const size_t max_elements_size; + const size_t max_entries; WeightFunction weight_function; @@ -188,11 +188,11 @@ protected: { /// Check if after remove all elements from probationary part there will be no more than max elements /// in protected queue and weight of all protected elements will be less then max protected weight. - /// It's not possible to check only cells.size() > max_elements_size + /// It's not possible to check only cells.size() > max_entries /// because protected elements move to probationary part and still remain in cache. need_remove = [&]() { - return ((max_elements_size != 0 && cells.size() - probationary_queue.size() > max_elements_size) + return ((max_entries != 0 && cells.size() - probationary_queue.size() > max_entries) || (current_weight_size > max_weight_size)) && (queue_size > 0); }; } @@ -200,7 +200,7 @@ protected: { need_remove = [&]() { - return ((max_elements_size != 0 && cells.size() > max_elements_size) + return ((max_entries != 0 && cells.size() > max_entries) || (current_weight_size > max_weight_size)) && (queue_size > 0); }; } From 9f6cb98c61718fa7c4c65b2195b36a5143e81c47 Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Mon, 13 Mar 2023 07:02:52 +0000 Subject: [PATCH 04/18] Cleanup: Remove default parameters from (S)LRUCachePolicy Allows to add new mandatory parameters without accidentally breaking something. --- src/Common/LRUCachePolicy.h | 2 +- src/Common/SLRUCachePolicy.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Common/LRUCachePolicy.h b/src/Common/LRUCachePolicy.h index be473310445..1db7c5f2333 100644 --- a/src/Common/LRUCachePolicy.h +++ b/src/Common/LRUCachePolicy.h @@ -30,7 +30,7 @@ public: /** Initialize LRUCachePolicy with max_size_in_bytes and max_entries. * max_entries == 0 means no elements size restrictions. */ - explicit LRUCachePolicy(size_t max_size_in_bytes_, size_t max_entries_ = 0, OnWeightLossFunction on_weight_loss_function_ = {}) + LRUCachePolicy(size_t max_size_in_bytes_, size_t max_entries_, OnWeightLossFunction on_weight_loss_function_) : max_size_in_bytes(std::max(static_cast(1), max_size_in_bytes_)) , max_entries(max_entries_) { diff --git a/src/Common/SLRUCachePolicy.h b/src/Common/SLRUCachePolicy.h index cd97a39af06..04e2b8dc4c9 100644 --- a/src/Common/SLRUCachePolicy.h +++ b/src/Common/SLRUCachePolicy.h @@ -34,7 +34,7 @@ public: * max_protected_size == 0 means that the default protected size is equal to half of the total max size. */ /// TODO: construct from special struct with cache policy parameters (also with max_protected_size). - explicit SLRUCachePolicy(size_t max_size_in_bytes_, size_t max_entries_ = 0, double size_ratio = 0.5, OnWeightLossFunction on_weight_loss_function_ = {}) + SLRUCachePolicy(size_t max_size_in_bytes_, size_t max_entries_, double size_ratio, OnWeightLossFunction on_weight_loss_function_) : max_protected_size(static_cast(max_size_in_bytes_ * std::min(1.0, size_ratio))) , max_size_in_bytes(max_size_in_bytes_) , max_entries(max_entries_) From 614810e471d91f0de6015179e9f9d0168edfeef6 Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Mon, 13 Mar 2023 07:18:33 +0000 Subject: [PATCH 05/18] Cleanup: Fix file name typo --- src/Common/tests/{gtest_slru_cahce.cpp => gtest_slru_cache.cpp} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/Common/tests/{gtest_slru_cahce.cpp => gtest_slru_cache.cpp} (100%) diff --git a/src/Common/tests/gtest_slru_cahce.cpp b/src/Common/tests/gtest_slru_cache.cpp similarity index 100% rename from src/Common/tests/gtest_slru_cahce.cpp rename to src/Common/tests/gtest_slru_cache.cpp From d165a15c58014ee64b32c2d1c6e693518fc365bb Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Mon, 13 Mar 2023 07:28:02 +0000 Subject: [PATCH 06/18] Cleanup: Simplify some typedefs --- src/Common/ICachePolicy.h | 4 +--- src/Common/LRUCachePolicy.h | 9 +++------ src/Common/SLRUCachePolicy.h | 9 +++------ 3 files changed, 7 insertions(+), 15 deletions(-) diff --git a/src/Common/ICachePolicy.h b/src/Common/ICachePolicy.h index 56e27cf39bd..1cdf379dcfc 100644 --- a/src/Common/ICachePolicy.h +++ b/src/Common/ICachePolicy.h @@ -16,12 +16,10 @@ struct EqualWeightFunction } }; -template , typename WeightFunction = EqualWeightFunction> +template , typename WeightFunction = EqualWeightFunction> class ICachePolicy { public: - using Key = TKey; - using Mapped = TMapped; using MappedPtr = std::shared_ptr; using OnWeightLossFunction = std::function; diff --git a/src/Common/LRUCachePolicy.h b/src/Common/LRUCachePolicy.h index 1db7c5f2333..40625e4de79 100644 --- a/src/Common/LRUCachePolicy.h +++ b/src/Common/LRUCachePolicy.h @@ -16,15 +16,12 @@ namespace DB /// Value weight should not change after insertion. /// To work with the thread-safe implementation of this class use a class "CacheBase" with first parameter "LRU" /// and next parameters in the same order as in the constructor of the current class. -template , typename WeightFunction = EqualWeightFunction> -class LRUCachePolicy : public ICachePolicy +template , typename WeightFunction = EqualWeightFunction> +class LRUCachePolicy : public ICachePolicy { public: - using Key = TKey; - using Mapped = TMapped; using MappedPtr = std::shared_ptr; - - using Base = ICachePolicy; + using Base = ICachePolicy; using typename Base::OnWeightLossFunction; /** Initialize LRUCachePolicy with max_size_in_bytes and max_entries. diff --git a/src/Common/SLRUCachePolicy.h b/src/Common/SLRUCachePolicy.h index 04e2b8dc4c9..25ac8d75599 100644 --- a/src/Common/SLRUCachePolicy.h +++ b/src/Common/SLRUCachePolicy.h @@ -18,15 +18,12 @@ namespace DB /// Value weight should not change after insertion. /// To work with the thread-safe implementation of this class use a class "CacheBase" with first parameter "SLRU" /// and next parameters in the same order as in the constructor of the current class. -template , typename WeightFunction = EqualWeightFunction> -class SLRUCachePolicy : public ICachePolicy +template , typename WeightFunction = EqualWeightFunction> +class SLRUCachePolicy : public ICachePolicy { public: - using Key = TKey; - using Mapped = TMapped; using MappedPtr = std::shared_ptr; - - using Base = ICachePolicy; + using Base = ICachePolicy; using typename Base::OnWeightLossFunction; /** Initialize SLRUCachePolicy with max_size_in_bytes and max_protected_size. From 475415e42146cdeda774dddf53ea47b2583de773 Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Mon, 13 Mar 2023 07:33:41 +0000 Subject: [PATCH 07/18] Cleanup: Move on_weight_loss_function into concrete cache policies Makes ICachePolicy a pure abstract interface --- src/Common/ICachePolicy.h | 3 --- src/Common/LRUCachePolicy.h | 6 ++++-- src/Common/SLRUCachePolicy.h | 12 ++++++------ 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/Common/ICachePolicy.h b/src/Common/ICachePolicy.h index 1cdf379dcfc..208cf480576 100644 --- a/src/Common/ICachePolicy.h +++ b/src/Common/ICachePolicy.h @@ -33,9 +33,6 @@ public: virtual void set(const Key & key, const MappedPtr & mapped, std::lock_guard & /* cache_lock */) = 0; virtual ~ICachePolicy() = default; - -protected: - OnWeightLossFunction on_weight_loss_function = [](size_t) {}; }; } diff --git a/src/Common/LRUCachePolicy.h b/src/Common/LRUCachePolicy.h index 40625e4de79..f191f0b7d6b 100644 --- a/src/Common/LRUCachePolicy.h +++ b/src/Common/LRUCachePolicy.h @@ -21,6 +21,7 @@ class LRUCachePolicy : public ICachePolicy; + using Base = ICachePolicy; using typename Base::OnWeightLossFunction; @@ -30,8 +31,8 @@ public: LRUCachePolicy(size_t max_size_in_bytes_, size_t max_entries_, OnWeightLossFunction on_weight_loss_function_) : max_size_in_bytes(std::max(static_cast(1), max_size_in_bytes_)) , max_entries(max_entries_) + , on_weight_loss_function(on_weight_loss_function_) { - Base::on_weight_loss_function = on_weight_loss_function_; } size_t weight(std::lock_guard & /* cache_lock */) const override @@ -139,6 +140,7 @@ protected: const size_t max_entries; WeightFunction weight_function; + OnWeightLossFunction on_weight_loss_function; void removeOverflow() { @@ -166,7 +168,7 @@ protected: --queue_size; } - Base::on_weight_loss_function(current_weight_lost); + on_weight_loss_function(current_weight_lost); if (current_size_in_bytes > (1ull << 63)) { diff --git a/src/Common/SLRUCachePolicy.h b/src/Common/SLRUCachePolicy.h index 25ac8d75599..80d622eba3e 100644 --- a/src/Common/SLRUCachePolicy.h +++ b/src/Common/SLRUCachePolicy.h @@ -23,6 +23,7 @@ class SLRUCachePolicy : public ICachePolicy; + using Base = ICachePolicy; using typename Base::OnWeightLossFunction; @@ -35,9 +36,9 @@ public: : max_protected_size(static_cast(max_size_in_bytes_ * std::min(1.0, size_ratio))) , max_size_in_bytes(max_size_in_bytes_) , max_entries(max_entries_) - { - Base::on_weight_loss_function = on_weight_loss_function_; - } + , on_weight_loss_function(on_weight_loss_function_) + { + } size_t weight(std::lock_guard & /* cache_lock */) const override { @@ -174,6 +175,7 @@ protected: const size_t max_entries; WeightFunction weight_function; + OnWeightLossFunction on_weight_loss_function; void removeOverflow(SLRUQueue & queue, const size_t max_weight_size, size_t & current_weight_size, bool is_protected) { @@ -233,9 +235,7 @@ protected: } if (!is_protected) - { - Base::on_weight_loss_function(current_weight_lost); - } + on_weight_loss_function(current_weight_lost); if (current_size_in_bytes > (1ull << 63)) { From 657aa654462137f4c4c90d095501eb301ee4ee80 Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Mon, 13 Mar 2023 09:06:58 +0000 Subject: [PATCH 08/18] Cleanup: Untangle CacheBase's constructors This prepares for the custom TTLCachePolicy which users should not be able to configure explicitly. --- programs/local/LocalServer.cpp | 4 ++-- programs/server/Server.cpp | 4 ++-- src/Common/CacheBase.h | 22 +++++++++++++++++----- src/Common/tests/gtest_lru_cache.cpp | 12 ++++++------ src/Common/tests/gtest_slru_cache.cpp | 22 +++++++++++----------- src/IO/UncompressedCache.h | 7 +++++-- src/Interpreters/Context.cpp | 8 ++++---- src/Interpreters/Context.h | 4 ++-- src/Storages/MarkCache.h | 7 +++++-- 9 files changed, 54 insertions(+), 36 deletions(-) diff --git a/programs/local/LocalServer.cpp b/programs/local/LocalServer.cpp index 8e092bdf8e4..5768e744f94 100644 --- a/programs/local/LocalServer.cpp +++ b/programs/local/LocalServer.cpp @@ -600,13 +600,13 @@ void LocalServer::processConfig() String uncompressed_cache_policy = config().getString("uncompressed_cache_policy", ""); size_t uncompressed_cache_size = config().getUInt64("uncompressed_cache_size", 0); if (uncompressed_cache_size) - global_context->setUncompressedCache(uncompressed_cache_size, uncompressed_cache_policy); + global_context->setUncompressedCache(uncompressed_cache_policy, uncompressed_cache_size); /// Size of cache for marks (index of MergeTree family of tables). String mark_cache_policy = config().getString("mark_cache_policy", ""); size_t mark_cache_size = config().getUInt64("mark_cache_size", 5368709120); if (mark_cache_size) - global_context->setMarkCache(mark_cache_size, mark_cache_policy); + global_context->setMarkCache(mark_cache_policy, mark_cache_size); /// Size of cache for uncompressed blocks of MergeTree indices. Zero means disabled. size_t index_uncompressed_cache_size = config().getUInt64("index_uncompressed_cache_size", 0); diff --git a/programs/server/Server.cpp b/programs/server/Server.cpp index 57d361886d2..02aa88620aa 100644 --- a/programs/server/Server.cpp +++ b/programs/server/Server.cpp @@ -1459,7 +1459,7 @@ try LOG_INFO(log, "Uncompressed cache size was lowered to {} because the system has low amount of memory", formatReadableSizeWithBinarySuffix(uncompressed_cache_size)); } - global_context->setUncompressedCache(uncompressed_cache_size, uncompressed_cache_policy); + global_context->setUncompressedCache(uncompressed_cache_policy, uncompressed_cache_size); /// Load global settings from default_profile and system_profile. global_context->setDefaultProfiles(config()); @@ -1484,7 +1484,7 @@ try LOG_INFO(log, "Mark cache size was lowered to {} because the system has low amount of memory", formatReadableSizeWithBinarySuffix(mark_cache_size)); } - global_context->setMarkCache(mark_cache_size, mark_cache_policy); + global_context->setMarkCache(mark_cache_policy, mark_cache_size); if (server_settings.index_uncompressed_cache_size) global_context->setIndexUncompressedCache(server_settings.index_uncompressed_cache_size); diff --git a/src/Common/CacheBase.h b/src/Common/CacheBase.h index 237bf664f6d..eb537c22834 100644 --- a/src/Common/CacheBase.h +++ b/src/Common/CacheBase.h @@ -37,12 +37,21 @@ public: using Mapped = TMapped; using MappedPtr = std::shared_ptr; - explicit CacheBase(size_t max_size_in_bytes, size_t max_entries = 0, String cache_policy_name = "", double size_ratio = 0.5) + /// Use this ctor if you don't care about the internal cache policy. + explicit CacheBase(size_t max_size_in_bytes, size_t max_entries = 0, double size_ratio = 0.5) + : CacheBase("SLRU", max_size_in_bytes, max_entries, size_ratio) + { + } + + /// Use this ctor if you want the user to configure the cache policy via some setting. Supports only general-purpose policies LRU and SLRU. + explicit CacheBase(std::string_view cache_policy_name, size_t max_size_in_bytes, size_t max_entries = 0, double size_ratio = 0.5) { auto on_weight_loss_function = [&](size_t weight_loss) { onRemoveOverflowWeightLoss(weight_loss); }; + static constexpr std::string_view default_cache_policy = "SLRU"; + if (cache_policy_name.empty()) - cache_policy_name = default_cache_policy_name; + cache_policy_name = default_cache_policy; if (cache_policy_name == "LRU") { @@ -55,9 +64,14 @@ public: cache_policy = std::make_unique(max_size_in_bytes, max_entries, size_ratio, on_weight_loss_function); } else - throw Exception(ErrorCodes::BAD_ARGUMENTS, "Undeclared cache policy name: {}", cache_policy_name); + throw Exception(ErrorCodes::BAD_ARGUMENTS, "Unknown cache policy name: {}", cache_policy_name); } + /// Use this ctor to provide an arbitrary cache policy. + explicit CacheBase(std::unique_ptr> cache_policy_) + : cache_policy(std::move(cache_policy_)) + {} + MappedPtr get(const Key & key) { std::lock_guard lock(mutex); @@ -188,8 +202,6 @@ private: std::unique_ptr cache_policy TSA_GUARDED_BY(mutex); - inline static const String default_cache_policy_name = "SLRU"; - std::atomic hits{0}; std::atomic misses{0}; diff --git a/src/Common/tests/gtest_lru_cache.cpp b/src/Common/tests/gtest_lru_cache.cpp index f74d1eb9464..9a2cb354bd5 100644 --- a/src/Common/tests/gtest_lru_cache.cpp +++ b/src/Common/tests/gtest_lru_cache.cpp @@ -6,7 +6,7 @@ TEST(LRUCache, set) { using SimpleCacheBase = DB::CacheBase; - auto lru_cache = SimpleCacheBase(/*max_size*/ 10, /*max_elements_size*/ 10, "LRU"); + auto lru_cache = SimpleCacheBase("LRU", /*max_size*/ 10, /*max_elements_size*/ 10); lru_cache.set(1, std::make_shared(2)); lru_cache.set(2, std::make_shared(3)); @@ -19,7 +19,7 @@ TEST(LRUCache, set) TEST(LRUCache, update) { using SimpleCacheBase = DB::CacheBase; - auto lru_cache = SimpleCacheBase(/*max_size*/ 10, /*max_elements_size*/ 10, "LRU"); + auto lru_cache = SimpleCacheBase("LRU", /*max_size*/ 10, /*max_elements_size*/ 10); lru_cache.set(1, std::make_shared(2)); lru_cache.set(1, std::make_shared(3)); auto val = lru_cache.get(1); @@ -30,7 +30,7 @@ TEST(LRUCache, update) TEST(LRUCache, get) { using SimpleCacheBase = DB::CacheBase; - auto lru_cache = SimpleCacheBase(/*max_size*/ 10, /*max_elements_size*/ 10, "LRU"); + auto lru_cache = SimpleCacheBase("LRU", /*max_size*/ 10, /*max_elements_size*/ 10); lru_cache.set(1, std::make_shared(2)); lru_cache.set(2, std::make_shared(3)); SimpleCacheBase::MappedPtr value = lru_cache.get(1); @@ -50,7 +50,7 @@ struct ValueWeight TEST(LRUCache, evictOnSize) { using SimpleCacheBase = DB::CacheBase; - auto lru_cache = SimpleCacheBase(/*max_size*/ 20, /*max_elements_size*/ 3, "LRU"); + auto lru_cache = SimpleCacheBase("LRU", /*max_size*/ 20, /*max_elements_size*/ 3); lru_cache.set(1, std::make_shared(2)); lru_cache.set(2, std::make_shared(3)); lru_cache.set(3, std::make_shared(4)); @@ -66,7 +66,7 @@ TEST(LRUCache, evictOnSize) TEST(LRUCache, evictOnWeight) { using SimpleCacheBase = DB::CacheBase, ValueWeight>; - auto lru_cache = SimpleCacheBase(/*max_size*/ 10, /*max_elements_size*/ 10, "LRU"); + auto lru_cache = SimpleCacheBase("LRU", /*max_size*/ 10, /*max_elements_size*/ 10); lru_cache.set(1, std::make_shared(2)); lru_cache.set(2, std::make_shared(3)); lru_cache.set(3, std::make_shared(4)); @@ -87,7 +87,7 @@ TEST(LRUCache, evictOnWeight) TEST(LRUCache, getOrSet) { using SimpleCacheBase = DB::CacheBase, ValueWeight>; - auto lru_cache = SimpleCacheBase(/*max_size*/ 10, /*max_elements_size*/ 10, "LRU"); + auto lru_cache = SimpleCacheBase("LRU", /*max_size*/ 10, /*max_elements_size*/ 10); size_t x = 10; auto load_func = [&] { return std::make_shared(x); }; auto [value, loaded] = lru_cache.getOrSet(1, load_func); diff --git a/src/Common/tests/gtest_slru_cache.cpp b/src/Common/tests/gtest_slru_cache.cpp index 66df0dbec77..f7ae9f9b16e 100644 --- a/src/Common/tests/gtest_slru_cache.cpp +++ b/src/Common/tests/gtest_slru_cache.cpp @@ -6,7 +6,7 @@ TEST(SLRUCache, set) { using SimpleCacheBase = DB::CacheBase; - auto slru_cache = SimpleCacheBase(/*max_size=*/10, /*max_elements_size=*/0, "SLRU", /*size_ratio*/0.5); + auto slru_cache = SimpleCacheBase("SLRU", /*max_size=*/10, /*max_elements_size=*/0, /*size_ratio*/0.5); slru_cache.set(1, std::make_shared(2)); slru_cache.set(2, std::make_shared(3)); @@ -19,7 +19,7 @@ TEST(SLRUCache, set) TEST(SLRUCache, update) { using SimpleCacheBase = DB::CacheBase; - auto slru_cache = SimpleCacheBase(/*max_size=*/10, /*max_elements_size=*/0, "SLRU", /*size_ratio*/0.5); + auto slru_cache = SimpleCacheBase("SLRU", /*max_size=*/10, /*max_elements_size=*/0, /*size_ratio*/0.5); slru_cache.set(1, std::make_shared(2)); slru_cache.set(1, std::make_shared(3)); @@ -31,7 +31,7 @@ TEST(SLRUCache, update) TEST(SLRUCache, get) { using SimpleCacheBase = DB::CacheBase; - auto slru_cache = SimpleCacheBase(/*max_size=*/10, /*max_elements_size=*/0, "SLRU", /*size_ratio*/0.5); + auto slru_cache = SimpleCacheBase("SLRU", /*max_size=*/10, /*max_elements_size=*/0, /*size_ratio*/0.5); slru_cache.set(1, std::make_shared(2)); slru_cache.set(2, std::make_shared(3)); @@ -47,7 +47,7 @@ TEST(SLRUCache, get) TEST(SLRUCache, remove) { using SimpleCacheBase = DB::CacheBase; - auto slru_cache = SimpleCacheBase(/*max_size=*/10, /*max_elements_size=*/0, "SLRU", /*size_ratio*/0.5); + auto slru_cache = SimpleCacheBase("SLRU", /*max_size=*/10, /*max_elements_size=*/0, /*size_ratio*/0.5); slru_cache.set(1, std::make_shared(2)); slru_cache.set(2, std::make_shared(3)); @@ -63,7 +63,7 @@ TEST(SLRUCache, remove) TEST(SLRUCache, removeFromProtected) { using SimpleCacheBase = DB::CacheBase; - auto slru_cache = SimpleCacheBase(/*max_size=*/2, /*max_elements_size=*/0, "SLRU", /*size_ratio*/0.5); + auto slru_cache = SimpleCacheBase("SLRU", /*max_size=*/2, /*max_elements_size=*/0, /*size_ratio*/0.5); slru_cache.set(1, std::make_shared(2)); slru_cache.set(1, std::make_shared(3)); @@ -96,7 +96,7 @@ TEST(SLRUCache, removeFromProtected) TEST(SLRUCache, reset) { using SimpleCacheBase = DB::CacheBase; - auto slru_cache = SimpleCacheBase(/*max_size=*/10, /*max_elements_size=*/0, "SLRU", /*size_ratio*/0.5); + auto slru_cache = SimpleCacheBase("SLRU", /*max_size=*/10, /*max_elements_size=*/0, /*size_ratio*/0.5); slru_cache.set(1, std::make_shared(2)); slru_cache.set(2, std::make_shared(3)); @@ -119,7 +119,7 @@ struct ValueWeight TEST(SLRUCache, evictOnElements) { using SimpleCacheBase = DB::CacheBase, ValueWeight>; - auto slru_cache = SimpleCacheBase(/*max_size=*/10, /*max_elements_size=*/1, "SLRU", /*size_ratio*/0.5); + auto slru_cache = SimpleCacheBase(/*max_size=*/10, /*max_elements_size=*/1, /*size_ratio*/0.5); slru_cache.set(1, std::make_shared(2)); slru_cache.set(2, std::make_shared(3)); @@ -140,7 +140,7 @@ TEST(SLRUCache, evictOnElements) TEST(SLRUCache, evictOnWeight) { using SimpleCacheBase = DB::CacheBase, ValueWeight>; - auto slru_cache = SimpleCacheBase(/*max_size=*/10, /*max_elements_size=*/0, "SLRU", /*size_ratio*/0.5); + auto slru_cache = SimpleCacheBase(/*max_size=*/10, /*max_elements_size=*/0, /*size_ratio*/0.5); slru_cache.set(1, std::make_shared(2)); slru_cache.set(2, std::make_shared(3)); slru_cache.set(3, std::make_shared(4)); @@ -161,7 +161,7 @@ TEST(SLRUCache, evictOnWeight) TEST(SLRUCache, evictFromProtectedPart) { using SimpleCacheBase = DB::CacheBase, ValueWeight>; - auto slru_cache = SimpleCacheBase(/*max_size=*/10, /*max_elements_size=*/0, "SLRU", /*size_ratio*/0.5); + auto slru_cache = SimpleCacheBase("SLRU", /*max_size=*/10, /*max_elements_size=*/0, /*size_ratio*/0.5); slru_cache.set(1, std::make_shared(2)); slru_cache.set(1, std::make_shared(2)); @@ -177,7 +177,7 @@ TEST(SLRUCache, evictFromProtectedPart) TEST(SLRUCache, evictStreamProtected) { using SimpleCacheBase = DB::CacheBase, ValueWeight>; - auto slru_cache = SimpleCacheBase(/*max_size=*/10, /*max_elements_size=*/0, "SLRU", /*size_ratio*/0.5); + auto slru_cache = SimpleCacheBase("SLRU", /*max_size=*/10, /*max_elements_size=*/0, /*size_ratio*/0.5); slru_cache.set(1, std::make_shared(2)); slru_cache.set(1, std::make_shared(2)); @@ -201,7 +201,7 @@ TEST(SLRUCache, evictStreamProtected) TEST(SLRUCache, getOrSet) { using SimpleCacheBase = DB::CacheBase, ValueWeight>; - auto slru_cache = SimpleCacheBase(/*max_size=*/10, /*max_elements_size=*/0, "SLRU", /*size_ratio*/0.5); + auto slru_cache = SimpleCacheBase("SLRU", /*max_size=*/10, /*max_elements_size=*/0, /*size_ratio*/0.5); size_t x = 5; auto load_func = [&] { return std::make_shared(x); }; auto [value, loaded] = slru_cache.getOrSet(1, load_func); diff --git a/src/IO/UncompressedCache.h b/src/IO/UncompressedCache.h index 3d1c907d364..2e654b27ed7 100644 --- a/src/IO/UncompressedCache.h +++ b/src/IO/UncompressedCache.h @@ -42,8 +42,11 @@ private: using Base = CacheBase; public: - explicit UncompressedCache(size_t max_size_in_bytes, const String & uncompressed_cache_policy = "") - : Base(max_size_in_bytes, 0, uncompressed_cache_policy) {} + explicit UncompressedCache(size_t max_size_in_bytes) + : Base(max_size_in_bytes) {} + + UncompressedCache(const String & uncompressed_cache_policy, size_t max_size_in_bytes) + : Base(uncompressed_cache_policy, max_size_in_bytes) {} /// Calculate key from path to file and offset. static UInt128 hash(const String & path_to_file, size_t offset) diff --git a/src/Interpreters/Context.cpp b/src/Interpreters/Context.cpp index cf1d5203bf7..0a2348d8749 100644 --- a/src/Interpreters/Context.cpp +++ b/src/Interpreters/Context.cpp @@ -1946,14 +1946,14 @@ QueryStatusPtr Context::getProcessListElement() const } -void Context::setUncompressedCache(size_t max_size_in_bytes, const String & uncompressed_cache_policy) +void Context::setUncompressedCache(const String & uncompressed_cache_policy, size_t max_size_in_bytes) { auto lock = getLock(); if (shared->uncompressed_cache) throw Exception(ErrorCodes::LOGICAL_ERROR, "Uncompressed cache has been already created."); - shared->uncompressed_cache = std::make_shared(max_size_in_bytes, uncompressed_cache_policy); + shared->uncompressed_cache = std::make_shared(uncompressed_cache_policy, max_size_in_bytes); } @@ -1972,14 +1972,14 @@ void Context::dropUncompressedCache() const } -void Context::setMarkCache(size_t cache_size_in_bytes, const String & mark_cache_policy) +void Context::setMarkCache(const String & mark_cache_policy, size_t cache_size_in_bytes) { auto lock = getLock(); if (shared->mark_cache) throw Exception(ErrorCodes::LOGICAL_ERROR, "Mark cache has been already created."); - shared->mark_cache = std::make_shared(cache_size_in_bytes, mark_cache_policy); + shared->mark_cache = std::make_shared(mark_cache_policy, cache_size_in_bytes); } MarkCachePtr Context::getMarkCache() const diff --git a/src/Interpreters/Context.h b/src/Interpreters/Context.h index 67594a41459..bbfbd4defdc 100644 --- a/src/Interpreters/Context.h +++ b/src/Interpreters/Context.h @@ -861,12 +861,12 @@ public: void setSystemZooKeeperLogAfterInitializationIfNeeded(); /// Create a cache of uncompressed blocks of specified size. This can be done only once. - void setUncompressedCache(size_t max_size_in_bytes, const String & uncompressed_cache_policy); + void setUncompressedCache(const String & uncompressed_cache_policy, size_t max_size_in_bytes); std::shared_ptr getUncompressedCache() const; void dropUncompressedCache() const; /// Create a cache of marks of specified size. This can be done only once. - void setMarkCache(size_t cache_size_in_bytes, const String & mark_cache_policy); + void setMarkCache(const String & mark_cache_policy, size_t cache_size_in_bytes); std::shared_ptr getMarkCache() const; void dropMarkCache() const; ThreadPool & getLoadMarksThreadpool() const; diff --git a/src/Storages/MarkCache.h b/src/Storages/MarkCache.h index 9095bf6bb35..ba521073928 100644 --- a/src/Storages/MarkCache.h +++ b/src/Storages/MarkCache.h @@ -40,8 +40,11 @@ private: using Base = CacheBase; public: - explicit MarkCache(size_t max_size_in_bytes, const String & mark_cache_policy = "") - : Base(max_size_in_bytes, 0, mark_cache_policy) {} + explicit MarkCache(size_t max_size_in_bytes) + : Base(max_size_in_bytes) {} + + MarkCache(const String & mark_cache_policy, size_t max_size_in_bytes) + : Base(mark_cache_policy, max_size_in_bytes) {} /// Calculate key from path to file and offset. static UInt128 hash(const String & path_to_file) From 1d8004756a8aed483c5e47f5924c601ae8b675c6 Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Mon, 13 Mar 2023 12:10:22 +0000 Subject: [PATCH 09/18] Cleanup: Reuse aliases instead of redeclaring it --- src/Common/CacheBase.h | 9 ++++----- src/Common/ICachePolicy.h | 4 +++- src/Common/LRUCachePolicy.h | 3 +-- src/Common/SLRUCachePolicy.h | 3 +-- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/Common/CacheBase.h b/src/Common/CacheBase.h index eb537c22834..204e21a1e89 100644 --- a/src/Common/CacheBase.h +++ b/src/Common/CacheBase.h @@ -33,9 +33,10 @@ template ; + using CachePolicy = ICachePolicy; + using Key = typename CachePolicy::Key; + using Mapped = typename CachePolicy::Mapped; + using MappedPtr = typename CachePolicy::MappedPtr; /// Use this ctor if you don't care about the internal cache policy. explicit CacheBase(size_t max_size_in_bytes, size_t max_entries = 0, double size_ratio = 0.5) @@ -198,8 +199,6 @@ protected: mutable std::mutex mutex; private: - using CachePolicy = ICachePolicy; - std::unique_ptr cache_policy TSA_GUARDED_BY(mutex); std::atomic hits{0}; diff --git a/src/Common/ICachePolicy.h b/src/Common/ICachePolicy.h index 208cf480576..a9dff7a99c1 100644 --- a/src/Common/ICachePolicy.h +++ b/src/Common/ICachePolicy.h @@ -16,10 +16,12 @@ struct EqualWeightFunction } }; -template , typename WeightFunction = EqualWeightFunction> +template , typename WeightFunction = EqualWeightFunction> class ICachePolicy { public: + using Key = TKey; + using Mapped = TMapped; using MappedPtr = std::shared_ptr; using OnWeightLossFunction = std::function; diff --git a/src/Common/LRUCachePolicy.h b/src/Common/LRUCachePolicy.h index f191f0b7d6b..19cd94be1d3 100644 --- a/src/Common/LRUCachePolicy.h +++ b/src/Common/LRUCachePolicy.h @@ -20,9 +20,8 @@ template , class LRUCachePolicy : public ICachePolicy { public: - using MappedPtr = std::shared_ptr; - using Base = ICachePolicy; + using typename Base::MappedPtr; using typename Base::OnWeightLossFunction; /** Initialize LRUCachePolicy with max_size_in_bytes and max_entries. diff --git a/src/Common/SLRUCachePolicy.h b/src/Common/SLRUCachePolicy.h index 80d622eba3e..fed958d3aff 100644 --- a/src/Common/SLRUCachePolicy.h +++ b/src/Common/SLRUCachePolicy.h @@ -22,9 +22,8 @@ template , class SLRUCachePolicy : public ICachePolicy { public: - using MappedPtr = std::shared_ptr; - using Base = ICachePolicy; + using typename Base::MappedPtr; using typename Base::OnWeightLossFunction; /** Initialize SLRUCachePolicy with max_size_in_bytes and max_protected_size. From e03618357e8b19b5025c70d9ad4d01a7ff674a73 Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Mon, 13 Mar 2023 14:42:06 +0000 Subject: [PATCH 10/18] Cleanup: Acquire lock in CacheBase::maxSize() The upcoming TTLPolicy will likely have a runtime-configurable cache size. --- src/Common/CacheBase.h | 4 ++-- src/Common/ICachePolicy.h | 2 +- src/Common/LRUCachePolicy.h | 2 +- src/Common/SLRUCachePolicy.h | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Common/CacheBase.h b/src/Common/CacheBase.h index 204e21a1e89..89aa78b390c 100644 --- a/src/Common/CacheBase.h +++ b/src/Common/CacheBase.h @@ -188,9 +188,9 @@ public: } size_t maxSize() const - TSA_NO_THREAD_SAFETY_ANALYSIS // disabled because max_size_in_bytes of cache_policy is a constant parameter { - return cache_policy->maxSize(); + std::lock_guard lock(mutex); + return cache_policy->maxSize(lock); } virtual ~CacheBase() = default; diff --git a/src/Common/ICachePolicy.h b/src/Common/ICachePolicy.h index a9dff7a99c1..f14e5b30c4f 100644 --- a/src/Common/ICachePolicy.h +++ b/src/Common/ICachePolicy.h @@ -27,7 +27,7 @@ public: virtual size_t weight(std::lock_guard & /* cache_lock */) const = 0; virtual size_t count(std::lock_guard & /* cache_lock */) const = 0; - virtual size_t maxSize() const = 0; + virtual size_t maxSize(std::lock_guard& /* cache_lock */) const = 0; virtual void reset(std::lock_guard & /* cache_lock */) = 0; virtual void remove(const Key & key, std::lock_guard & /* cache_lock */) = 0; diff --git a/src/Common/LRUCachePolicy.h b/src/Common/LRUCachePolicy.h index 19cd94be1d3..e308d016587 100644 --- a/src/Common/LRUCachePolicy.h +++ b/src/Common/LRUCachePolicy.h @@ -44,7 +44,7 @@ public: return cells.size(); } - size_t maxSize() const override + size_t maxSize(std::lock_guard & /* cache_lock */) const override { return max_size_in_bytes; } diff --git a/src/Common/SLRUCachePolicy.h b/src/Common/SLRUCachePolicy.h index fed958d3aff..a568d7f9c7f 100644 --- a/src/Common/SLRUCachePolicy.h +++ b/src/Common/SLRUCachePolicy.h @@ -49,7 +49,7 @@ public: return cells.size(); } - size_t maxSize() const override + size_t maxSize(std::lock_guard & /* cache_lock */) const override { return max_size_in_bytes; } From f1450923cbb3c836c833c1d3825ffae968894769 Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Mon, 13 Mar 2023 15:28:03 +0000 Subject: [PATCH 11/18] Extend CacheBase with dump() method --- src/Common/CacheBase.h | 7 +++++++ src/Common/ICachePolicy.h | 8 ++++++++ src/Common/LRUCachePolicy.h | 9 +++++++++ src/Common/SLRUCachePolicy.h | 9 +++++++++ 4 files changed, 33 insertions(+) diff --git a/src/Common/CacheBase.h b/src/Common/CacheBase.h index 89aa78b390c..f38c3943f09 100644 --- a/src/Common/CacheBase.h +++ b/src/Common/CacheBase.h @@ -37,6 +37,7 @@ public: using Key = typename CachePolicy::Key; using Mapped = typename CachePolicy::Mapped; using MappedPtr = typename CachePolicy::MappedPtr; + using KeyMapped = typename CachePolicy::KeyMapped; /// Use this ctor if you don't care about the internal cache policy. explicit CacheBase(size_t max_size_in_bytes, size_t max_entries = 0, double size_ratio = 0.5) @@ -160,6 +161,12 @@ public: out_misses = misses; } + std::vector dump() const + { + std::lock_guard lock(mutex); + return cache_policy->dump(); + } + void reset() { std::lock_guard lock(mutex); diff --git a/src/Common/ICachePolicy.h b/src/Common/ICachePolicy.h index f14e5b30c4f..c9d5bb282f6 100644 --- a/src/Common/ICachePolicy.h +++ b/src/Common/ICachePolicy.h @@ -25,6 +25,12 @@ public: using MappedPtr = std::shared_ptr; using OnWeightLossFunction = std::function; + struct KeyMapped + { + Key key; + MappedPtr mapped; + }; + virtual size_t weight(std::lock_guard & /* cache_lock */) const = 0; virtual size_t count(std::lock_guard & /* cache_lock */) const = 0; virtual size_t maxSize(std::lock_guard& /* cache_lock */) const = 0; @@ -34,6 +40,8 @@ public: virtual MappedPtr get(const Key & key, std::lock_guard & /* cache_lock */) = 0; virtual void set(const Key & key, const MappedPtr & mapped, std::lock_guard & /* cache_lock */) = 0; + virtual std::vector dump() const = 0; + virtual ~ICachePolicy() = default; }; diff --git a/src/Common/LRUCachePolicy.h b/src/Common/LRUCachePolicy.h index e308d016587..d08434eb1bf 100644 --- a/src/Common/LRUCachePolicy.h +++ b/src/Common/LRUCachePolicy.h @@ -22,6 +22,7 @@ class LRUCachePolicy : public ICachePolicy; using typename Base::MappedPtr; + using typename Base::KeyMapped; using typename Base::OnWeightLossFunction; /** Initialize LRUCachePolicy with max_size_in_bytes and max_entries. @@ -116,6 +117,14 @@ public: removeOverflow(); } + std::vector dump() const override + { + std::vector res; + for (const auto & [key, cell] : cells) + res.push_back({key, cell.value}); + return res; + } + protected: using LRUQueue = std::list; using LRUQueueIterator = typename LRUQueue::iterator; diff --git a/src/Common/SLRUCachePolicy.h b/src/Common/SLRUCachePolicy.h index a568d7f9c7f..42ee9916805 100644 --- a/src/Common/SLRUCachePolicy.h +++ b/src/Common/SLRUCachePolicy.h @@ -24,6 +24,7 @@ class SLRUCachePolicy : public ICachePolicy; using typename Base::MappedPtr; + using typename Base::KeyMapped; using typename Base::OnWeightLossFunction; /** Initialize SLRUCachePolicy with max_size_in_bytes and max_protected_size. @@ -148,6 +149,14 @@ public: removeOverflow(probationary_queue, max_size_in_bytes, current_size_in_bytes, /*is_protected=*/false); } + std::vector dump() const override + { + std::vector res; + for (const auto & [key, cell] : cells) + res.push_back({key, cell.value}); + return res; + } + protected: using SLRUQueue = std::list; using SLRUQueueIterator = typename SLRUQueue::iterator; From e1fb25a004e44b9549f881206fef0e88a1b77328 Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Mon, 13 Mar 2023 15:38:36 +0000 Subject: [PATCH 12/18] Extend CacheBase with getWithKey() method --- src/Common/CacheBase.h | 10 ++++++++++ src/Common/ICachePolicy.h | 4 ++++ src/Common/LRUCachePolicy.h | 18 +++++++++++++++--- src/Common/SLRUCachePolicy.h | 27 ++++++++++++++++++++++----- 4 files changed, 51 insertions(+), 8 deletions(-) diff --git a/src/Common/CacheBase.h b/src/Common/CacheBase.h index f38c3943f09..d9013973081 100644 --- a/src/Common/CacheBase.h +++ b/src/Common/CacheBase.h @@ -82,7 +82,17 @@ public: ++hits; else ++misses; + return res; + } + std::optional getWithKey(const Key & key) + { + std::lock_guard lock(mutex); + auto res = cache_policy->getWithKey(key, lock); + if (res.has_value()) + ++hits; + else + ++misses; return res; } diff --git a/src/Common/ICachePolicy.h b/src/Common/ICachePolicy.h index c9d5bb282f6..7f31af06296 100644 --- a/src/Common/ICachePolicy.h +++ b/src/Common/ICachePolicy.h @@ -37,7 +37,11 @@ public: virtual void reset(std::lock_guard & /* cache_lock */) = 0; virtual void remove(const Key & key, std::lock_guard & /* cache_lock */) = 0; + /// HashFunction usually hashes the entire key and the found key will be equal the provided key. In such cases, use get(). It is also + /// possible to store other, non-hashed data in the key. In that case, the found key is potentially different from the provided key. + /// Then use getWithKey() to also return the found key including it's non-hashed data. virtual MappedPtr get(const Key & key, std::lock_guard & /* cache_lock */) = 0; + virtual std::optional getWithKey(const Key &, std::lock_guard & /*cache_lock*/) = 0; virtual void set(const Key & key, const MappedPtr & mapped, std::lock_guard & /* cache_lock */) = 0; virtual std::vector dump() const = 0; diff --git a/src/Common/LRUCachePolicy.h b/src/Common/LRUCachePolicy.h index d08434eb1bf..0e1ef42474d 100644 --- a/src/Common/LRUCachePolicy.h +++ b/src/Common/LRUCachePolicy.h @@ -72,9 +72,7 @@ public: { auto it = cells.find(key); if (it == cells.end()) - { - return MappedPtr(); - } + return {}; Cell & cell = it->second; @@ -84,6 +82,20 @@ public: return cell.value; } + std::optional getWithKey(const Key & key, std::lock_guard & /*cache_lock*/) override + { + auto it = cells.find(key); + if (it == cells.end()) + return std::nullopt; + + Cell & cell = it->second; + + /// Move the key to the end of the queue. The iterator remains valid. + queue.splice(queue.end(), queue, cell.queue_iterator); + + return std::make_optional({it->first, cell.value}); + } + void set(const Key & key, const MappedPtr & mapped, std::lock_guard & /* cache_lock */) override { auto [it, inserted] = cells.emplace(std::piecewise_construct, diff --git a/src/Common/SLRUCachePolicy.h b/src/Common/SLRUCachePolicy.h index 42ee9916805..c16a9daaf8c 100644 --- a/src/Common/SLRUCachePolicy.h +++ b/src/Common/SLRUCachePolicy.h @@ -84,16 +84,12 @@ public: { auto it = cells.find(key); if (it == cells.end()) - { - return MappedPtr(); - } + return {}; Cell & cell = it->second; if (cell.is_protected) - { protected_queue.splice(protected_queue.end(), protected_queue, cell.queue_iterator); - } else { cell.is_protected = true; @@ -105,6 +101,27 @@ public: return cell.value; } + std::optional getWithKey(const Key & key, std::lock_guard & /*cache_lock*/) override + { + auto it = cells.find(key); + if (it == cells.end()) + return std::nullopt; + + Cell & cell = it->second; + + if (cell.is_protected) + protected_queue.splice(protected_queue.end(), protected_queue, cell.queue_iterator); + else + { + cell.is_protected = true; + current_protected_size += cell.size; + protected_queue.splice(protected_queue.end(), probationary_queue, cell.queue_iterator); + removeOverflow(protected_queue, max_protected_size, current_protected_size, /*is_protected=*/true); + } + + return std::make_optional({it->first, cell.value}); + } + void set(const Key & key, const MappedPtr & mapped, std::lock_guard & /* cache_lock */) override { auto [it, inserted] = cells.emplace(std::piecewise_construct, From b98579559393ae46f9379ea00e762246d2669161 Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Mon, 13 Mar 2023 15:39:42 +0000 Subject: [PATCH 13/18] Cleanup: Group ICachePolicy methods --- src/Common/ICachePolicy.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Common/ICachePolicy.h b/src/Common/ICachePolicy.h index 7f31af06296..342ae58cd1c 100644 --- a/src/Common/ICachePolicy.h +++ b/src/Common/ICachePolicy.h @@ -35,15 +35,17 @@ public: virtual size_t count(std::lock_guard & /* cache_lock */) const = 0; virtual size_t maxSize(std::lock_guard& /* cache_lock */) const = 0; - virtual void reset(std::lock_guard & /* cache_lock */) = 0; - virtual void remove(const Key & key, std::lock_guard & /* cache_lock */) = 0; /// HashFunction usually hashes the entire key and the found key will be equal the provided key. In such cases, use get(). It is also /// possible to store other, non-hashed data in the key. In that case, the found key is potentially different from the provided key. /// Then use getWithKey() to also return the found key including it's non-hashed data. virtual MappedPtr get(const Key & key, std::lock_guard & /* cache_lock */) = 0; virtual std::optional getWithKey(const Key &, std::lock_guard & /*cache_lock*/) = 0; + virtual void set(const Key & key, const MappedPtr & mapped, std::lock_guard & /* cache_lock */) = 0; + virtual void remove(const Key & key, std::lock_guard & /* cache_lock */) = 0; + + virtual void reset(std::lock_guard & /* cache_lock */) = 0; virtual std::vector dump() const = 0; virtual ~ICachePolicy() = default; From eed365c8c8dc94cb25047fbc385fb6631ecbd4a9 Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Tue, 14 Mar 2023 09:11:36 +0000 Subject: [PATCH 14/18] Cleanup: Slightly more naming consistency (probably rolls back earlier changes) --- src/Common/CacheBase.h | 10 +++++----- src/Common/LRUCachePolicy.h | 12 ++++++------ src/Common/SLRUCachePolicy.h | 12 ++++++------ 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/Common/CacheBase.h b/src/Common/CacheBase.h index d9013973081..e1b933923b6 100644 --- a/src/Common/CacheBase.h +++ b/src/Common/CacheBase.h @@ -40,13 +40,13 @@ public: using KeyMapped = typename CachePolicy::KeyMapped; /// Use this ctor if you don't care about the internal cache policy. - explicit CacheBase(size_t max_size_in_bytes, size_t max_entries = 0, double size_ratio = 0.5) - : CacheBase("SLRU", max_size_in_bytes, max_entries, size_ratio) + explicit CacheBase(size_t max_size_in_bytes, size_t max_count = 0, double size_ratio = 0.5) + : CacheBase("SLRU", max_size_in_bytes, max_count, size_ratio) { } /// Use this ctor if you want the user to configure the cache policy via some setting. Supports only general-purpose policies LRU and SLRU. - explicit CacheBase(std::string_view cache_policy_name, size_t max_size_in_bytes, size_t max_entries = 0, double size_ratio = 0.5) + explicit CacheBase(std::string_view cache_policy_name, size_t max_size_in_bytes, size_t max_count = 0, double size_ratio = 0.5) { auto on_weight_loss_function = [&](size_t weight_loss) { onRemoveOverflowWeightLoss(weight_loss); }; @@ -58,12 +58,12 @@ public: if (cache_policy_name == "LRU") { using LRUPolicy = LRUCachePolicy; - cache_policy = std::make_unique(max_size_in_bytes, max_entries, on_weight_loss_function); + cache_policy = std::make_unique(max_size_in_bytes, max_count, on_weight_loss_function); } else if (cache_policy_name == "SLRU") { using SLRUPolicy = SLRUCachePolicy; - cache_policy = std::make_unique(max_size_in_bytes, max_entries, size_ratio, on_weight_loss_function); + cache_policy = std::make_unique(max_size_in_bytes, max_count, size_ratio, on_weight_loss_function); } else throw Exception(ErrorCodes::BAD_ARGUMENTS, "Unknown cache policy name: {}", cache_policy_name); diff --git a/src/Common/LRUCachePolicy.h b/src/Common/LRUCachePolicy.h index 0e1ef42474d..3128ac3001b 100644 --- a/src/Common/LRUCachePolicy.h +++ b/src/Common/LRUCachePolicy.h @@ -25,12 +25,12 @@ public: using typename Base::KeyMapped; using typename Base::OnWeightLossFunction; - /** Initialize LRUCachePolicy with max_size_in_bytes and max_entries. - * max_entries == 0 means no elements size restrictions. + /** Initialize LRUCachePolicy with max_size_in_bytes and max_count. + * max_count == 0 means no elements size restrictions. */ - LRUCachePolicy(size_t max_size_in_bytes_, size_t max_entries_, OnWeightLossFunction on_weight_loss_function_) + LRUCachePolicy(size_t max_size_in_bytes_, size_t max_count_, OnWeightLossFunction on_weight_loss_function_) : max_size_in_bytes(std::max(static_cast(1), max_size_in_bytes_)) - , max_entries(max_entries_) + , max_count(max_count_) , on_weight_loss_function(on_weight_loss_function_) { } @@ -157,7 +157,7 @@ protected: /// Total weight of values. size_t current_size_in_bytes = 0; const size_t max_size_in_bytes; - const size_t max_entries; + const size_t max_count; WeightFunction weight_function; OnWeightLossFunction on_weight_loss_function; @@ -167,7 +167,7 @@ protected: size_t current_weight_lost = 0; size_t queue_size = cells.size(); - while ((current_size_in_bytes > max_size_in_bytes || (max_entries != 0 && queue_size > max_entries)) && (queue_size > 0)) + while ((current_size_in_bytes > max_size_in_bytes || (max_count != 0 && queue_size > max_count)) && (queue_size > 0)) { const Key & key = queue.front(); diff --git a/src/Common/SLRUCachePolicy.h b/src/Common/SLRUCachePolicy.h index c16a9daaf8c..5325c7f4094 100644 --- a/src/Common/SLRUCachePolicy.h +++ b/src/Common/SLRUCachePolicy.h @@ -32,10 +32,10 @@ public: * max_protected_size == 0 means that the default protected size is equal to half of the total max size. */ /// TODO: construct from special struct with cache policy parameters (also with max_protected_size). - SLRUCachePolicy(size_t max_size_in_bytes_, size_t max_entries_, double size_ratio, OnWeightLossFunction on_weight_loss_function_) + SLRUCachePolicy(size_t max_size_in_bytes_, size_t max_count_, double size_ratio, OnWeightLossFunction on_weight_loss_function_) : max_protected_size(static_cast(max_size_in_bytes_ * std::min(1.0, size_ratio))) , max_size_in_bytes(max_size_in_bytes_) - , max_entries(max_entries_) + , max_count(max_count_) , on_weight_loss_function(on_weight_loss_function_) { } @@ -197,7 +197,7 @@ protected: size_t current_size_in_bytes = 0; const size_t max_protected_size; const size_t max_size_in_bytes; - const size_t max_entries; + const size_t max_count; WeightFunction weight_function; OnWeightLossFunction on_weight_loss_function; @@ -212,11 +212,11 @@ protected: { /// Check if after remove all elements from probationary part there will be no more than max elements /// in protected queue and weight of all protected elements will be less then max protected weight. - /// It's not possible to check only cells.size() > max_entries + /// It's not possible to check only cells.size() > max_count /// because protected elements move to probationary part and still remain in cache. need_remove = [&]() { - return ((max_entries != 0 && cells.size() - probationary_queue.size() > max_entries) + return ((max_count != 0 && cells.size() - probationary_queue.size() > max_count) || (current_weight_size > max_weight_size)) && (queue_size > 0); }; } @@ -224,7 +224,7 @@ protected: { need_remove = [&]() { - return ((max_entries != 0 && cells.size() > max_entries) + return ((max_count != 0 && cells.size() > max_count) || (current_weight_size > max_weight_size)) && (queue_size > 0); }; } From 73afae2d3ff7a768e123b9e4de4105b6fa9d450e Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Mon, 13 Mar 2023 09:34:03 +0000 Subject: [PATCH 15/18] Introduce TTLCachePolicy --- src/Common/CacheBase.h | 16 +- src/Common/ICachePolicy.h | 10 ++ src/Common/LRUCachePolicy.h | 2 +- src/Common/SLRUCachePolicy.h | 2 +- src/Common/TTLCachePolicy.h | 148 ++++++++++++++++++ src/Interpreters/Cache/QueryCache.cpp | 133 +++++++--------- src/Interpreters/Cache/QueryCache.h | 65 ++++---- .../System/StorageSystemQueryCache.cpp | 8 +- 8 files changed, 262 insertions(+), 122 deletions(-) create mode 100644 src/Common/TTLCachePolicy.h diff --git a/src/Common/CacheBase.h b/src/Common/CacheBase.h index e1b933923b6..4ae313d7ecf 100644 --- a/src/Common/CacheBase.h +++ b/src/Common/CacheBase.h @@ -32,8 +32,10 @@ namespace ErrorCodes template , typename WeightFunction = EqualWeightFunction> class CacheBase { -public: +private: using CachePolicy = ICachePolicy; + +public: using Key = typename CachePolicy::Key; using Mapped = typename CachePolicy::Mapped; using MappedPtr = typename CachePolicy::MappedPtr; @@ -210,6 +212,18 @@ public: return cache_policy->maxSize(lock); } + void setMaxCount(size_t max_count) + { + std::lock_guard lock(mutex); + return cache_policy->setMaxCount(max_count, lock); + } + + void setMaxSize(size_t max_size_in_bytes) + { + std::lock_guard lock(mutex); + return cache_policy->setMaxSize(max_size_in_bytes, lock); + } + virtual ~CacheBase() = default; protected: diff --git a/src/Common/ICachePolicy.h b/src/Common/ICachePolicy.h index 342ae58cd1c..dca82095af1 100644 --- a/src/Common/ICachePolicy.h +++ b/src/Common/ICachePolicy.h @@ -1,5 +1,7 @@ #pragma once +#include + #include #include #include @@ -7,6 +9,11 @@ namespace DB { +namespace ErrorCodes +{ + extern const int NOT_IMPLEMENTED; +} + template struct EqualWeightFunction { @@ -35,6 +42,9 @@ public: virtual size_t count(std::lock_guard & /* cache_lock */) const = 0; virtual size_t maxSize(std::lock_guard& /* cache_lock */) const = 0; + virtual void setMaxCount(size_t /*max_count*/, std::lock_guard & /* cache_lock */) { throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Not implemented for cache policy"); } + virtual void setMaxSize(size_t /*max_size_in_bytes*/, std::lock_guard & /* cache_lock */) { throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Not implemented for cache policy"); } + /// HashFunction usually hashes the entire key and the found key will be equal the provided key. In such cases, use get(). It is also /// possible to store other, non-hashed data in the key. In that case, the found key is potentially different from the provided key. /// Then use getWithKey() to also return the found key including it's non-hashed data. diff --git a/src/Common/LRUCachePolicy.h b/src/Common/LRUCachePolicy.h index 3128ac3001b..56974b18115 100644 --- a/src/Common/LRUCachePolicy.h +++ b/src/Common/LRUCachePolicy.h @@ -137,7 +137,7 @@ public: return res; } -protected: +private: using LRUQueue = std::list; using LRUQueueIterator = typename LRUQueue::iterator; diff --git a/src/Common/SLRUCachePolicy.h b/src/Common/SLRUCachePolicy.h index 5325c7f4094..e36bca83c61 100644 --- a/src/Common/SLRUCachePolicy.h +++ b/src/Common/SLRUCachePolicy.h @@ -174,7 +174,7 @@ public: return res; } -protected: +private: using SLRUQueue = std::list; using SLRUQueueIterator = typename SLRUQueue::iterator; diff --git a/src/Common/TTLCachePolicy.h b/src/Common/TTLCachePolicy.h new file mode 100644 index 00000000000..fc6367b807f --- /dev/null +++ b/src/Common/TTLCachePolicy.h @@ -0,0 +1,148 @@ +#pragma once + +#include + +#include + +namespace DB +{ + +/// TTLCachePolicy evicts entries for which IsStaleFunction returns true. +/// The cache size (in bytes and number of entries) can be changed at runtime. It is expected to set both sizes explicitly after construction. +template +class TTLCachePolicy : public ICachePolicy +{ +public: + using Base = ICachePolicy; + using typename Base::MappedPtr; + using typename Base::KeyMapped; + using typename Base::OnWeightLossFunction; + + TTLCachePolicy() + : max_size_in_bytes(0) + , max_count(0) + { + } + + size_t weight(std::lock_guard & /* cache_lock */) const override + { + return size_in_bytes; + } + + size_t count(std::lock_guard & /* cache_lock */) const override + { + return cache.size(); + } + + size_t maxSize(std::lock_guard & /* cache_lock */) const override + { + return max_size_in_bytes; + } + + void setMaxCount(size_t max_count_, std::lock_guard & /* cache_lock */) override + { + /// lazy behavior: the cache only shrinks upon the next insert + max_count = max_count_; + } + + void setMaxSize(size_t max_size_in_bytes_, std::lock_guard & /* cache_lock */) override + { + /// lazy behavior: the cache only shrinks upon the next insert + max_size_in_bytes = max_size_in_bytes_; + } + + void reset(std::lock_guard & /* cache_lock */) override + { + cache.clear(); + } + + void remove(const Key & key, std::lock_guard & /* cache_lock */) override + { + auto it = cache.find(key); + if (it == cache.end()) + return; + size_in_bytes -= weight_function(*it->second); + cache.erase(it); + } + + MappedPtr get(const Key & key, std::lock_guard & /* cache_lock */) override + { + auto it = cache.find(key); + if (it == cache.end()) + return {}; + return it->second; + } + + std::optional getWithKey(const Key & key, std::lock_guard & /* cache_lock */) override + { + auto it = cache.find(key); + if (it == cache.end()) + return std::nullopt; + return std::make_optional({it->first, it->second}); + } + + /// Evicts on a best-effort basis. If there are too many non-stale entries, the new entry may not be cached at all! + void set(const Key & key, const MappedPtr & mapped, std::lock_guard & /* cache_lock */) override + { + chassert(mapped.get()); + + const size_t entry_size_in_bytes = weight_function(*mapped); + + auto sufficient_space_in_cache = [&]() + { + return (size_in_bytes + entry_size_in_bytes <= max_size_in_bytes) && (cache.size() + 1 <= max_count); + }; + + if (!sufficient_space_in_cache()) + { + /// Remove stale entries + for (auto it = cache.begin(); it != cache.end();) + if (is_stale_function(it->first)) + { + size_in_bytes -= weight_function(*it->second); + it = cache.erase(it); + } + else + ++it; + } + + if (sufficient_space_in_cache()) + { + /// Insert or replace key + if (auto it = cache.find(key); it != cache.end()) + { + size_in_bytes -= weight_function(*it->second); + cache.erase(it); // stupid bug: (*) doesn't replace existing entries (likely due to custom hash function), need to erase explicitly + } + + cache[key] = std::move(mapped); // (*) + size_in_bytes += entry_size_in_bytes; + } + } + + std::vector dump() const override + { + std::vector res; + for (const auto & [key, mapped] : cache) + res.push_back({key, mapped}); + return res; + } + +private: + using Cache = std::unordered_map; + Cache cache; + + /// TODO To speed up removal of stale entries, we could also add another container sorted on expiry times which maps keys to iterators + /// into the cache. To insert an entry, add it to the cache + add the iterator to the sorted container. To remove stale entries, do a + /// binary search on the sorted container and erase all left of the found key. + + size_t size_in_bytes = 0; + size_t max_size_in_bytes; + size_t max_count; + + WeightFunction weight_function; + IsStaleFunction is_stale_function; + /// TODO support OnWeightLossFunction callback +}; + +} diff --git a/src/Interpreters/Cache/QueryCache.cpp b/src/Interpreters/Cache/QueryCache.cpp index b0c8766e505..ce2373a8af9 100644 --- a/src/Interpreters/Cache/QueryCache.cpp +++ b/src/Interpreters/Cache/QueryCache.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include /// chassert @@ -152,43 +153,32 @@ size_t QueryCache::KeyHasher::operator()(const Key & key) const return res; } -size_t QueryCache::QueryResult::sizeInBytes() const +size_t QueryCache::QueryResultWeight::operator()(const QueryResult & chunks) const { size_t res = 0; - for (const auto & chunk : *chunks) + for (const auto & chunk : chunks) res += chunk.allocatedBytes(); return res; -}; +} -namespace -{ - -auto is_stale = [](const QueryCache::Key & key) +bool QueryCache::IsStale::operator()(const Key & key) const { return (key.expires_at < std::chrono::system_clock::now()); }; -} - -QueryCache::Writer::Writer(std::mutex & mutex_, Cache & cache_, const Key & key_, - size_t & cache_size_in_bytes_, size_t max_cache_size_in_bytes_, - size_t max_cache_entries_, +QueryCache::Writer::Writer(Cache & cache_, const Key & key_, size_t max_entry_size_in_bytes_, size_t max_entry_size_in_rows_, std::chrono::milliseconds min_query_runtime_) - : mutex(mutex_) - , cache(cache_) + : cache(cache_) , key(key_) - , cache_size_in_bytes(cache_size_in_bytes_) - , max_cache_size_in_bytes(max_cache_size_in_bytes_) - , max_cache_entries(max_cache_entries_) , max_entry_size_in_bytes(max_entry_size_in_bytes_) , max_entry_size_in_rows(max_entry_size_in_rows_) , min_query_runtime(min_query_runtime_) { - if (auto it = cache.find(key); it != cache.end() && !is_stale(it->first)) + if (auto entry = cache.getWithKey(key); entry.has_value() && !IsStale()(entry->key)) { skip_insert = true; /// Key already contained in cache and did not expire yet --> don't replace it - LOG_TRACE(&Poco::Logger::get("QueryResultCache"), "Skipped insert (non-stale entry found), query: {}", key.queryStringFromAst()); + LOG_TRACE(&Poco::Logger::get("QueryCache"), "Skipped insert (non-stale entry found), query: {}", key.queryStringFromAst()); } } @@ -197,18 +187,20 @@ void QueryCache::Writer::buffer(Chunk && partial_query_result) if (skip_insert) return; - auto & chunks = query_result.chunks; + std::lock_guard lock(mutex); - chunks->emplace_back(std::move(partial_query_result)); + auto & chunks = *query_result; - new_entry_size_in_bytes += chunks->back().allocatedBytes(); - new_entry_size_in_rows += chunks->back().getNumRows(); + chunks.emplace_back(std::move(partial_query_result)); + + new_entry_size_in_bytes += chunks.back().allocatedBytes(); + new_entry_size_in_rows += chunks.back().getNumRows(); if ((new_entry_size_in_bytes > max_entry_size_in_bytes) || (new_entry_size_in_rows > max_entry_size_in_rows)) { - chunks->clear(); /// eagerly free some space + chunks.clear(); /// eagerly free some space skip_insert = true; - LOG_TRACE(&Poco::Logger::get("QueryResultCache"), "Skipped insert (query result too big), new_entry_size_in_bytes: {} ({}), new_entry_size_in_rows: {} ({}), query: {}", new_entry_size_in_bytes, max_entry_size_in_bytes, new_entry_size_in_rows, max_entry_size_in_rows, key.queryStringFromAst()); + LOG_TRACE(&Poco::Logger::get("QueryCache"), "Skipped insert (query result too big), new_entry_size_in_bytes: {} ({}), new_entry_size_in_rows: {} ({}), query: {}", new_entry_size_in_bytes, max_entry_size_in_bytes, new_entry_size_in_rows, max_entry_size_in_rows, key.queryStringFromAst()); } } @@ -217,81 +209,47 @@ void QueryCache::Writer::finalizeWrite() if (skip_insert) return; - if (std::chrono::duration_cast(std::chrono::system_clock::now() - query_start_time) < min_query_runtime) - { - LOG_TRACE(&Poco::Logger::get("QueryResultCache"), "Skipped insert (query not expensive enough), query: {}", key.queryStringFromAst()); - return; - } - std::lock_guard lock(mutex); - if (auto it = cache.find(key); it != cache.end() && !is_stale(it->first)) + if (std::chrono::duration_cast(std::chrono::system_clock::now() - query_start_time) < min_query_runtime) { - /// same check as in ctor because a parallel Writer could have inserted the current key in the meantime - LOG_TRACE(&Poco::Logger::get("QueryResultCache"), "Skipped insert (non-stale entry found), query: {}", key.queryStringFromAst()); + LOG_TRACE(&Poco::Logger::get("QueryCache"), "Skipped insert (query not expensive enough), query: {}", key.queryStringFromAst()); return; } - auto sufficient_space_in_cache = [this]() TSA_REQUIRES(mutex) + if (auto entry = cache.getWithKey(key); entry.has_value() && !IsStale()(entry->key)) { - return (cache_size_in_bytes + new_entry_size_in_bytes <= max_cache_size_in_bytes) && (cache.size() + 1 <= max_cache_entries); - }; - - if (!sufficient_space_in_cache()) - { - size_t removed_items = 0; - /// Remove stale entries - for (auto it = cache.begin(); it != cache.end();) - if (is_stale(it->first)) - { - cache_size_in_bytes -= it->second.sizeInBytes(); - it = cache.erase(it); - ++removed_items; - } - else - ++it; - LOG_TRACE(&Poco::Logger::get("QueryCache"), "Removed {} stale entries", removed_items); + /// same check as in ctor because a parallel Writer could have inserted the current key in the meantime + LOG_TRACE(&Poco::Logger::get("QueryCache"), "Skipped insert (non-stale entry found), query: {}", key.queryStringFromAst()); + return; } - if (!sufficient_space_in_cache()) - LOG_TRACE(&Poco::Logger::get("QueryResultCache"), "Skipped insert (cache has insufficient space), query: {}", key.queryStringFromAst()); - else - { - //// Insert or replace key - cache_size_in_bytes += query_result.sizeInBytes(); - if (auto it = cache.find(key); it != cache.end()) - cache_size_in_bytes -= it->second.sizeInBytes(); // key replacement - - cache[key] = std::move(query_result); - LOG_TRACE(&Poco::Logger::get("QueryCache"), "Stored result of query {}", key.queryStringFromAst()); - } + cache.set(key, query_result); } -QueryCache::Reader::Reader(const Cache & cache_, const Key & key, size_t & cache_size_in_bytes_, const std::lock_guard &) +QueryCache::Reader::Reader(Cache & cache_, const Key & key, const std::lock_guard &) { - auto it = cache_.find(key); + auto entry = cache_.getWithKey(key); - if (it == cache_.end()) + if (!entry.has_value()) { LOG_TRACE(&Poco::Logger::get("QueryCache"), "No entry found for query {}", key.queryStringFromAst()); return; } - if (it->first.username.has_value() && it->first.username != key.username) + if (entry->key.username.has_value() && entry->key.username != key.username) { LOG_TRACE(&Poco::Logger::get("QueryCache"), "Inaccessible entry found for query {}", key.queryStringFromAst()); return; } - if (is_stale(it->first)) + if (IsStale()(entry->key)) { - cache_size_in_bytes_ -= it->second.sizeInBytes(); - const_cast(cache_).erase(it); - LOG_TRACE(&Poco::Logger::get("QueryCache"), "Stale entry found and removed for query {}", key.queryStringFromAst()); + LOG_TRACE(&Poco::Logger::get("QueryCache"), "Stale entry found for query {}", key.queryStringFromAst()); return; } - pipe = Pipe(std::make_shared(it->first.header, it->second.chunks)); + pipe = Pipe(std::make_shared(entry->key.header, entry->mapped)); LOG_TRACE(&Poco::Logger::get("QueryCache"), "Entry found for query {}", key.queryStringFromAst()); } @@ -316,19 +274,19 @@ Pipe && QueryCache::Reader::getPipe() QueryCache::Reader QueryCache::createReader(const Key & key) { std::lock_guard lock(mutex); - return Reader(cache, key, cache_size_in_bytes, lock); + return Reader(cache, key, lock); } QueryCache::Writer QueryCache::createWriter(const Key & key, std::chrono::milliseconds min_query_runtime) { std::lock_guard lock(mutex); - return Writer(mutex, cache, key, cache_size_in_bytes, max_cache_size_in_bytes, max_cache_entries, max_cache_entry_size_in_bytes, max_cache_entry_size_in_rows, min_query_runtime); + return Writer(cache, key, max_entry_size_in_bytes, max_entry_size_in_rows, min_query_runtime); } void QueryCache::reset() { + cache.reset(); std::lock_guard lock(mutex); - cache.clear(); times_executed.clear(); cache_size_in_bytes = 0; } @@ -344,13 +302,28 @@ size_t QueryCache::recordQueryRun(const Key & key) return times; } +std::vector QueryCache::dump() const +{ + return cache.dump(); +} + +QueryCache::QueryCache() + : cache(std::make_unique>()) +{ +} + void QueryCache::updateConfiguration(const Poco::Util::AbstractConfiguration & config) { std::lock_guard lock(mutex); - max_cache_size_in_bytes = config.getUInt64("query_cache.size", 1_GiB); - max_cache_entries = config.getUInt64("query_cache.max_entries", 1024); - max_cache_entry_size_in_bytes = config.getUInt64("query_cache.max_entry_size", 1_MiB); - max_cache_entry_size_in_rows = config.getUInt64("query_cache.max_entry_rows", 30'000'000); + + size_t max_size_in_bytes = config.getUInt64("query_cache.size", 1_GiB); + cache.setMaxSize(max_size_in_bytes); + + size_t max_entries = config.getUInt64("query_cache.max_entries", 1024); + cache.setMaxCount(max_entries); + + max_entry_size_in_bytes = config.getUInt64("query_cache.max_entry_size", 1_MiB); + max_entry_size_in_rows = config.getUInt64("query_cache.max_entry_rows", 30'000'000); } } diff --git a/src/Interpreters/Cache/QueryCache.h b/src/Interpreters/Cache/QueryCache.h index 66477d77dcb..763e797ac07 100644 --- a/src/Interpreters/Cache/QueryCache.h +++ b/src/Interpreters/Cache/QueryCache.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include @@ -41,7 +42,7 @@ public: /// Result metadata for constructing the pipe. const Block header; - /// Std::nullopt means that the associated entry can be read by other users. In general, sharing is a bad idea: First, it is + /// std::nullopt means that the associated entry can be read by other users. In general, sharing is a bad idea: First, it is /// unlikely that different users pose the same queries. Second, sharing potentially breaches security. E.g. User A should not be /// able to bypass row policies on some table by running the same queries as user B for whom no row policies exist. const std::optional username; @@ -57,15 +58,7 @@ public: String queryStringFromAst() const; }; - struct QueryResult - { - std::shared_ptr chunks = std::make_shared(); - size_t sizeInBytes() const; - - /// Notes: 1. For performance reasons, we cache the original result chunks as-is (no concatenation during cache insert or lookup). - /// 2. Ref-counting (shared_ptr) ensures that eviction of an entry does not affect queries which still read from the cache. - /// (this can also be achieved by copying the chunks during lookup but that would be under the cache lock --> too slow) - }; + using QueryResult = Chunks; private: struct KeyHasher @@ -73,8 +66,18 @@ private: size_t operator()(const Key & key) const; }; + struct QueryResultWeight + { + size_t operator()(const QueryResult & chunks) const; + }; + + struct IsStale + { + bool operator()(const Key & key) const; + }; + /// query --> query result - using Cache = std::unordered_map; + using Cache = CacheBase; /// query --> query execution count using TimesExecuted = std::unordered_map; @@ -97,24 +100,19 @@ public: void buffer(Chunk && partial_query_result); void finalizeWrite(); private: - std::mutex & mutex; - Cache & cache TSA_GUARDED_BY(mutex); + std::mutex mutex; + Cache & cache; const Key key; - size_t & cache_size_in_bytes TSA_GUARDED_BY(mutex); - const size_t max_cache_size_in_bytes; - const size_t max_cache_entries; - size_t new_entry_size_in_bytes = 0; + size_t new_entry_size_in_bytes TSA_GUARDED_BY(mutex) = 0; const size_t max_entry_size_in_bytes; - size_t new_entry_size_in_rows = 0; + size_t new_entry_size_in_rows TSA_GUARDED_BY(mutex) = 0; const size_t max_entry_size_in_rows; const std::chrono::time_point query_start_time = std::chrono::system_clock::now(); /// Writer construction and finalizeWrite() coincide with query start/end const std::chrono::milliseconds min_query_runtime; - QueryResult query_result; + std::shared_ptr query_result TSA_GUARDED_BY(mutex) = std::make_shared(); std::atomic skip_insert = false; - Writer(std::mutex & mutex_, Cache & cache_, const Key & key_, - size_t & cache_size_in_bytes_, size_t max_cache_size_in_bytes_, - size_t max_cache_entries_, + Writer(Cache & cache_, const Key & key_, size_t max_entry_size_in_bytes_, size_t max_entry_size_in_rows_, std::chrono::milliseconds min_query_runtime_); @@ -128,11 +126,13 @@ public: bool hasCacheEntryForKey() const; Pipe && getPipe(); /// must be called only if hasCacheEntryForKey() returns true private: - Reader(const Cache & cache_, const Key & key, size_t & cache_size_in_bytes_, const std::lock_guard &); + Reader(Cache & cache_, const Key & key, const std::lock_guard &); Pipe pipe; friend class QueryCache; /// for createReader() }; + QueryCache(); + void updateConfiguration(const Poco::Util::AbstractConfiguration & config); Reader createReader(const Key & key); @@ -143,23 +143,18 @@ public: /// Record new execution of query represented by key. Returns number of executions so far. size_t recordQueryRun(const Key & key); + /// For debugging and system tables + std::vector dump() const; + private: - /// Implementation note: The query result implements a custom caching mechanism and doesn't make use of CacheBase, unlike many other - /// internal caches in ClickHouse. The main reason is that we don't need standard CacheBase (S)LRU eviction as the expiry times - /// associated with cache entries provide a "natural" eviction criterion. As a future TODO, we could make an expiry-based eviction - /// policy and use that with CacheBase (e.g. see #23706) - /// TODO To speed up removal of stale entries, we could also add another container sorted on expiry times which maps keys to iterators - /// into the cache. To insert an entry, add it to the cache + add the iterator to the sorted container. To remove stale entries, do a - /// binary search on the sorted container and erase all left of the found key. + Cache cache; + mutable std::mutex mutex; - Cache cache TSA_GUARDED_BY(mutex); TimesExecuted times_executed TSA_GUARDED_BY(mutex); /// Cache configuration - size_t max_cache_size_in_bytes TSA_GUARDED_BY(mutex) = 0; - size_t max_cache_entries TSA_GUARDED_BY(mutex) = 0; - size_t max_cache_entry_size_in_bytes TSA_GUARDED_BY(mutex) = 0; - size_t max_cache_entry_size_in_rows TSA_GUARDED_BY(mutex) = 0; + size_t max_entry_size_in_bytes TSA_GUARDED_BY(mutex) = 0; + size_t max_entry_size_in_rows TSA_GUARDED_BY(mutex) = 0; size_t cache_size_in_bytes TSA_GUARDED_BY(mutex) = 0; /// Updated in each cache insert/delete diff --git a/src/Storages/System/StorageSystemQueryCache.cpp b/src/Storages/System/StorageSystemQueryCache.cpp index 2de8e4594b9..2cbcc773ad6 100644 --- a/src/Storages/System/StorageSystemQueryCache.cpp +++ b/src/Storages/System/StorageSystemQueryCache.cpp @@ -33,11 +33,11 @@ void StorageSystemQueryCache::fillData(MutableColumns & res_columns, ContextPtr if (!query_cache) return; + std::vector content = query_cache->dump(); + const String & username = context->getUserName(); - std::lock_guard lock(query_cache->mutex); - - for (const auto & [key, result] : query_cache->cache) + for (const auto & [key, query_result] : content) { /// Showing other user's queries is considered a security risk if (key.username.has_value() && key.username != username) @@ -48,7 +48,7 @@ void StorageSystemQueryCache::fillData(MutableColumns & res_columns, ContextPtr res_columns[2]->insert(std::chrono::system_clock::to_time_t(key.expires_at)); res_columns[3]->insert(key.expires_at < std::chrono::system_clock::now()); res_columns[4]->insert(!key.username.has_value()); - res_columns[5]->insert(result.sizeInBytes()); + res_columns[5]->insert(QueryCache::QueryResultWeight()(*query_result)); } } From b2aa9324ac5f6ed0a46bba0ec66d7033d98d0d82 Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Wed, 15 Mar 2023 11:03:42 +0000 Subject: [PATCH 16/18] Un-friend system view (no longer necessary) --- src/Interpreters/Cache/QueryCache.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Interpreters/Cache/QueryCache.h b/src/Interpreters/Cache/QueryCache.h index 763e797ac07..4c91d7f1ae7 100644 --- a/src/Interpreters/Cache/QueryCache.h +++ b/src/Interpreters/Cache/QueryCache.h @@ -157,8 +157,6 @@ private: size_t max_entry_size_in_rows TSA_GUARDED_BY(mutex) = 0; size_t cache_size_in_bytes TSA_GUARDED_BY(mutex) = 0; /// Updated in each cache insert/delete - - friend class StorageSystemQueryCache; }; using QueryCachePtr = std::shared_ptr; From 1f9c84e74757ca588ebb60b5f3fffd2eea9a6795 Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Wed, 15 Mar 2023 16:53:01 +0000 Subject: [PATCH 17/18] Revert "Un-friend system view (no longer necessary)" This reverts commit b2aa9324ac5f6ed0a46bba0ec66d7033d98d0d82. --- src/Interpreters/Cache/QueryCache.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Interpreters/Cache/QueryCache.h b/src/Interpreters/Cache/QueryCache.h index 4c91d7f1ae7..763e797ac07 100644 --- a/src/Interpreters/Cache/QueryCache.h +++ b/src/Interpreters/Cache/QueryCache.h @@ -157,6 +157,8 @@ private: size_t max_entry_size_in_rows TSA_GUARDED_BY(mutex) = 0; size_t cache_size_in_bytes TSA_GUARDED_BY(mutex) = 0; /// Updated in each cache insert/delete + + friend class StorageSystemQueryCache; }; using QueryCachePtr = std::shared_ptr; From eb206bbe254e8bcf474d6a1063ee4d3034b44757 Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Thu, 16 Mar 2023 16:08:24 +0000 Subject: [PATCH 18/18] Fix clang-tidy build --- src/Common/tests/gtest_lru_cache.cpp | 12 ++++++------ src/Common/tests/gtest_slru_cache.cpp | 22 +++++++++++----------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/Common/tests/gtest_lru_cache.cpp b/src/Common/tests/gtest_lru_cache.cpp index 9a2cb354bd5..1185dd58e5e 100644 --- a/src/Common/tests/gtest_lru_cache.cpp +++ b/src/Common/tests/gtest_lru_cache.cpp @@ -6,7 +6,7 @@ TEST(LRUCache, set) { using SimpleCacheBase = DB::CacheBase; - auto lru_cache = SimpleCacheBase("LRU", /*max_size*/ 10, /*max_elements_size*/ 10); + auto lru_cache = SimpleCacheBase("LRU", /*max_size_in_bytes*/ 10, /*max_count*/ 10); lru_cache.set(1, std::make_shared(2)); lru_cache.set(2, std::make_shared(3)); @@ -19,7 +19,7 @@ TEST(LRUCache, set) TEST(LRUCache, update) { using SimpleCacheBase = DB::CacheBase; - auto lru_cache = SimpleCacheBase("LRU", /*max_size*/ 10, /*max_elements_size*/ 10); + auto lru_cache = SimpleCacheBase("LRU", /*max_size_in_bytes*/ 10, /*max_count*/ 10); lru_cache.set(1, std::make_shared(2)); lru_cache.set(1, std::make_shared(3)); auto val = lru_cache.get(1); @@ -30,7 +30,7 @@ TEST(LRUCache, update) TEST(LRUCache, get) { using SimpleCacheBase = DB::CacheBase; - auto lru_cache = SimpleCacheBase("LRU", /*max_size*/ 10, /*max_elements_size*/ 10); + auto lru_cache = SimpleCacheBase("LRU", /*max_size_in_bytes*/ 10, /*max_count*/ 10); lru_cache.set(1, std::make_shared(2)); lru_cache.set(2, std::make_shared(3)); SimpleCacheBase::MappedPtr value = lru_cache.get(1); @@ -50,7 +50,7 @@ struct ValueWeight TEST(LRUCache, evictOnSize) { using SimpleCacheBase = DB::CacheBase; - auto lru_cache = SimpleCacheBase("LRU", /*max_size*/ 20, /*max_elements_size*/ 3); + auto lru_cache = SimpleCacheBase("LRU", /*max_size_in_bytes*/ 20, /*max_count*/ 3); lru_cache.set(1, std::make_shared(2)); lru_cache.set(2, std::make_shared(3)); lru_cache.set(3, std::make_shared(4)); @@ -66,7 +66,7 @@ TEST(LRUCache, evictOnSize) TEST(LRUCache, evictOnWeight) { using SimpleCacheBase = DB::CacheBase, ValueWeight>; - auto lru_cache = SimpleCacheBase("LRU", /*max_size*/ 10, /*max_elements_size*/ 10); + auto lru_cache = SimpleCacheBase("LRU", /*max_size_in_bytes*/ 10, /*max_count*/ 10); lru_cache.set(1, std::make_shared(2)); lru_cache.set(2, std::make_shared(3)); lru_cache.set(3, std::make_shared(4)); @@ -87,7 +87,7 @@ TEST(LRUCache, evictOnWeight) TEST(LRUCache, getOrSet) { using SimpleCacheBase = DB::CacheBase, ValueWeight>; - auto lru_cache = SimpleCacheBase("LRU", /*max_size*/ 10, /*max_elements_size*/ 10); + auto lru_cache = SimpleCacheBase("LRU", /*max_size_in_bytes*/ 10, /*max_count*/ 10); size_t x = 10; auto load_func = [&] { return std::make_shared(x); }; auto [value, loaded] = lru_cache.getOrSet(1, load_func); diff --git a/src/Common/tests/gtest_slru_cache.cpp b/src/Common/tests/gtest_slru_cache.cpp index f7ae9f9b16e..52549592f0e 100644 --- a/src/Common/tests/gtest_slru_cache.cpp +++ b/src/Common/tests/gtest_slru_cache.cpp @@ -6,7 +6,7 @@ TEST(SLRUCache, set) { using SimpleCacheBase = DB::CacheBase; - auto slru_cache = SimpleCacheBase("SLRU", /*max_size=*/10, /*max_elements_size=*/0, /*size_ratio*/0.5); + auto slru_cache = SimpleCacheBase("SLRU", /*max_size_in_bytes=*/10, /*max_count=*/0, /*size_ratio*/0.5); slru_cache.set(1, std::make_shared(2)); slru_cache.set(2, std::make_shared(3)); @@ -19,7 +19,7 @@ TEST(SLRUCache, set) TEST(SLRUCache, update) { using SimpleCacheBase = DB::CacheBase; - auto slru_cache = SimpleCacheBase("SLRU", /*max_size=*/10, /*max_elements_size=*/0, /*size_ratio*/0.5); + auto slru_cache = SimpleCacheBase("SLRU", /*max_size_in_bytes=*/10, /*max_count=*/0, /*size_ratio*/0.5); slru_cache.set(1, std::make_shared(2)); slru_cache.set(1, std::make_shared(3)); @@ -31,7 +31,7 @@ TEST(SLRUCache, update) TEST(SLRUCache, get) { using SimpleCacheBase = DB::CacheBase; - auto slru_cache = SimpleCacheBase("SLRU", /*max_size=*/10, /*max_elements_size=*/0, /*size_ratio*/0.5); + auto slru_cache = SimpleCacheBase("SLRU", /*max_size_in_bytes=*/10, /*max_count=*/0, /*size_ratio*/0.5); slru_cache.set(1, std::make_shared(2)); slru_cache.set(2, std::make_shared(3)); @@ -47,7 +47,7 @@ TEST(SLRUCache, get) TEST(SLRUCache, remove) { using SimpleCacheBase = DB::CacheBase; - auto slru_cache = SimpleCacheBase("SLRU", /*max_size=*/10, /*max_elements_size=*/0, /*size_ratio*/0.5); + auto slru_cache = SimpleCacheBase("SLRU", /*max_size_in_bytes=*/10, /*max_count=*/0, /*size_ratio*/0.5); slru_cache.set(1, std::make_shared(2)); slru_cache.set(2, std::make_shared(3)); @@ -63,7 +63,7 @@ TEST(SLRUCache, remove) TEST(SLRUCache, removeFromProtected) { using SimpleCacheBase = DB::CacheBase; - auto slru_cache = SimpleCacheBase("SLRU", /*max_size=*/2, /*max_elements_size=*/0, /*size_ratio*/0.5); + auto slru_cache = SimpleCacheBase("SLRU", /*max_size_in_bytes=*/2, /*max_count=*/0, /*size_ratio*/0.5); slru_cache.set(1, std::make_shared(2)); slru_cache.set(1, std::make_shared(3)); @@ -96,7 +96,7 @@ TEST(SLRUCache, removeFromProtected) TEST(SLRUCache, reset) { using SimpleCacheBase = DB::CacheBase; - auto slru_cache = SimpleCacheBase("SLRU", /*max_size=*/10, /*max_elements_size=*/0, /*size_ratio*/0.5); + auto slru_cache = SimpleCacheBase("SLRU", /*max_size_in_bytes=*/10, /*max_count=*/0, /*size_ratio*/0.5); slru_cache.set(1, std::make_shared(2)); slru_cache.set(2, std::make_shared(3)); @@ -119,7 +119,7 @@ struct ValueWeight TEST(SLRUCache, evictOnElements) { using SimpleCacheBase = DB::CacheBase, ValueWeight>; - auto slru_cache = SimpleCacheBase(/*max_size=*/10, /*max_elements_size=*/1, /*size_ratio*/0.5); + auto slru_cache = SimpleCacheBase(/*max_size_in_bytes=*/10, /*max_count=*/1, /*size_ratio*/0.5); slru_cache.set(1, std::make_shared(2)); slru_cache.set(2, std::make_shared(3)); @@ -140,7 +140,7 @@ TEST(SLRUCache, evictOnElements) TEST(SLRUCache, evictOnWeight) { using SimpleCacheBase = DB::CacheBase, ValueWeight>; - auto slru_cache = SimpleCacheBase(/*max_size=*/10, /*max_elements_size=*/0, /*size_ratio*/0.5); + auto slru_cache = SimpleCacheBase(/*max_size_in_bytes=*/10, /*max_count=*/0, /*size_ratio*/0.5); slru_cache.set(1, std::make_shared(2)); slru_cache.set(2, std::make_shared(3)); slru_cache.set(3, std::make_shared(4)); @@ -161,7 +161,7 @@ TEST(SLRUCache, evictOnWeight) TEST(SLRUCache, evictFromProtectedPart) { using SimpleCacheBase = DB::CacheBase, ValueWeight>; - auto slru_cache = SimpleCacheBase("SLRU", /*max_size=*/10, /*max_elements_size=*/0, /*size_ratio*/0.5); + auto slru_cache = SimpleCacheBase("SLRU", /*max_size_in_bytes=*/10, /*max_count=*/0, /*size_ratio*/0.5); slru_cache.set(1, std::make_shared(2)); slru_cache.set(1, std::make_shared(2)); @@ -177,7 +177,7 @@ TEST(SLRUCache, evictFromProtectedPart) TEST(SLRUCache, evictStreamProtected) { using SimpleCacheBase = DB::CacheBase, ValueWeight>; - auto slru_cache = SimpleCacheBase("SLRU", /*max_size=*/10, /*max_elements_size=*/0, /*size_ratio*/0.5); + auto slru_cache = SimpleCacheBase("SLRU", /*max_size_in_bytes=*/10, /*max_count=*/0, /*size_ratio*/0.5); slru_cache.set(1, std::make_shared(2)); slru_cache.set(1, std::make_shared(2)); @@ -201,7 +201,7 @@ TEST(SLRUCache, evictStreamProtected) TEST(SLRUCache, getOrSet) { using SimpleCacheBase = DB::CacheBase, ValueWeight>; - auto slru_cache = SimpleCacheBase("SLRU", /*max_size=*/10, /*max_elements_size=*/0, /*size_ratio*/0.5); + auto slru_cache = SimpleCacheBase("SLRU", /*max_size_in_bytes=*/10, /*max_count=*/0, /*size_ratio*/0.5); size_t x = 5; auto load_func = [&] { return std::make_shared(x); }; auto [value, loaded] = slru_cache.getOrSet(1, load_func);