From 5daa426a18bf5349584154b51a5404f2b1b69d1a Mon Sep 17 00:00:00 2001 From: sdong Date: Wed, 24 Jul 2019 12:04:58 -0700 Subject: [PATCH] Fix regression bug of Auto rolling logger when handling failures (#5622) Summary: Auto roll logger fails to handle file creation error in the correct way, which may expose to seg fault condition to users. Fix it. Pull Request resolved: https://github.com/facebook/rocksdb/pull/5622 Test Plan: Add a unit test on creating file under a non-existing directory. The test fails without the fix. Differential Revision: D16460853 fbshipit-source-id: e96da4bef4f16db171ea04a11b2ec5a9448ddbde --- logging/auto_roll_logger.cc | 5 ++--- logging/auto_roll_logger_test.cc | 9 +++++++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/logging/auto_roll_logger.cc b/logging/auto_roll_logger.cc index ec240f5a3..223dfbe30 100644 --- a/logging/auto_roll_logger.cc +++ b/logging/auto_roll_logger.cc @@ -46,9 +46,8 @@ AutoRollLogger::AutoRollLogger(Env* env, const std::string& dbname, } GetExistingFiles(); ResetLogger(); - s = TrimOldLogFiles(); - if (!status_.ok()) { - status_ = s; + if (status_.ok()) { + status_ = TrimOldLogFiles(); } } diff --git a/logging/auto_roll_logger_test.cc b/logging/auto_roll_logger_test.cc index fa668114c..dd279d62a 100644 --- a/logging/auto_roll_logger_test.cc +++ b/logging/auto_roll_logger_test.cc @@ -635,6 +635,15 @@ TEST_F(AutoRollLoggerTest, LogFileExistence) { delete db; } +TEST_F(AutoRollLoggerTest, FileCreateFailure) { + Options options; + options.max_log_file_size = 100 * 1024 * 1024; + options.db_log_dir = "/a/dir/does/not/exist/at/all"; + + std::shared_ptr logger; + ASSERT_NOK(CreateLoggerFromOptions("", options, &logger)); + ASSERT_TRUE(!logger); +} } // namespace rocksdb int main(int argc, char** argv) {