From 79c14c89ee39bea6017e41e375f9b6a59e3e7899 Mon Sep 17 00:00:00 2001 From: kssenii Date: Fri, 2 Jun 2023 12:53:25 +0200 Subject: [PATCH] Add some assertions --- src/Interpreters/Cache/FileCache.cpp | 6 +++--- src/Interpreters/Cache/FileSegment.cpp | 18 +++++++++++++----- src/Interpreters/Cache/Metadata.cpp | 21 ++++++++++++--------- src/Interpreters/Cache/Metadata.h | 4 ++-- 4 files changed, 30 insertions(+), 19 deletions(-) diff --git a/src/Interpreters/Cache/FileCache.cpp b/src/Interpreters/Cache/FileCache.cpp index 1908a4ce895..163e1b71ab9 100644 --- a/src/Interpreters/Cache/FileCache.cpp +++ b/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 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; } diff --git a/src/Interpreters/Cache/FileSegment.cpp b/src/Interpreters/Cache/FileSegment.cpp index 60228573666..fb0ba0eba14 100644 --- a/src/Interpreters/Cache/FileSegment.cpp +++ b/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(fs::file_size(getPathInLocalCache()) > 0); + chassert(downloaded_size > 0); + chassert(fs::file_size(getPathInLocalCache()) == downloaded_size); } void FileSegment::setDownloadFailedUnlocked(const FileSegmentGuard::Lock & lock) @@ -845,7 +852,8 @@ void FileSegment::detach(const FileSegmentGuard::Lock & lock, const LockedKey &) if (download_state == State::DETACHED) return; - resetDownloaderUnlocked(lock); + if (!downloader_id.empty()) + resetDownloaderUnlocked(lock); setDetachedState(lock); } diff --git a/src/Interpreters/Cache/Metadata.cpp b/src/Interpreters/Cache/Metadata.cpp index 843ffd45b63..9dff77e2af8 100644 --- a/src/Interpreters/Cache/Metadata.cpp +++ b/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; - - const auto key_str = key.toString(); - return fs::path(path) / key_str.substr(0, 3) / key_str / getFileNameForFileSegment(offset, segment_kind); + return fs::path(getPathForKey(key)) / 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( - 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); diff --git a/src/Interpreters/Cache/Metadata.h b/src/Interpreters/Cache/Metadata.h index 2e015b07ed0..a7e101c3d9d 100644 --- a/src/Interpreters/Cache/Metadata.h +++ b/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);