From 1cd73b907c1493729699578231fa223f57de2f35 Mon Sep 17 00:00:00 2001 From: kssenii Date: Fri, 5 Apr 2024 12:59:57 +0200 Subject: [PATCH] Better --- src/Interpreters/Cache/EvictionCandidates.h | 11 -- src/Interpreters/Cache/FileCache.cpp | 123 ++++++++++++------ src/Interpreters/Cache/IFileCachePriority.h | 3 +- .../Cache/LRUFileCachePriority.cpp | 6 +- src/Interpreters/Cache/LRUFileCachePriority.h | 3 +- .../Cache/SLRUFileCachePriority.cpp | 33 +++-- .../Cache/SLRUFileCachePriority.h | 3 +- tests/config/config.d/storage_conf.xml | 6 +- 8 files changed, 116 insertions(+), 72 deletions(-) diff --git a/src/Interpreters/Cache/EvictionCandidates.h b/src/Interpreters/Cache/EvictionCandidates.h index b2bb6ae9e9f..db062561ad7 100644 --- a/src/Interpreters/Cache/EvictionCandidates.h +++ b/src/Interpreters/Cache/EvictionCandidates.h @@ -10,15 +10,6 @@ public: using FinalizeEvictionFunc = std::function; EvictionCandidates() = default; - EvictionCandidates(EvictionCandidates && other) noexcept - { - candidates = std::move(other.candidates); - candidates_size = std::move(other.candidates_size); - on_finalize = std::move(other.on_finalize); - queue_entries_to_invalidate = std::move(other.queue_entries_to_invalidate); - hold_space = std::move(other.hold_space); - } - ~EvictionCandidates(); void add( @@ -26,8 +17,6 @@ public: LockedKey & locked_key, const CachePriorityGuard::Lock &); - void add(const EvictionCandidates & other, const CachePriorityGuard::Lock &) { candidates.insert(other.candidates.begin(), other.candidates.end()); } - void evict(); void onFinalize(FinalizeEvictionFunc && func) { on_finalize.emplace_back(std::move(func)); } diff --git a/src/Interpreters/Cache/FileCache.cpp b/src/Interpreters/Cache/FileCache.cpp index 3f16a8dde4e..61401c8ea0a 100644 --- a/src/Interpreters/Cache/FileCache.cpp +++ b/src/Interpreters/Cache/FileCache.cpp @@ -953,49 +953,71 @@ void FileCache::freeSpaceRatioKeepingThreadFunc() static constexpr auto space_ratio_satisfied_reschedule_ms = 5000; static constexpr auto general_reschedule_ms = 5000; - while (true) + if (shutdown) + return; + + Stopwatch watch; + + auto lock = tryLockCache(); + + /// To avoid deteriorating contention on cache, + /// proceed only if cache is not heavily used. + if (!lock) { + keep_up_free_space_ratio_task->scheduleAfter(lock_failed_reschedule_ms); + return; + } + + const size_t size_limit = main_priority->getSizeLimit(lock); + const size_t elements_limit = main_priority->getElementsLimit(lock); + + const size_t desired_size = std::lround(keep_current_size_to_max_ratio * size_limit); + const size_t desired_elements_num = std::lround(keep_current_elements_to_max_ratio * elements_limit); + + if ((size_limit == 0 || main_priority->getSize(lock) <= desired_size) + && (elements_limit == 0 || main_priority->getElementsCount(lock) <= desired_elements_num)) + { + /// Nothing to free - all limits are satisfied. + keep_up_free_space_ratio_task->scheduleAfter(space_ratio_satisfied_reschedule_ms); + return; + } + + FileCacheReserveStat stat; + EvictionCandidates eviction_candidates; + + try + { + /// Collect at most `keep_up_free_space_remove_batch` elements to evict, + /// (we use batches to make sure we do not block cache for too long, + /// by default the batch size is quite small). + const bool limits_satisfied = main_priority->collectCandidatesForEviction( + desired_size, desired_elements_num, keep_up_free_space_remove_batch, stat, eviction_candidates, lock); + +#ifdef ABORT_ON_LOGICAL_ERROR + /// Let's make sure that we correctly processed the limits. + if (limits_satisfied && eviction_candidates.size() < keep_up_free_space_remove_batch) + { + const auto current_size = main_priority->getSize(lock); + chassert(current_size >= stat.total_stat.releasable_size); + chassert(!size_limit + || current_size <= desired_size + || current_size - stat.total_stat.releasable_size <= desired_size); + + const auto current_elements_count = main_priority->getElementsCount(lock); + chassert(current_elements_count >= stat.total_stat.releasable_count); + chassert(!elements_limit + || current_elements_count <= desired_elements_num + || current_elements_count - stat.total_stat.releasable_count <= desired_elements_num); + } +#else + UNUSED(limits_satisfied); +#endif + if (shutdown) return; - auto lock = tryLockCache(); - if (!lock) + if (eviction_candidates.size() > 0) { - keep_up_free_space_ratio_task->scheduleAfter(lock_failed_reschedule_ms); - return; - } - - const size_t size_limit = main_priority->getSizeLimit(lock); - const size_t elements_limit = main_priority->getElementsLimit(lock); - - const size_t desired_size = std::lround(keep_current_size_to_max_ratio * size_limit); - const size_t desired_elements_num = std::lround(keep_current_elements_to_max_ratio * elements_limit); - - if ((size_limit == 0 || main_priority->getSize(lock) <= desired_size) - && (elements_limit == 0 || main_priority->getElementsCount(lock) <= desired_elements_num)) - { - /// Nothing to free - all limits are satisfied. - keep_up_free_space_ratio_task->scheduleAfter(space_ratio_satisfied_reschedule_ms); - return; - } - - try - { - FileCacheReserveStat stat; - auto eviction_candidates = main_priority->collectCandidatesForEviction( - desired_size, desired_elements_num, keep_up_free_space_remove_batch, stat, lock); - - if (shutdown) - return; - - if (eviction_candidates.size() == 0) - { - /// This case is impossible in realistic cache setup, - /// e.g. we should always be able to evict something. - keep_up_free_space_ratio_task->scheduleAfter(general_reschedule_ms); - return; - } - LOG_TRACE(log, "Current usage {}/{} in size, {}/{} in elements count " "(trying to keep size ration at {} and elements ratio at {}). " "Collected {} eviction candidates, " @@ -1006,20 +1028,35 @@ void FileCache::freeSpaceRatioKeepingThreadFunc() eviction_candidates.size(), stat.total_stat.non_releasable_count); lock.unlock(); + + /// Remove files from filesystem. eviction_candidates.evict(); + /// Take lock again to finalize eviction, + /// e.g. to update the in-memory state. lock.lock(); eviction_candidates.finalize(nullptr, lock); } - catch (...) + else { - tryLogCurrentException(__PRETTY_FUNCTION__); keep_up_free_space_ratio_task->scheduleAfter(general_reschedule_ms); - - /// Let's catch such cases in ci, in general there should not be exceptions. - chassert(false); } } + catch (...) + { + tryLogCurrentException(__PRETTY_FUNCTION__); + + if (eviction_candidates.size() > 0) + eviction_candidates.finalize(nullptr, lockCache()); + + keep_up_free_space_ratio_task->scheduleAfter(general_reschedule_ms); + + /// Let's catch such cases in ci, + /// in general there should not be exceptions. + chassert(false); + } + + LOG_TRACE(log, "Free space ratio keeping thread finished in {} ms", watch.elapsedMilliseconds()); } void FileCache::iterate(IterateFunc && func, const UserID & user_id) diff --git a/src/Interpreters/Cache/IFileCachePriority.h b/src/Interpreters/Cache/IFileCachePriority.h index dba925ba37b..e1ac6c6661d 100644 --- a/src/Interpreters/Cache/IFileCachePriority.h +++ b/src/Interpreters/Cache/IFileCachePriority.h @@ -148,11 +148,12 @@ public: const CachePriorityGuard::Lock &) = 0; /// Collect eviction `candidates_num` candidates for eviction. - virtual EvictionCandidates collectCandidatesForEviction( + virtual bool collectCandidatesForEviction( size_t desired_size, size_t desired_elements_count, size_t max_candidates_to_evict, FileCacheReserveStat & stat, + EvictionCandidates & candidates, const CachePriorityGuard::Lock &) = 0; virtual bool modifySizeLimits(size_t max_size_, size_t max_elements_, double size_ratio_, const CachePriorityGuard::Lock &) = 0; diff --git a/src/Interpreters/Cache/LRUFileCachePriority.cpp b/src/Interpreters/Cache/LRUFileCachePriority.cpp index 203fc680b47..8ac60cd9e6e 100644 --- a/src/Interpreters/Cache/LRUFileCachePriority.cpp +++ b/src/Interpreters/Cache/LRUFileCachePriority.cpp @@ -322,24 +322,24 @@ bool LRUFileCachePriority::collectCandidatesForEviction( } } -EvictionCandidates LRUFileCachePriority::collectCandidatesForEviction( +bool LRUFileCachePriority::collectCandidatesForEviction( size_t desired_size, size_t desired_elements_count, size_t max_candidates_to_evict, FileCacheReserveStat & stat, + EvictionCandidates & res, const CachePriorityGuard::Lock & lock) { if (!max_candidates_to_evict) return {}; - EvictionCandidates res; auto stop_condition = [&, this]() { return (getSize(lock) <= desired_size && getElementsCount(lock) <= desired_elements_count) || res.size() >= max_candidates_to_evict; }; iterateForEviction(res, stat, stop_condition, lock); - return res; + return stop_condition(); } void LRUFileCachePriority::iterateForEviction( diff --git a/src/Interpreters/Cache/LRUFileCachePriority.h b/src/Interpreters/Cache/LRUFileCachePriority.h index d05c67fe5d0..4cc80f07664 100644 --- a/src/Interpreters/Cache/LRUFileCachePriority.h +++ b/src/Interpreters/Cache/LRUFileCachePriority.h @@ -62,11 +62,12 @@ public: const UserID & user_id, const CachePriorityGuard::Lock &) override; - EvictionCandidates collectCandidatesForEviction( + bool collectCandidatesForEviction( size_t desired_size, size_t desired_elements_count, size_t max_candidates_to_evict, FileCacheReserveStat & stat, + EvictionCandidates & res, const CachePriorityGuard::Lock &) override; void shuffle(const CachePriorityGuard::Lock &) override; diff --git a/src/Interpreters/Cache/SLRUFileCachePriority.cpp b/src/Interpreters/Cache/SLRUFileCachePriority.cpp index a2730d88abb..4b6221d972a 100644 --- a/src/Interpreters/Cache/SLRUFileCachePriority.cpp +++ b/src/Interpreters/Cache/SLRUFileCachePriority.cpp @@ -251,11 +251,12 @@ bool SLRUFileCachePriority::collectCandidatesForEvictionInProtected( return true; } -EvictionCandidates SLRUFileCachePriority::collectCandidatesForEviction( +bool SLRUFileCachePriority::collectCandidatesForEviction( size_t desired_size, size_t desired_elements_count, size_t max_candidates_to_evict, FileCacheReserveStat & stat, + EvictionCandidates & res, const CachePriorityGuard::Lock & lock) { if (!max_candidates_to_evict) @@ -264,22 +265,36 @@ EvictionCandidates SLRUFileCachePriority::collectCandidatesForEviction( const auto desired_probationary_size = getRatio(desired_size, 1 - size_ratio); const auto desired_probationary_elements_num = getRatio(desired_elements_count, 1 - size_ratio); - auto res = probationary_queue.collectCandidatesForEviction( - desired_probationary_size, desired_probationary_elements_num, max_candidates_to_evict, stat, lock); + FileCacheReserveStat probationary_stat; + const bool probationary_limit_satisfied = probationary_queue.collectCandidatesForEviction( + desired_probationary_size, desired_probationary_elements_num, + max_candidates_to_evict, probationary_stat, res, lock); + + stat += probationary_stat; + + LOG_TEST(log, "Collected {} to evict from probationary queue. Total size: {}", + res.size(), probationary_stat.total_stat.releasable_size); chassert(res.size() <= max_candidates_to_evict); chassert(res.size() == stat.total_stat.releasable_count); - if (res.size() == max_candidates_to_evict) - return res; + if (res.size() >= max_candidates_to_evict) + return probationary_limit_satisfied; const auto desired_protected_size = getRatio(max_size, size_ratio); const auto desired_protected_elements_num = getRatio(max_elements, size_ratio); - auto res_add = protected_queue.collectCandidatesForEviction( - desired_protected_size, desired_protected_elements_num, max_candidates_to_evict - res.size(), stat, lock); - res.add(res_add, lock); - return res; + FileCacheReserveStat protected_stat; + const bool protected_limit_satisfied = protected_queue.collectCandidatesForEviction( + desired_protected_size, desired_protected_elements_num, + max_candidates_to_evict - res.size(), protected_stat, res, lock); + + stat += protected_stat; + + LOG_TEST(log, "Collected {} to evict from protected queue. Total size: {}", + res.size(), protected_stat.total_stat.releasable_size); + + return probationary_limit_satisfied && protected_limit_satisfied; } void SLRUFileCachePriority::downgrade(IteratorPtr iterator, const CachePriorityGuard::Lock & lock) diff --git a/src/Interpreters/Cache/SLRUFileCachePriority.h b/src/Interpreters/Cache/SLRUFileCachePriority.h index e837b8541c2..ee3cafe322d 100644 --- a/src/Interpreters/Cache/SLRUFileCachePriority.h +++ b/src/Interpreters/Cache/SLRUFileCachePriority.h @@ -58,11 +58,12 @@ public: const UserID & user_id, const CachePriorityGuard::Lock &) override; - EvictionCandidates collectCandidatesForEviction( + bool collectCandidatesForEviction( size_t desired_size, size_t desired_elements_count, size_t max_candidates_to_evict, FileCacheReserveStat & stat, + EvictionCandidates & res, const CachePriorityGuard::Lock &) override; void shuffle(const CachePriorityGuard::Lock &) override; diff --git a/tests/config/config.d/storage_conf.xml b/tests/config/config.d/storage_conf.xml index 24e85031a3f..9d1f7e6b474 100644 --- a/tests/config/config.d/storage_conf.xml +++ b/tests/config/config.d/storage_conf.xml @@ -19,13 +19,13 @@ cache s3_disk s3_cache/ - 64Mi + 100Mi 1 100 LRU 0.3 - 0.1 - 0.1 + 0.15 + 0.15 cache