From c004fe47f837fa546c594e199d43f2e76a34bc9e Mon Sep 17 00:00:00 2001 From: Ashin Gau Date: Wed, 22 May 2024 07:49:13 +0800 Subject: [PATCH] [opt](file cache) set failed_if_exists=true when create the cache directory (#35096) follow up: #34935 There are many segment files for a cache remote file, so the cached directory will create many times. Therefore, we should call `fs->create_directory(dir, failed_if_exists=false)` instead of `fs->create_directory(dir, failed_if_exists=true)`. When `failed_if_exists=true`, the next line `if (!st.ok() && !st.is())` try to prevent the `ALREADY_EXIST` errors. --- be/src/common/status.h | 2 -- be/src/io/cache/fs_file_cache_storage.cpp | 2 +- be/src/io/fs/local_file_system.cpp | 20 ++++++++++---------- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/be/src/common/status.h b/be/src/common/status.h index 043eddd32293c1..99b591639e9eab 100644 --- a/be/src/common/status.h +++ b/be/src/common/status.h @@ -412,7 +412,6 @@ class [[nodiscard]] Status { if (stacktrace && ErrorCode::error_states[abs(code)].stacktrace) { // Delete the first one frame pointers, which are inside the status.h status._err_msg->_stack = get_stack_trace(1); - LOG(WARNING) << "meet error status: " << status; // may print too many stacks. } #endif return status; @@ -431,7 +430,6 @@ class [[nodiscard]] Status { #ifdef ENABLE_STACKTRACE if (stacktrace && ErrorCode::error_states[abs(code)].stacktrace) { status._err_msg->_stack = get_stack_trace(1); - LOG(WARNING) << "meet error status: " << status; // may print too many stacks. } #endif return status; diff --git a/be/src/io/cache/fs_file_cache_storage.cpp b/be/src/io/cache/fs_file_cache_storage.cpp index 18c26fa05d4b81..c66c73516447df 100644 --- a/be/src/io/cache/fs_file_cache_storage.cpp +++ b/be/src/io/cache/fs_file_cache_storage.cpp @@ -118,7 +118,7 @@ Status FSFileCacheStorage::append(const FileCacheKey& key, const Slice& value) { writer = iter->second.get(); } else { std::string dir = get_path_in_local_cache(key.hash, key.meta.expiration_time); - auto st = fs->create_directory(dir, true); + auto st = fs->create_directory(dir, false); if (!st.ok() && !st.is()) { return st; } diff --git a/be/src/io/fs/local_file_system.cpp b/be/src/io/fs/local_file_system.cpp index d7b0cce14fffc0..9620674d2e951a 100644 --- a/be/src/io/fs/local_file_system.cpp +++ b/be/src/io/fs/local_file_system.cpp @@ -93,17 +93,17 @@ Status LocalFileSystem::open_file_impl(const Path& file, FileReaderSPtr* reader, } Status LocalFileSystem::create_directory_impl(const Path& dir, bool failed_if_exists) { - if (failed_if_exists) { - bool exists = true; - RETURN_IF_ERROR(exists_impl(dir, &exists)); - if (exists) { - return Status::AlreadyExist("failed to create {}, already exists", dir.native()); - } + bool exists = true; + RETURN_IF_ERROR(exists_impl(dir, &exists)); + if (exists && failed_if_exists) { + return Status::AlreadyExist("failed to create {}, already exists", dir.native()); } - std::error_code ec; - std::filesystem::create_directories(dir, ec); - if (ec) { - return localfs_error(ec, fmt::format("failed to create {}", dir.native())); + if (!exists) { + std::error_code ec; + std::filesystem::create_directories(dir, ec); + if (ec) { + return localfs_error(ec, fmt::format("failed to create {}", dir.native())); + } } return Status::OK(); }