Fix

src/Interpreters/Cache/Metadata.cpp

@@ -605,7 +605,7 @@ KeyMetadata::iterator LockedKey::removeFileSegment(size_t offset, const FileSegm
 {
     auto it = key_metadata->find(offset);
     if (it == key_metadata->end())
-        throw Exception(ErrorCodes::BAD_ARGUMENTS, "There is no offset {}", offset);
+        throw Exception(ErrorCodes::BAD_ARGUMENTS, "There is no offset {} in key {}", offset, getKey());
 
     return removeFileSegmentImpl(it, segment_lock, can_be_broken);
 }
@@ -625,23 +625,38 @@ KeyMetadata::iterator LockedKey::removeFileSegmentImpl(KeyMetadata::iterator it,
 
     file_segment->detach(segment_lock, *this);
 
-    const auto path = key_metadata->getFileSegmentPath(*file_segment);
+    try
-    bool exists = fs::exists(path);
-    if (exists)
     {
-        fs::remove(path);
+        const auto path = key_metadata->getFileSegmentPath(*file_segment);
+        bool exists = fs::exists(path);
+        if (exists)
+        {
+            fs::remove(path);
 
-        /// Clear OpenedFileCache to avoid reading from incorrect file descriptor.
+            /// Clear OpenedFileCache to avoid reading from incorrect file descriptor.
-        int flags = file_segment->getFlagsForLocalRead();
+            int flags = file_segment->getFlagsForLocalRead();
-        /// Files are created with flags from file_segment->getFlagsForLocalRead()
+            /// Files are created with flags from file_segment->getFlagsForLocalRead()
-        /// plus optionally O_DIRECT is added, depends on query setting, so remove both.
+            /// plus optionally O_DIRECT is added, depends on query setting, so remove both.
-        OpenedFileCache::instance().remove(path, flags);
+            OpenedFileCache::instance().remove(path, flags);
-        OpenedFileCache::instance().remove(path, flags | O_DIRECT);
+            OpenedFileCache::instance().remove(path, flags | O_DIRECT);
 
-        LOG_TEST(key_metadata->log, "Removed file segment at path: {}", path);
+            LOG_TEST(key_metadata->log, "Removed file segment at path: {}", path);
+        }
+        else if (file_segment->downloaded_size && !can_be_broken)
+        {
+#ifdef ABORT_ON_LOGICAL_ERROR
+            throw Exception(ErrorCodes::LOGICAL_ERROR, "Expected path {} to exist", path);
+#else
+            LOG_WARNING(key_metadata->log, "Expected path {} to exist, while removing {}:{}",
+                        path, getKey(), file_segment->offset());
+#endif
+        }
+    }
+    catch (...)
+    {
+        tryLogCurrentException(__PRETTY_FUNCTION__);
+        chassert(false);
     }
-    else if (file_segment->downloaded_size && !can_be_broken)
-        throw Exception(ErrorCodes::LOGICAL_ERROR, "Expected path {} to exist", path);
 
     return key_metadata->erase(it);
 }
@@ -763,7 +778,12 @@ FileSegments LockedKey::sync()
     for (auto it = key_metadata->begin(); it != key_metadata->end();)
     {
         auto file_segment = it->second->file_segment;
-        chassert(!file_segment->isDetached());
+        if (file_segment->isDetached())
+        {
+            throw Exception(
+                ErrorCodes::LOGICAL_ERROR,
+                "File segment has unexpected state: DETACHED ({})", file_segment->getInfoForLog());
+        }
 
         if (file_segment->getDownloadedSize(false) == 0)
         {
@@ -780,7 +800,7 @@ FileSegments LockedKey::sync()
                 file_segment->getInfoForLog());
 
             broken.push_back(FileSegment::getSnapshot(file_segment));
-            it = removeFileSegment(file_segment->offset(), file_segment->lock(), false);
+            it = removeFileSegment(file_segment->offset(), file_segment->lock(), /* can_be_broken */true);
             continue;
         }
 
@@ -799,18 +819,7 @@ FileSegments LockedKey::sync()
             actual_size, expected_size, file_segment->getInfoForLog());
 
         broken.push_back(FileSegment::getSnapshot(file_segment));
-
+        it = removeFileSegment(file_segment->offset(), file_segment->lock(), /* can_be_broken */false);
-        auto file_segment_lock = file_segment->lock();
-        if (actual_size < expected_size)
-        {
-            file_segment->downloaded_size = actual_size;
-            file_segment->download_state = FileSegment::State::PARTIALLY_DOWNLOADED_NO_CONTINUATION;
-            ++it;
-        }
-        else
-        {
-            it = removeFileSegment(file_segment->offset(), file_segment_lock, false);
-        }
     }
     return broken;
 }

tests/queries/0_stateless/02810_system_sync_filesystem_cache.sh

@@ -1,5 +1,5 @@
 #!/usr/bin/env bash
-# Tags: no-fasttest, no-parallel, no-s3-storage, no-random-settings, no-tsan, no-asan, no-ubsan, no-msan, no-debug
+# Tags: no-fasttest, no-parallel, no-s3-storage, no-random-settings
 
 # set -x
 
@@ -38,7 +38,8 @@ SELECT cache_path FROM system.filesystem_cache WHERE key = '$key' AND file_segme
 
 rm $path
 
-$CLICKHOUSE_CLIENT  --query "SELECT * FROM test FORMAT Null SETTINGS enable_filesystem_cache_log = 1" 2>&1 | grep -f -q "File path does not exist" && echo 'ok' || echo 'fail'
+
+$CLICKHOUSE_CLIENT  --query "SELECT * FROM test FORMAT Null SETTINGS enable_filesystem_cache_log = 1" 2>&1 | grep -F -e "No such file or directory" -e "File path does not exist" > /dev/null && echo "ok" || echo "fail"
 
 CLICKHOUSE_CLIENT=$(echo ${CLICKHOUSE_CLIENT} | sed 's/'"--send_logs_level=${CLICKHOUSE_CLIENT_SERVER_LOGS_LEVEL}"'/--send_logs_level=fatal/g')
 
@@ -60,7 +61,7 @@ SELECT cache_path FROM system.filesystem_cache WHERE key = '$key' AND file_segme
 
 echo -n 'fff' > $path
 
-cat $path
+#cat $path
 
 $CLICKHOUSE_CLIENT --query "SYSTEM SYNC FILESYSTEM CACHE" 2>&1 | grep -q "$key" && echo 'ok' || echo 'fail'