diff --git a/dbms/src/Storages/S3/FileCache.cpp b/dbms/src/Storages/S3/FileCache.cpp index 5ba2b77d00d..1646217df7e 100644 --- a/dbms/src/Storages/S3/FileCache.cpp +++ b/dbms/src/Storages/S3/FileCache.cpp @@ -166,7 +166,7 @@ FileSegmentPtr FileCache::get(const S3::S3FilenameView & s3_fname, const std::op } // Remove `local_fname` from disk and remove parent directory if parent directory is empty. -void FileCache::removeDiskFile(const String & local_fname) +void FileCache::removeDiskFile(const String & local_fname, bool update_fsize_metrics) const { if (!std::filesystem::exists(local_fname)) { @@ -181,9 +181,7 @@ void FileCache::removeDiskFile(const String & local_fname) if (s != cache_dir && (s == local_fname || std::filesystem::is_empty(p))) { std::filesystem::remove(p); // If p is a directory, remove success only when it is empty. - // Temporary files are not reported size to metrics until they are renamed. - // So we don't need to free its size here. - if (s == local_fname && !isTemporaryFilename(local_fname)) + if (s == local_fname && update_fsize_metrics) { capacity_metrics->freeUsedSize(local_fname, fsize); } @@ -231,9 +229,10 @@ std::pair::iterator> FileCache::removeImpl( return {-1, {}}; } const auto & local_fname = f->getLocalFileName(); - removeDiskFile(local_fname); + removeDiskFile(local_fname, /*update_fsize_metrics*/ true); auto temp_fname = toTemporaryFilename(local_fname); - removeDiskFile(temp_fname); + // Not update fsize metrics for temporary files because they are not add to fsize metrics before. + removeDiskFile(temp_fname, /*update_fsize_metrics*/ false); auto release_size = f->getSize(); GET_METRIC(tiflash_storage_remote_cache, type_dtfile_evict).Increment(); @@ -651,7 +650,8 @@ void FileCache::restoreDMFile(const std::filesystem::directory_entry & dmfile_en auto fname = file_entry.path().string(); if (unlikely(isTemporaryFilename(fname))) { - removeDiskFile(fname); + // Not update fsize metrics for temporary files because they are not add to fsize metrics before. + removeDiskFile(fname, /*update_fsize_metrics*/ false); } else { @@ -669,7 +669,8 @@ void FileCache::restoreDMFile(const std::filesystem::directory_entry & dmfile_en } else { - removeDiskFile(fname); + // Not update fsize metrics because this file is not added to metrics yet. + removeDiskFile(fname, /*update_fsize_metrics*/ false); } } } diff --git a/dbms/src/Storages/S3/FileCache.h b/dbms/src/Storages/S3/FileCache.h index 9425e573acd..c4d8d66aada 100644 --- a/dbms/src/Storages/S3/FileCache.h +++ b/dbms/src/Storages/S3/FileCache.h @@ -256,7 +256,7 @@ class FileCache const String & s3_key, FileSegmentPtr & f, bool force = false); - void removeDiskFile(const String & local_fname); + void removeDiskFile(const String & local_fname, bool update_fsize_metrics) const; // Estimated size is an empirical value. // We don't know object size before get object from S3. diff --git a/dbms/src/Storages/S3/tests/gtest_filecache.cpp b/dbms/src/Storages/S3/tests/gtest_filecache.cpp index d5d65d7390a..187e76e43c3 100644 --- a/dbms/src/Storages/S3/tests/gtest_filecache.cpp +++ b/dbms/src/Storages/S3/tests/gtest_filecache.cpp @@ -379,7 +379,7 @@ TEST_F(FileCacheTest, FileSystem) } ASSERT_TRUE(std::filesystem::exists(local_file2)); - file_cache.removeDiskFile(local_fname1); + file_cache.removeDiskFile(local_fname1, false); ASSERT_FALSE(std::filesystem::exists(local_file1)) << local_file1.generic_string(); ASSERT_TRUE(std::filesystem::exists(local_file2)) << local_file2.generic_string(); ASSERT_TRUE(std::filesystem::exists(dmf)) << dmf.generic_string(); @@ -393,7 +393,7 @@ TEST_F(FileCacheTest, FileSystem) ASSERT_TRUE(std::filesystem::exists(cache_root)) << cache_root.generic_string(); ASSERT_EQ(cache_root.generic_string(), cache_config.getDTFileCacheDir()); - file_cache.removeDiskFile(local_fname2); + file_cache.removeDiskFile(local_fname2, false); ASSERT_FALSE(std::filesystem::exists(local_file2)) << local_file2.generic_string(); ASSERT_FALSE(std::filesystem::exists(dmf)) << dmf.generic_string(); ASSERT_FALSE(std::filesystem::exists(table)) << table.generic_string();