Close the Logger before rolling to next one in AutoRollLogger (#10488)

Summary:
Close the existing logger first to release the existing
handle before renaming the file using the file system.
Since `AutoRollLogger::Flush` pinned down the `logger_`, `logger_` can't be closed unless its
the last reference otherwise it  gives seg fault during Flush on file
that has been closed.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/10488

Test Plan: CircleCI jobs

Reviewed By: ajkr

Differential Revision: D38469249

Pulled By: akankshamahajan15

fbshipit-source-id: dfbdb89b4ac37639aefcc503526f24753445fd3f

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?

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
+  // (2) New logger will be cut in AutoRollLogger::RollLogFile only when flush
-  //     condition case, which is executing the flush with the pinned (old)
+  //     is completed and reference to pinned logger is released.
-  //     logger after auto-roll logger has cut over to a new logger.
   // (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"},
+        "AutoRollLoggerTest::LogFlushWhileRolling:PreRollAndPostThreadInit"}},
-       {"EnvLogger::Flush:Begin1",
-        "AutoRollLogger::ResetLogger:BeforeNewLogger"},
-       {"AutoRollLogger::ResetLogger:AfterNewLogger",
-        "EnvLogger::Flush:Begin2"}},
       {{"AutoRollLogger::Flush:PinnedLogger", "EnvLogger::Flush:Begin1"},
        {"AutoRollLogger::Flush:PinnedLogger", "EnvLogger::Flush:Begin2"}});
   ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing();