From a78571de1175e5fae0b39843f85937a377c7a02b Mon Sep 17 00:00:00 2001 From: kssenii Date: Wed, 31 Aug 2022 18:47:47 +0200 Subject: [PATCH] Fix --- src/Common/FileCache.cpp | 39 +++++++++++++++++++++------------------ src/Common/FileCache.h | 2 ++ 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/src/Common/FileCache.cpp b/src/Common/FileCache.cpp index 44ecac2cc02..d290ee30ed1 100644 --- a/src/Common/FileCache.cpp +++ b/src/Common/FileCache.cpp @@ -59,6 +59,24 @@ String FileCache::getPathInLocalCache(const Key & key) const return fs::path(cache_base_path) / key_str.substr(0, 3) / key_str; } +void FileCache::removeKeyDirectoryIfExists(const Key & key, std::lock_guard & /* cache_lock */) const +{ + /// Note: it is guaranteed that there is no concurrency here with files deletion + /// because cache key directories are create only in FileCache class under cache_lock. + + auto key_str = key.toString(); + auto key_prefix_path = fs::path(cache_base_path) / key_str.substr(0, 3); + auto key_path = key_prefix_path / key_str; + + if (!fs::exists(key_path)) + return; + + fs::remove_all(key_path); + + if (fs::is_empty(key_prefix_path)) + fs::remove(key_prefix_path); +} + static bool isQueryInitialized() { return CurrentThread::isInitialized() @@ -174,15 +192,8 @@ FileSegments FileCache::getImpl( const auto & file_segments = it->second; if (file_segments.empty()) { - auto key_path = getPathInLocalCache(key); - files.erase(key); - - /// Note: it is guaranteed that there is no concurrency with files deletion, - /// because cache files are deleted only inside FileCache and under cache lock. - if (fs::exists(key_path)) - fs::remove_all(key_path); - + removeKeyDirectoryIfExists(key, cache_lock); return {}; } @@ -827,14 +838,10 @@ void FileCache::removeIfExists(const Key & key) } } - auto key_path = getPathInLocalCache(key); - if (!some_cells_were_skipped) { files.erase(key); - - if (fs::exists(key_path)) - fs::remove_all(key_path); + removeKeyDirectoryIfExists(key, cache_lock); } } @@ -924,12 +931,8 @@ void FileCache::remove( if (is_initialized && offsets.empty()) { - auto key_path = getPathInLocalCache(key); - files.erase(key); - - if (fs::exists(key_path)) - fs::remove_all(key_path); + removeKeyDirectoryIfExists(key, cache_lock); } } catch (...) diff --git a/src/Common/FileCache.h b/src/Common/FileCache.h index b5b1e917e76..cdd0612cc00 100644 --- a/src/Common/FileCache.h +++ b/src/Common/FileCache.h @@ -261,6 +261,8 @@ private: void assertCacheCellsCorrectness(const FileSegmentsByOffset & cells_by_offset, std::lock_guard & cache_lock); + void removeKeyDirectoryIfExists(const Key & key, std::lock_guard & cache_lock) const; + /// Used to track and control the cache access of each query. /// Through it, we can realize the processing of different queries by the cache layer. struct QueryContext