From 52bccacabfacd7e651575b04da93726ceceb25c7 Mon Sep 17 00:00:00 2001 From: Xin Liao Date: Mon, 18 May 2026 15:01:35 +0800 Subject: [PATCH] [fix](filecache) avoid recursive empty dir cleanup --- be/src/io/cache/fs_file_cache_storage.cpp | 7 +++++-- be/src/io/fs/local_file_system.cpp | 20 +++++++++++++++++++ be/src/io/fs/local_file_system.h | 3 +++ be/test/io/fs/local_file_system_test.cpp | 24 +++++++++++++++++++++++ 4 files changed, 52 insertions(+), 2 deletions(-) diff --git a/be/src/io/cache/fs_file_cache_storage.cpp b/be/src/io/cache/fs_file_cache_storage.cpp index 48448e142ea04f..2362a3acb52b32 100644 --- a/be/src/io/cache/fs_file_cache_storage.cpp +++ b/be/src/io/cache/fs_file_cache_storage.cpp @@ -240,8 +240,11 @@ Status FSFileCacheStorage::remove(const FileCacheKey& key) { std::vector files; bool exists {false}; RETURN_IF_ERROR(fs->list(dir, true, &files, &exists)); - if (files.empty()) { - RETURN_IF_ERROR(fs->delete_directory(dir)); + if (exists && files.empty()) { + auto st = fs->delete_empty_directory(dir); + if (!st.ok()) { + LOG_WARNING("failed to remove cache directory {}", 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 6dc366a0bfd0ed..c56f62f3e42288 100644 --- a/be/src/io/fs/local_file_system.cpp +++ b/be/src/io/fs/local_file_system.cpp @@ -166,6 +166,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 c1ffbd22949252..b1546bc473426f 100644 --- a/be/src/io/fs/local_file_system.h +++ b/be/src/io/fs/local_file_system.h @@ -63,6 +63,8 @@ class LocalFileSystem final : public FileSystem { 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 @@ class LocalFileSystem final : public FileSystem { 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& 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/fs/local_file_system_test.cpp b/be/test/io/fs/local_file_system_test.cpp index 450c3daea8f2f6..5947dffef9cf16 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); {