Merge pull request #49675 from ClickHouse/cache-cleanup-after-locking-rework

Some cache cleanup after rework locking

src/Interpreters/Cache/FileSegment.h

@@ -61,7 +61,7 @@ struct CreateFileSegmentSettings
         : kind(kind_), unbounded(unbounded_) {}
 };
 
-class FileSegment : private boost::noncopyable, public std::enable_shared_from_this<FileSegment>
+class FileSegment : private boost::noncopyable
 {
 friend struct LockedKey;
 friend class FileCache; /// Because of reserved_size in tryReserve().

src/Interpreters/Cache/Guards.h

@@ -1,8 +1,6 @@
 #pragma once
 #include <mutex>
-#include <Interpreters/Cache/FileCache_fwd.h>
 #include <boost/noncopyable.hpp>
-#include <map>
 
 namespace DB
 {
@@ -63,6 +61,8 @@ namespace DB
  */
 struct CacheGuard : private boost::noncopyable
 {
+    /// struct is used (not keyword `using`) to make CacheGuard::Lock non-interchangable with other guards locks
+    /// so, we wouldn't be able to pass CacheGuard::Lock to a function which accepts KeyGuard::Lock, for example
     struct Lock : public std::unique_lock<std::mutex>
     {
         explicit Lock(std::mutex & mutex_) : std::unique_lock<std::mutex>(mutex_) {}

src/Interpreters/Cache/Metadata.cpp

@@ -208,10 +208,9 @@ LockedKeyPtr CacheMetadata::lockKeyMetadata(
         chassert(key_not_found_policy == KeyNotFoundPolicy::CREATE_EMPTY);
     }
 
-    /// Not we are at a case:
-    /// key_state == KeyMetadata::KeyState::REMOVED
-    /// and KeyNotFoundPolicy == CREATE_EMPTY
-    /// Retry.
+    /// Now we are at the case when the key was removed (key_state == KeyMetadata::KeyState::REMOVED)
+    /// but we need to return empty key (key_not_found_policy == KeyNotFoundPolicy::CREATE_EMPTY)
+    /// Retry
     return lockKeyMetadata(key, key_not_found_policy);
 }
 
@@ -241,13 +240,6 @@ void CacheMetadata::doCleanup()
 {
     auto lock = guard.lock();
 
-    /// Let's mention this case.
-    /// This metadata cleanup is delayed so what is we marked key as deleted and
-    /// put it to deletion queue, but then the same key was added to cache before
-    /// we actually performed this delayed removal?
-    /// In this case it will work fine because on each attempt to add any key to cache
-    /// we perform this delayed removal.
-
     FileCacheKey cleanup_key;
     while (cleanup_queue->tryPop(cleanup_key))
     {