Add some assertions

src/Interpreters/Cache/FileCache.cpp

@@ -48,12 +48,12 @@ const String & FileCache::getBasePath() const
 
 String FileCache::getPathInLocalCache(const Key & key, size_t offset, FileSegmentKind segment_kind) const
 {
-    return metadata.getPathInLocalCache(key, offset, segment_kind);
+    return metadata.getPathForFileSegment(key, offset, segment_kind);
 }
 
 String FileCache::getPathInLocalCache(const Key & key) const
 {
-    return metadata.getPathInLocalCache(key);
+    return metadata.getPathForKey(key);
 }
 
 void FileCache::assertInitialized() const
@@ -1019,7 +1019,7 @@ std::vector<String> FileCache::tryGetCachePaths(const Key & key)
     for (const auto & [offset, file_segment_metadata] : *locked_key->getKeyMetadata())
     {
         if (file_segment_metadata->file_segment->state() == FileSegment::State::DOWNLOADED)
-            cache_paths.push_back(metadata.getPathInLocalCache(key, offset, file_segment_metadata->file_segment->getKind()));
+            cache_paths.push_back(metadata.getPathForFileSegment(key, offset, file_segment_metadata->file_segment->getKind()));
     }
     return cache_paths;
 }

src/Interpreters/Cache/FileSegment.cpp

@@ -313,6 +313,13 @@ void FileSegment::write(const char * from, size_t size, size_t offset)
     if (!size)
         throw Exception(ErrorCodes::LOGICAL_ERROR, "Writing zero size is not allowed");
 
+    auto file_segment_path = getPathInLocalCache();
+    if (offset == range().left && fs::exists(file_segment_path))
+    {
+        fs::remove(file_segment_path);
+        chassert(false);
+    }
+
     {
         auto lock = segment_guard.lock();
 
@@ -365,7 +372,7 @@ void FileSegment::write(const char * from, size_t size, size_t offset)
 
         downloaded_size += size;
 
-        chassert(std::filesystem::file_size(getPathInLocalCache()) == downloaded_size);
+        chassert(std::filesystem::file_size(file_segment_path) == downloaded_size);
     }
     catch (ErrnoException & e)
     {
@@ -375,7 +382,7 @@ void FileSegment::write(const char * from, size_t size, size_t offset)
         int code = e.getErrno();
         if (code == /* No space left on device */28 || code == /* Quota exceeded */122)
         {
-            const auto file_size = fs::file_size(getPathInLocalCache());
+            const auto file_size = fs::file_size(file_segment_path);
             chassert(downloaded_size <= file_size);
             chassert(reserved_size >= file_size);
             if (downloaded_size != file_size)
@@ -520,8 +527,8 @@ void FileSegment::setDownloadedUnlocked(const FileSegmentGuard::Lock &)
         remote_file_reader.reset();
     }
 
-    chassert(getDownloadedSize(false) > 0);
+    chassert(downloaded_size > 0);
-    chassert(fs::file_size(getPathInLocalCache()) > 0);
+    chassert(fs::file_size(getPathInLocalCache()) == downloaded_size);
 }
 
 void FileSegment::setDownloadFailedUnlocked(const FileSegmentGuard::Lock & lock)
@@ -845,6 +852,7 @@ void FileSegment::detach(const FileSegmentGuard::Lock & lock, const LockedKey &)
     if (download_state == State::DETACHED)
         return;
 
+    if (!downloader_id.empty())
         resetDownloaderUnlocked(lock);
     setDetachedState(lock);
 }

src/Interpreters/Cache/Metadata.cpp

@@ -145,15 +145,12 @@ String CacheMetadata::getFileNameForFileSegment(size_t offset, FileSegmentKind s
     return std::to_string(offset) + file_suffix;
 }
 
-String CacheMetadata::getPathInLocalCache(const Key & key, size_t offset, FileSegmentKind segment_kind) const
+String CacheMetadata::getPathForFileSegment(const Key & key, size_t offset, FileSegmentKind segment_kind) const
 {
-    String file_suffix;
+    return fs::path(getPathForKey(key)) / getFileNameForFileSegment(offset, segment_kind);
-
-    const auto key_str = key.toString();
-    return fs::path(path) / key_str.substr(0, 3) / key_str / getFileNameForFileSegment(offset, segment_kind);
 }
 
-String CacheMetadata::getPathInLocalCache(const Key & key) const
+String CacheMetadata::getPathForKey(const Key & key) const
 {
     const auto key_str = key.toString();
     return fs::path(path) / key_str.substr(0, 3) / key_str;
@@ -178,7 +175,7 @@ LockedKeyPtr CacheMetadata::lockKeyMetadata(
 
             it = emplace(
                 key, std::make_shared<KeyMetadata>(
-                    key, getPathInLocalCache(key), *cleanup_queue, is_initial_load)).first;
+                    key, getPathForKey(key), *cleanup_queue, is_initial_load)).first;
         }
 
         key_metadata = it->second;
@@ -260,7 +257,7 @@ void CacheMetadata::doCleanup()
         erase(it);
         LOG_DEBUG(log, "Key {} is removed from metadata", cleanup_key);
 
-        const fs::path key_directory = getPathInLocalCache(cleanup_key);
+        const fs::path key_directory = getPathForKey(cleanup_key);
         const fs::path key_prefix_directory = key_directory.parent_path();
 
         try
@@ -370,8 +367,14 @@ KeyMetadata::iterator LockedKey::removeFileSegment(size_t offset, const FileSegm
         file_segment->queue_iterator->annul();
 
     const auto path = key_metadata->getFileSegmentPath(*file_segment);
-    if (fs::exists(path))
+    bool exists = fs::exists(path);
+    if (exists)
+    {
+        LOG_TEST(log, "Removed file segment at path: {}", path);
         fs::remove(path);
+    }
+    else if (file_segment->downloaded_size)
+        throw Exception(ErrorCodes::LOGICAL_ERROR, "Expected path {} to exist");
 
     file_segment->detach(segment_lock, *this);
     return key_metadata->erase(it);

src/Interpreters/Cache/Metadata.h

@@ -85,12 +85,12 @@ public:
 
     const String & getBaseDirectory() const { return path; }
 
-    String getPathInLocalCache(
+    String getPathForFileSegment(
         const Key & key,
         size_t offset,
         FileSegmentKind segment_kind) const;
 
-    String getPathInLocalCache(const Key & key) const;
+    String getPathForKey(const Key & key) const;
     static String getFileNameForFileSegment(size_t offset, FileSegmentKind segment_kind);
 
     void iterate(IterateCacheMetadataFunc && func);