From db85f6018e9ad70d06bab584353b118e113732f6 Mon Sep 17 00:00:00 2001 From: zhengyu Date: Wed, 5 Mar 2025 16:16:53 +0800 Subject: [PATCH 1/2] [fix](cloud) speed up file cache initializtion The initialization of the file cache involves asynchronous loading logic and synchronous upgrade directories. The latter mainly handles the conversion from version1 to version2 format and some fallback logic for problematic directories, which involves a large number of directory traversals and can be very slow. Previously, in PR #44429, we changed the initialization of multiple cache directories from parallel to serial to avoid the disorder caused by concurrent initialization, which led to a long cache initialization time and affected the startup speed of the BE. We found that the upgrade directory is only meaningful during upgrades and does not need to be executed on every restart. Therefore, if we detect that the version file has been successfully written, we consider the cache directory to have completed the upgrade and skip these redundant directory traversals Of course, we could further optimize the directory traversal process to make it asynchronous and not block the BE startup. However, this would result in three concurrent operations on the file system: asynchronous loading, asynchronous updating, and lazy loading on query. This would increase code complexity, the likelihood of errors, and the difficulty of troubleshooting. Considering that old clusters are not very common and that a cluster only needs to go through such an upgrade once in its lifecycle, we assessed that this optimization would have low cost-effectiveness and decided not to pursue it. Signed-off-by: zhengyu --- be/src/io/cache/fs_file_cache_storage.cpp | 78 +++++++++++++++-------- 1 file changed, 52 insertions(+), 26 deletions(-) diff --git a/be/src/io/cache/fs_file_cache_storage.cpp b/be/src/io/cache/fs_file_cache_storage.cpp index 43a4a541cb0dc8..3dc54f9f30269a 100644 --- a/be/src/io/cache/fs_file_cache_storage.cpp +++ b/be/src/io/cache/fs_file_cache_storage.cpp @@ -309,15 +309,31 @@ std::string FSFileCacheStorage::get_path_in_local_cache(const UInt128Wrapper& va } Status FSFileCacheStorage::upgrade_cache_dir_if_necessary() const { - /// version 1.0: cache_base_path / key / offset - /// version 2.0: cache_base_path / key_prefix / key / offset + /* + * If use version2 but was version 1, do upgrade: + * + * Action I: + * version 1.0: cache_base_path / key / offset + * version 2.0: cache_base_path / key_prefix / key / offset + * + * Action II: + * add '_0' to hash dir + * + * Note: This is a sync operation with tons of IOs, so it may affect BE + * boot time heavily. Fortunately, Action I & II will only happen when + * upgrading (once in the cluster life time). + */ + std::string version; + std::error_code ec; + RETURN_IF_ERROR(read_file_cache_version(&version)); if (USE_CACHE_VERSION2 && version != "2.0") { // move directories format as version 2.0 - std::error_code ec; std::filesystem::directory_iterator key_it {_cache_base_path, ec}; if (ec) { + LOG(WARNING) << "Failed to list directory: " << _cache_base_path + << ", error: " << ec.message(); return Status::InternalError("Failed to list dir {}: {}", _cache_base_path, ec.message()); } @@ -328,31 +344,43 @@ Status FSFileCacheStorage::upgrade_cache_dir_if_necessary() const { std::string key_prefix = Path(_cache_base_path) / cache_key.substr(0, KEY_PREFIX_LENGTH); bool exists = false; - RETURN_IF_ERROR(fs->exists(key_prefix, &exists)); + auto exists_status = fs->exists(key_prefix, &exists); + if (!exists_status.ok()) { + LOG(WARNING) << "Failed to check directory existence: " << key_prefix + << ", error: " << exists_status.to_string(); + return exists_status; + } if (!exists) { - RETURN_IF_ERROR(fs->create_directory(key_prefix)); + auto create_status = fs->create_directory(key_prefix); + if (!create_status.ok()) { + LOG(WARNING) << "Failed to create directory: " << key_prefix + << ", error: " << create_status.to_string(); + return create_status; + } + } + auto rename_status = fs->rename(key_it->path(), key_prefix / cache_key); + if (!rename_status.ok()) { + LOG(WARNING) + << "Failed to rename directory from " << key_it->path().native() + << " to " << (key_prefix / cache_key).native() + << ", error: " << rename_status.to_string(); + return rename_status; } - RETURN_IF_ERROR(fs->rename(key_it->path(), key_prefix / cache_key)); } } } - if (!write_file_cache_version().ok()) { - return Status::InternalError("Failed to write version hints for file cache"); - } - } - auto rebuild_dir = [&](std::filesystem::directory_iterator& upgrade_key_it) -> Status { - for (; upgrade_key_it != std::filesystem::directory_iterator(); ++upgrade_key_it) { - if (upgrade_key_it->path().filename().native().find('_') == std::string::npos) { - RETURN_IF_ERROR(fs->delete_directory(upgrade_key_it->path().native() + "_0")); - RETURN_IF_ERROR( - fs->rename(upgrade_key_it->path(), upgrade_key_it->path().native() + "_0")); + auto rebuild_dir = [&](std::filesystem::directory_iterator& upgrade_key_it) -> Status { + for (; upgrade_key_it != std::filesystem::directory_iterator(); ++upgrade_key_it) { + if (upgrade_key_it->path().filename().native().find('_') == std::string::npos) { + RETURN_IF_ERROR(fs->delete_directory(upgrade_key_it->path().native() + "_0")); + RETURN_IF_ERROR(fs->rename(upgrade_key_it->path(), + upgrade_key_it->path().native() + "_0")); + } } - } - return Status::OK(); - }; - std::error_code ec; - if constexpr (USE_CACHE_VERSION2) { + return Status::OK(); + }; + std::filesystem::directory_iterator key_prefix_it {_cache_base_path, ec}; if (ec) [[unlikely]] { LOG(WARNING) << ec.message(); @@ -374,13 +402,11 @@ Status FSFileCacheStorage::upgrade_cache_dir_if_necessary() const { } RETURN_IF_ERROR(rebuild_dir(key_it)); } - } else { - std::filesystem::directory_iterator key_it {_cache_base_path, ec}; - if (ec) [[unlikely]] { - return Status::IOError(ec.message()); + if (!write_file_cache_version().ok()) { + return Status::InternalError("Failed to write version hints for file cache"); } - RETURN_IF_ERROR(rebuild_dir(key_it)); } + return Status::OK(); } From 0b45bab3c9bcaada3645b731ce83ea0217a3525a Mon Sep 17 00:00:00 2001 From: zhengyu Date: Thu, 6 Mar 2025 16:29:06 +0800 Subject: [PATCH 2/2] response to the reviewer Signed-off-by: zhengyu --- be/src/io/cache/fs_file_cache_storage.cpp | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/be/src/io/cache/fs_file_cache_storage.cpp b/be/src/io/cache/fs_file_cache_storage.cpp index 3dc54f9f30269a..c6df21aceedb9e 100644 --- a/be/src/io/cache/fs_file_cache_storage.cpp +++ b/be/src/io/cache/fs_file_cache_storage.cpp @@ -309,7 +309,7 @@ std::string FSFileCacheStorage::get_path_in_local_cache(const UInt128Wrapper& va } Status FSFileCacheStorage::upgrade_cache_dir_if_necessary() const { - /* + /* * If use version2 but was version 1, do upgrade: * * Action I: @@ -326,8 +326,13 @@ Status FSFileCacheStorage::upgrade_cache_dir_if_necessary() const { std::string version; std::error_code ec; + int rename_count = 0; + auto start_time = std::chrono::steady_clock::now(); RETURN_IF_ERROR(read_file_cache_version(&version)); + LOG(INFO) << "Checking cache version upgrade. Current version: " << version + << ", target version: 2.0, need upgrade: " + << (USE_CACHE_VERSION2 && version != "2.0"); if (USE_CACHE_VERSION2 && version != "2.0") { // move directories format as version 2.0 std::filesystem::directory_iterator key_it {_cache_base_path, ec}; @@ -359,7 +364,9 @@ Status FSFileCacheStorage::upgrade_cache_dir_if_necessary() const { } } auto rename_status = fs->rename(key_it->path(), key_prefix / cache_key); - if (!rename_status.ok()) { + if (rename_status.ok()) { + ++rename_count; + } else { LOG(WARNING) << "Failed to rename directory from " << key_it->path().native() << " to " << (key_prefix / cache_key).native() @@ -374,8 +381,12 @@ Status FSFileCacheStorage::upgrade_cache_dir_if_necessary() const { for (; upgrade_key_it != std::filesystem::directory_iterator(); ++upgrade_key_it) { if (upgrade_key_it->path().filename().native().find('_') == std::string::npos) { RETURN_IF_ERROR(fs->delete_directory(upgrade_key_it->path().native() + "_0")); - RETURN_IF_ERROR(fs->rename(upgrade_key_it->path(), - upgrade_key_it->path().native() + "_0")); + auto rename_status = fs->rename(upgrade_key_it->path(), + upgrade_key_it->path().native() + "_0"); + if (rename_status.ok()) { + ++rename_count; + } + RETURN_IF_ERROR(rename_status); } } return Status::OK(); @@ -407,6 +418,10 @@ Status FSFileCacheStorage::upgrade_cache_dir_if_necessary() const { } } + auto end_time = std::chrono::steady_clock::now(); + auto duration = std::chrono::duration_cast(end_time - start_time); + LOG(INFO) << "Cache directory upgrade completed. Total files renamed: " << rename_count + << ", Time taken: " << duration.count() << "ms"; return Status::OK(); }