From 1adef76cbdacee93d0ed2fcc59088c8ad7021046 Mon Sep 17 00:00:00 2001 From: kssenii Date: Sun, 26 Mar 2023 22:10:14 +0200 Subject: [PATCH] Better comment --- src/Interpreters/Cache/Metadata.cpp | 9 +++++---- src/Interpreters/tests/gtest_lru_file_cache.cpp | 13 ------------- 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/src/Interpreters/Cache/Metadata.cpp b/src/Interpreters/Cache/Metadata.cpp index a68d4b3e5ec..b9f7353e636 100644 --- a/src/Interpreters/Cache/Metadata.cpp +++ b/src/Interpreters/Cache/Metadata.cpp @@ -141,10 +141,11 @@ void CacheMetadata::doCleanup() auto key_metadata = it->second; auto key_lock = key_metadata->lock(); - /// As in lockKeyMetadata we extract key metadata from cache metadata - /// under CacheMetadataGuard::Lock, but take KeyGuard::Lock only after we - /// released cache CacheMetadataGuard::Lock, then we must to take into - /// account it here. + /// As in lockKeyMetadata we extract key metadata from cache metadata under + /// CacheMetadataGuard::Lock, but take KeyGuard::Lock only after we released + /// cache CacheMetadataGuard::Lock (because CacheMetadataGuard::Lock must be lightweight). + /// So it is possible that a key which was submitted to cleanup queue was afterwards added + /// to cache, so here we need to check this case. if (key_metadata->getCleanupState(key_lock) == KeyMetadata::CleanupState::NOT_SUBMITTED) continue; diff --git a/src/Interpreters/tests/gtest_lru_file_cache.cpp b/src/Interpreters/tests/gtest_lru_file_cache.cpp index 93df029a592..16c8498f04e 100644 --- a/src/Interpreters/tests/gtest_lru_file_cache.cpp +++ b/src/Interpreters/tests/gtest_lru_file_cache.cpp @@ -189,36 +189,23 @@ TEST_F(FileCacheTest, get) std::cerr << "Step 1\n"; auto cache = FileCache(cache_base_path, settings); - std::cerr << "Step 1\n"; cache.initialize(); - std::cerr << "Step 1\n"; auto key = cache.createKeyForPath("key1"); - std::cerr << "Step 1\n"; { - std::cerr << "Step 1\n"; auto holder = cache.getOrSet(key, 0, 10, {}); /// Add range [0, 9] - std::cerr << "Step 1\n"; assertEqual(holder, { Range(0, 9) }, { State::EMPTY }); - std::cerr << "Step 1\n"; download(holder->front()); - std::cerr << "Step 1\n"; assertEqual(holder, { Range(0, 9) }, { State::DOWNLOADED }); - std::cerr << "Step 1\n"; } /// Current cache: [__________] /// ^ ^ /// 0 9 - std::cerr << "Step 1\n"; assertEqual(cache.getSnapshot(key), { Range(0, 9) }); - std::cerr << "Step 1\n"; assertEqual(cache.dumpQueue(), { Range(0, 9) }); - std::cerr << "Step 1\n"; ASSERT_EQ(cache.getFileSegmentsNum(), 1); - std::cerr << "Step 1\n"; ASSERT_EQ(cache.getUsedCacheSize(), 10); - std::cerr << "Step 1\n"; std::cerr << "Step 2\n";