diff --git a/logging/auto_roll_logger.cc b/logging/auto_roll_logger.cc index a362b6f23..66f346518 100644 --- a/logging/auto_roll_logger.cc +++ b/logging/auto_roll_logger.cc @@ -94,6 +94,18 @@ void AutoRollLogger::RollLogFile() { dbname_, now, db_absolute_path_, db_log_dir_); now++; } while (fs_->FileExists(old_fname, io_options_, &io_context_).ok()); + // Wait for logger_ reference count to turn to 1 as it might be pinned by + // Flush. Pinned Logger can't be closed till Flush is completed on that + // Logger. + while (logger_.use_count() > 1) { + } + // Close the existing logger first to release the existing handle + // before renaming the file using the file system. If this call + // fails there is nothing much we can do and we will continue with the + // rename and hence ignoring the result status. + if (logger_) { + logger_->Close().PermitUncheckedError(); + } Status s = fs_->RenameFile(log_fname_, old_fname, io_options_, &io_context_); if (!s.ok()) { // What should we do on error? diff --git a/logging/auto_roll_logger_test.cc b/logging/auto_roll_logger_test.cc index a3a89ad91..f95f4215d 100644 --- a/logging/auto_roll_logger_test.cc +++ b/logging/auto_roll_logger_test.cc @@ -468,18 +468,13 @@ TEST_F(AutoRollLoggerTest, LogFlushWhileRolling) { // (1) Need to pin the old logger before beginning the roll, as rolling grabs // the mutex, which would prevent us from accessing the old logger. This // also marks flush_thread with AutoRollLogger::Flush:PinnedLogger. - // (2) Need to reset logger during EnvLogger::Flush() to exercise a race - // condition case, which is executing the flush with the pinned (old) - // logger after auto-roll logger has cut over to a new logger. + // (2) New logger will be cut in AutoRollLogger::RollLogFile only when flush + // is completed and reference to pinned logger is released. // (3) EnvLogger::Flush() happens in both threads but its SyncPoints only // are enabled in flush_thread (the one pinning the old logger). ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->LoadDependencyAndMarkers( {{"AutoRollLogger::Flush:PinnedLogger", - "AutoRollLoggerTest::LogFlushWhileRolling:PreRollAndPostThreadInit"}, - {"EnvLogger::Flush:Begin1", - "AutoRollLogger::ResetLogger:BeforeNewLogger"}, - {"AutoRollLogger::ResetLogger:AfterNewLogger", - "EnvLogger::Flush:Begin2"}}, + "AutoRollLoggerTest::LogFlushWhileRolling:PreRollAndPostThreadInit"}}, {{"AutoRollLogger::Flush:PinnedLogger", "EnvLogger::Flush:Begin1"}, {"AutoRollLogger::Flush:PinnedLogger", "EnvLogger::Flush:Begin2"}}); ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing();