This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.1
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.1 by this push:
new da6ff4d07e1 branch-4.1: [fix](filecache) clean empty v3 cache dirs
#63344 (#63616)
da6ff4d07e1 is described below
commit da6ff4d07e191f94aaa6cf4534522e132ca992b6
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Tue May 26 12:04:26 2026 +0800
branch-4.1: [fix](filecache) clean empty v3 cache dirs #63344 (#63616)
Cherry-picked from #63344
Co-authored-by: Xin Liao <[email protected]>
---
be/src/io/cache/fs_file_cache_storage.cpp | 32 ++++++++----
be/src/io/fs/local_file_system.cpp | 20 ++++++++
be/src/io/fs/local_file_system.h | 3 ++
.../fs_file_cache_storage_leak_cleaner_test.cpp | 57 ++++++++++++++++++++++
be/test/io/fs/local_file_system_test.cpp | 24 +++++++++
5 files changed, 127 insertions(+), 9 deletions(-)
diff --git a/be/src/io/cache/fs_file_cache_storage.cpp
b/be/src/io/cache/fs_file_cache_storage.cpp
index 029d37425e5..5ed82dcaf43 100644
--- a/be/src/io/cache/fs_file_cache_storage.cpp
+++ b/be/src/io/cache/fs_file_cache_storage.cpp
@@ -279,25 +279,39 @@ Status FSFileCacheStorage::read(const FileCacheKey& key,
size_t value_offset, Sl
}
Status FSFileCacheStorage::remove(const FileCacheKey& key) {
- std::string dir = get_path_in_local_cache_v3(key.hash);
- std::string file = get_path_in_local_cache_v3(dir, key.offset);
+ const std::string v3_dir = get_path_in_local_cache_v3(key.hash);
+ const std::string v3_file = get_path_in_local_cache_v3(v3_dir, key.offset);
FDCache::instance()->remove_file_reader(std::make_pair(key.hash,
key.offset));
- RETURN_IF_ERROR(fs->delete_file(file));
+ RETURN_IF_ERROR(fs->delete_file(v3_file));
// return OK not means the file is deleted, it may be not exist
+ std::string v2_dir;
{ // try to detect the file with old v2 format
- dir = get_path_in_local_cache_v2(key.hash, key.meta.expiration_time);
- file = get_path_in_local_cache_v2(dir, key.offset, key.meta.type);
- RETURN_IF_ERROR(fs->delete_file(file));
+ v2_dir = get_path_in_local_cache_v2(key.hash,
key.meta.expiration_time);
+ const std::string v2_file = get_path_in_local_cache_v2(v2_dir,
key.offset, key.meta.type);
+ RETURN_IF_ERROR(fs->delete_file(v2_file));
}
BlockMetaKey mkey(key.meta.tablet_id, UInt128Wrapper(key.hash),
key.offset);
_meta_store->delete_key(mkey);
std::vector<FileInfo> files;
bool exists {false};
- RETURN_IF_ERROR(fs->list(dir, true, &files, &exists));
- if (files.empty()) {
- RETURN_IF_ERROR(fs->delete_directory(dir));
+ RETURN_IF_ERROR(fs->list(v2_dir, true, &files, &exists));
+ if (exists && files.empty()) {
+ auto st = fs->delete_empty_directory(v2_dir);
+ if (!st.ok()) {
+ LOG_WARNING("failed to remove cache directory {}",
v2_dir).error(st);
+ }
+ }
+
+ files.clear();
+ exists = false;
+ RETURN_IF_ERROR(fs->list(v3_dir, true, &files, &exists));
+ if (exists && files.empty()) {
+ auto st = fs->delete_empty_directory(v3_dir);
+ if (!st.ok()) {
+ LOG_WARNING("failed to remove cache directory {}",
v3_dir).error(st);
+ }
}
return Status::OK();
}
diff --git a/be/src/io/fs/local_file_system.cpp
b/be/src/io/fs/local_file_system.cpp
index 7edabd836c1..4f9b6baf2a7 100644
--- a/be/src/io/fs/local_file_system.cpp
+++ b/be/src/io/fs/local_file_system.cpp
@@ -167,6 +167,26 @@ Status LocalFileSystem::delete_directory_or_file(const
Path& path) {
FILESYSTEM_M(delete_directory_or_file_impl(path));
}
+Status LocalFileSystem::delete_empty_directory(const Path& dir) {
+ FILESYSTEM_M(delete_empty_directory_impl(dir));
+}
+
+Status LocalFileSystem::delete_empty_directory_impl(const Path& dir) {
+ Path path;
+ RETURN_IF_ERROR(absolute_path(dir, path));
+ VLOG_DEBUG << "delete empty directory: " << path.native();
+ int ret = 0;
+ RETRY_ON_EINTR(ret, rmdir(path.c_str()));
+ if (ret != 0) {
+ std::error_code ec(errno, std::generic_category());
+ if (ec == std::errc::no_such_file_or_directory) {
+ return Status::OK();
+ }
+ return localfs_error(ec, fmt::format("failed to delete empty directory
{}", path.native()));
+ }
+ return Status::OK();
+}
+
Status LocalFileSystem::delete_directory_or_file_impl(const Path& path) {
bool is_dir;
RETURN_IF_ERROR(is_directory(path, &is_dir));
diff --git a/be/src/io/fs/local_file_system.h b/be/src/io/fs/local_file_system.h
index c1ffbd22949..b1546bc4734 100644
--- a/be/src/io/fs/local_file_system.h
+++ b/be/src/io/fs/local_file_system.h
@@ -63,6 +63,8 @@ public:
static bool equal_or_sub_path(const Path& parent, const Path& child);
// delete dir or file
Status delete_directory_or_file(const Path& path);
+ // delete directory only if it is empty
+ Status delete_empty_directory(const Path& dir);
// change the file permission of the given path
Status permission(const Path& file, std::filesystem::perms prms);
@@ -87,6 +89,7 @@ protected:
Status delete_file_impl(const Path& file) override;
Status delete_directory_impl(const Path& dir) override;
Status delete_directory_or_file_impl(const Path& path);
+ Status delete_empty_directory_impl(const Path& dir);
Status batch_delete_impl(const std::vector<Path>& files) override;
Status exists_impl(const Path& path, bool* res) const override;
Status file_size_impl(const Path& file, int64_t* file_size) const override;
diff --git a/be/test/io/cache/fs_file_cache_storage_leak_cleaner_test.cpp
b/be/test/io/cache/fs_file_cache_storage_leak_cleaner_test.cpp
index 49ef8587ced..e4e066ad458 100644
--- a/be/test/io/cache/fs_file_cache_storage_leak_cleaner_test.cpp
+++ b/be/test/io/cache/fs_file_cache_storage_leak_cleaner_test.cpp
@@ -288,6 +288,63 @@ TEST_F(FSFileCacheLeakCleanerTest,
remove_orphan_and_tmp_files) {
fs::remove_all(dir, ec);
}
+TEST_F(FSFileCacheLeakCleanerTest,
remove_cleans_empty_v2_and_v3_key_directories) {
+ auto dir = prepare_test_dir();
+ FileCacheSettings settings = default_settings();
+ BlockFileCache mgr(dir.string(), settings);
+ FSFileCacheStorage storage;
+ setup_storage(storage, mgr, dir);
+
+ FileCacheKey key;
+ key.hash = BlockFileCache::hash("remove_empty_v3_dir");
+ key.offset = 0;
+ key.meta.expiration_time = 123456789;
+ key.meta.type = FileCacheType::NORMAL;
+ key.meta.tablet_id = 0;
+
+ auto key_dir = storage.get_path_in_local_cache_v3(key.hash);
+ auto block_path = FSFileCacheStorage::get_path_in_local_cache_v3(key_dir,
key.offset);
+ auto old_key_dir = storage.get_path_in_local_cache_v2(key.hash,
key.meta.expiration_time);
+ auto old_block_path =
+ FSFileCacheStorage::get_path_in_local_cache_v2(old_key_dir,
key.offset, key.meta.type);
+ create_regular_file(block_path, 'r');
+ create_regular_file(old_block_path, 'o');
+
+ ASSERT_TRUE(storage.remove(key).ok());
+
+ EXPECT_FALSE(fs::exists(block_path));
+ EXPECT_FALSE(fs::exists(key_dir));
+ EXPECT_FALSE(fs::exists(old_block_path));
+ EXPECT_FALSE(fs::exists(old_key_dir));
+}
+
+TEST_F(FSFileCacheLeakCleanerTest, remove_v3_block_when_v2_directory_missing) {
+ auto dir = prepare_test_dir();
+ FileCacheSettings settings = default_settings();
+ BlockFileCache mgr(dir.string(), settings);
+ FSFileCacheStorage storage;
+ setup_storage(storage, mgr, dir);
+
+ FileCacheKey key;
+ key.hash = BlockFileCache::hash("remove_v3_without_v2_dir");
+ key.offset = 0;
+ key.meta.expiration_time = 123456789;
+ key.meta.type = FileCacheType::NORMAL;
+ key.meta.tablet_id = 0;
+
+ auto key_dir = storage.get_path_in_local_cache_v3(key.hash);
+ auto block_path = FSFileCacheStorage::get_path_in_local_cache_v3(key_dir,
key.offset);
+ auto old_key_dir = storage.get_path_in_local_cache_v2(key.hash,
key.meta.expiration_time);
+ create_regular_file(block_path, 'r');
+ ASSERT_FALSE(fs::exists(old_key_dir));
+
+ ASSERT_TRUE(storage.remove(key).ok());
+
+ EXPECT_FALSE(fs::exists(block_path));
+ EXPECT_FALSE(fs::exists(key_dir));
+ EXPECT_FALSE(fs::exists(old_key_dir));
+}
+
TEST_F(FSFileCacheLeakCleanerTest,
snapshot_metadata_for_hash_offsets_handles_missing_hash) {
auto dir = prepare_test_dir();
FileCacheSettings settings = default_settings();
diff --git a/be/test/io/fs/local_file_system_test.cpp
b/be/test/io/fs/local_file_system_test.cpp
index 450c3daea8f..5947dffef9c 100644
--- a/be/test/io/fs/local_file_system_test.cpp
+++ b/be/test/io/fs/local_file_system_test.cpp
@@ -212,6 +212,30 @@ TEST_F(LocalFileSystemTest, Delete) {
ASSERT_TRUE(check_exist(fmt::format("{}/dir/dir1", test_dir))); // Parent
should exist
}
+TEST_F(LocalFileSystemTest, DeleteEmptyDirectory) {
+ auto empty_dir = fmt::format("{}/empty_dir", test_dir);
+ auto st = io::global_local_filesystem()->create_directory(empty_dir);
+ ASSERT_TRUE(st.ok()) << st;
+
+ st = io::global_local_filesystem()->delete_empty_directory(empty_dir);
+ ASSERT_TRUE(st.ok()) << st;
+ ASSERT_FALSE(check_exist(empty_dir));
+ st = io::global_local_filesystem()->delete_empty_directory(empty_dir);
+ ASSERT_TRUE(st.ok()) << st;
+
+ auto non_empty_dir = fmt::format("{}/non_empty_dir", test_dir);
+ auto child_file = fmt::format("{}/child", non_empty_dir);
+ st = io::global_local_filesystem()->create_directory(non_empty_dir);
+ ASSERT_TRUE(st.ok()) << st;
+ st = save_string_file(child_file, "abc");
+ ASSERT_TRUE(st.ok()) << st;
+
+ st = io::global_local_filesystem()->delete_empty_directory(non_empty_dir);
+ ASSERT_FALSE(st.ok()) << st;
+ ASSERT_TRUE(check_exist(non_empty_dir));
+ ASSERT_TRUE(check_exist(child_file));
+}
+
TEST_F(LocalFileSystemTest, AbnormalFileWriter) {
auto fname = fmt::format("{}/abc", test_dir);
{
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
