From 6a369438d5d90e67980bc4c4c9fef8cbd376dd8a Mon Sep 17 00:00:00 2001 From: "gd.zhou" Date: Wed, 29 Apr 2026 17:02:28 +0800 Subject: [PATCH] [bugfix] Fix GetInfoLogFiles using wrong path for log file prefix When db_log_dir is set and dbname is a relative path, GetInfoLogFiles incorrectly uses dbname instead of db_absolute_path to build the log file prefix. This causes mismatch with InfoLogFileName which uses db_absolute_path, resulting in failure to identify existing log files during AutoRollLogger restart. Consequently, log files accumulate beyond the keep_log_file_num limit. Root cause: GetInfoLogFiles used dbname for InfoLogPrefix construction while InfoLogFileName uses db_absolute_path. When these differ (e.g., relative dbname vs absolute cwd), the prefixes don't match. Fix: Add db_absolute_path parameter to GetInfoLogFiles and use it for InfoLogPrefix construction, consistent with InfoLogFileName behavior. Changes: - file/filename.h: Add db_absolute_path parameter - file/filename.cc: Use db_absolute_path for InfoLogPrefix - logging/auto_roll_logger.cc: Pass db_absolute_path_ to GetInfoLogFiles - utilities/convenience/info_log_finder.cc: Get and pass db_absolute_path - logging/auto_roll_logger_test.cc: Add test case GetInfoLogFilesPrefixMismatch --- file/filename.cc | 3 +- file/filename.h | 3 ++ logging/auto_roll_logger.cc | 3 +- logging/auto_roll_logger_test.cc | 43 ++++++++++++++++++++++++ utilities/convenience/info_log_finder.cc | 12 ++++++- 5 files changed, 61 insertions(+), 3 deletions(-) diff --git a/file/filename.cc b/file/filename.cc index 1e04c73395e7..e9fca256a34a 100644 --- a/file/filename.cc +++ b/file/filename.cc @@ -471,6 +471,7 @@ IOStatus SyncManifest(const ImmutableDBOptions* db_options, Status GetInfoLogFiles(const std::shared_ptr& fs, const std::string& db_log_dir, const std::string& dbname, + const std::string& db_absolute_path, std::string* parent_dir, std::vector* info_log_list) { assert(parent_dir != nullptr); @@ -484,7 +485,7 @@ Status GetInfoLogFiles(const std::shared_ptr& fs, *parent_dir = dbname; } - InfoLogPrefix info_log_prefix(!db_log_dir.empty(), dbname); + InfoLogPrefix info_log_prefix(!db_log_dir.empty(), db_absolute_path); std::vector file_names; Status s = fs->GetChildren(*parent_dir, IOOptions(), &file_names, nullptr); diff --git a/file/filename.h b/file/filename.h index 2eb125b6a178..31566d24e945 100644 --- a/file/filename.h +++ b/file/filename.h @@ -178,9 +178,12 @@ extern IOStatus SyncManifest(const ImmutableDBOptions* db_options, // The list only contains file name. The parent directory name is stored // in `parent_dir`. // `db_log_dir` should be the one as in options.db_log_dir +// `db_absolute_path` is the absolute path of dbname, used for generating +// the correct log file prefix when db_log_dir is set. extern Status GetInfoLogFiles(const std::shared_ptr& fs, const std::string& db_log_dir, const std::string& dbname, + const std::string& db_absolute_path, std::string* parent_dir, std::vector* file_names); diff --git a/logging/auto_roll_logger.cc b/logging/auto_roll_logger.cc index 9e9ad45aee0f..f84fe8af26f1 100644 --- a/logging/auto_roll_logger.cc +++ b/logging/auto_roll_logger.cc @@ -122,7 +122,8 @@ void AutoRollLogger::GetExistingFiles() { std::string parent_dir; std::vector info_log_files; Status s = - GetInfoLogFiles(fs_, db_log_dir_, dbname_, &parent_dir, &info_log_files); + GetInfoLogFiles(fs_, db_log_dir_, dbname_, db_absolute_path_, + &parent_dir, &info_log_files); if (status_.ok()) { status_ = s; } diff --git a/logging/auto_roll_logger_test.cc b/logging/auto_roll_logger_test.cc index 3d0ec1763f28..60455cc1354e 100644 --- a/logging/auto_roll_logger_test.cc +++ b/logging/auto_roll_logger_test.cc @@ -721,6 +721,49 @@ TEST_F(AutoRollLoggerTest, RenameError) { } } +TEST_F(AutoRollLoggerTest, GetInfoLogFilesPrefixMismatch) { + + InitTestDb(); + const size_t kFileNum = 3; + const size_t kMaxFileSize = 512; + + + std::string relative_dbname = "test_relative_db"; + std::string db_log_dir = kTestDir; + + + ASSERT_OK(default_env->CreateDirIfMissing(relative_dbname)); + + + { + AutoRollLogger logger(FileSystem::Default(), SystemClock::Default(), + relative_dbname, db_log_dir, kMaxFileSize, 0, kFileNum); + + RollNTimesBySize(&logger, kFileNum + 2, kMaxFileSize); + } + std::vector files = GetLogFiles(); + ASSERT_EQ(kFileNum, files.size()) + << "Log files should be created"; + + { + AutoRollLogger logger(FileSystem::Default(), SystemClock::Default(), + relative_dbname, db_log_dir, kMaxFileSize, 0, kFileNum); + + RollNTimesBySize(&logger, 1, kMaxFileSize); + } + + + files = GetLogFiles(); + + ASSERT_EQ(kFileNum, files.size()) + << "Log files exceed keep_log_file_num limit. " + << "Bug: GetInfoLogFiles uses wrong prefix when db_log_dir is set. " + << "Expected: " << kFileNum << ", Actual: " << files.size(); + + default_env->DeleteDir(relative_dbname).PermitUncheckedError(); + CleanupLogFiles(); +} + } // namespace ROCKSDB_NAMESPACE int main(int argc, char** argv) { diff --git a/utilities/convenience/info_log_finder.cc b/utilities/convenience/info_log_finder.cc index fe62fd56168a..afcdceb9f45d 100644 --- a/utilities/convenience/info_log_finder.cc +++ b/utilities/convenience/info_log_finder.cc @@ -20,7 +20,17 @@ Status GetInfoLogList(DB* db, std::vector* info_log_list) { } std::string parent_path; const Options& options = db->GetOptions(); + std::string dbname = db->GetName(); + + std::string db_absolute_path; + Status s = options.env->GetAbsolutePath(dbname, &db_absolute_path); + if (s.IsNotSupported()) { + db_absolute_path = dbname; + } else if (!s.ok()) { + return s; + } + return GetInfoLogFiles(options.env->GetFileSystem(), options.db_log_dir, - db->GetName(), &parent_path, info_log_list); + dbname, db_absolute_path, &parent_path, info_log_list); } } // namespace ROCKSDB_NAMESPACE