Clean and expose CreateLoggerFromOptions

Summary:
CreateLoggerFromOptions have some parameters like  db_log_dir and env, these parameters are redundant since they already exist in DBOptions

this patch remove the redundant parameters and expose CreateLoggerFromOptions to users

Test Plan: make check

Reviewers: igor, anthony, yhchiang, rven, kradhakrishnan, sdong

Reviewed By: sdong

Subscribers: dhruba, hermanlee4

Differential Revision: https://reviews.facebook.net/D49713

HISTORY.md

@@ -6,6 +6,7 @@
 * The need-compaction hint given by TablePropertiesCollector::NeedCompact() will be persistent and recoverable after DB recovery. This introduces a breaking format change. If you use this experimental feature, including NewCompactOnDeletionCollectorFactory() in the new version, you may not be able to directly downgrade the DB back to version 4.0 or lower.
 * TablePropertiesCollectorFactory::CreateTablePropertiesCollector() now takes an option Context, containing the information of column family ID for the file being written.
 * Remove DefaultCompactionFilterFactory.
+* Introduce CreateLoggerFromOptions(), this function create a Logger for provided DBOptions. 
 
 ## 4.1.0 (10/8/2015)
 ### New Features

db/db_impl.cc

@@ -133,8 +133,7 @@ DBOptions SanitizeOptions(const std::string& dbname, const DBOptions& src) {
   }
 
   if (result.info_log == nullptr) {
-    Status s = CreateLoggerFromOptions(dbname, result.db_log_dir, src.env,
+    Status s = CreateLoggerFromOptions(dbname, result, &result.info_log);
-                                       result, &result.info_log);
     if (!s.ok()) {
       // No place suitable for logging
       result.info_log = nullptr;

include/rocksdb/options.h

@@ -1346,6 +1346,11 @@ extern Options GetOptions(size_t total_write_buffer_limit,
                           int write_amplification_threshold = 32,
                           uint64_t target_db_size = 68719476736 /* 64GB */);
 
+// Create a Logger from provided DBOptions
+extern Status CreateLoggerFromOptions(const std::string& dbname,
+                                      const DBOptions& options,
+                                      std::shared_ptr<Logger>* logger);
+
 // CompactionOptions are used in CompactFiles() call.
 struct CompactionOptions {
   // Compaction output compression type

util/auto_roll_logger.cc

@@ -122,22 +122,25 @@ bool AutoRollLogger::LogExpired() {
   return cached_now >= ctime_ + kLogFileTimeToRoll;
 }
 
-Status CreateLoggerFromOptions(
+Status CreateLoggerFromOptions(const std::string& dbname,
-    const std::string& dbname,
+                               const DBOptions& options,
-    const std::string& db_log_dir,
+                               std::shared_ptr<Logger>* logger) {
-    Env* env,
+  if (options.info_log) {
-    const DBOptions& options,
+    *logger = options.info_log;
-    std::shared_ptr<Logger>* logger) {
+    return Status::OK();
+  }
+
+  Env* env = options.env;
   std::string db_absolute_path;
   env->GetAbsolutePath(dbname, &db_absolute_path);
-  std::string fname = InfoLogFileName(dbname, db_absolute_path, db_log_dir);
+  std::string fname =
+      InfoLogFileName(dbname, db_absolute_path, options.db_log_dir);
 
   env->CreateDirIfMissing(dbname);  // In case it does not exist
   // Currently we only support roll by time-to-roll and log size
   if (options.log_file_time_to_roll > 0 || options.max_log_file_size > 0) {
     AutoRollLogger* result = new AutoRollLogger(
-        env, dbname, db_log_dir,
+        env, dbname, options.db_log_dir, options.max_log_file_size,
-        options.max_log_file_size,
         options.log_file_time_to_roll, options.info_log_level);
     Status s = result->GetStatus();
     if (!s.ok()) {
@@ -148,8 +151,9 @@ Status CreateLoggerFromOptions(
     return s;
   } else {
     // Open a log file in the same directory as the db
-    env->RenameFile(fname, OldInfoLogFileName(dbname, env->NowMicros(),
+    env->RenameFile(
-                                              db_absolute_path, db_log_dir));
+        fname, OldInfoLogFileName(dbname, env->NowMicros(), db_absolute_path,
+                                  options.db_log_dir));
     auto s = env->NewLogger(fname, logger);
     if (logger->get() != nullptr) {
       (*logger)->SetInfoLogLevel(options.info_log_level);

util/auto_roll_logger.h

@@ -8,6 +8,7 @@
 
 #pragma once
 #include <list>
+#include <string>
 
 #include "db/filename.h"
 #include "port/port.h"
@@ -104,11 +105,8 @@ class AutoRollLogger : public Logger {
 };
 
 // Facade to craete logger automatically
-Status CreateLoggerFromOptions(
+Status CreateLoggerFromOptions(const std::string& dbname,
-    const std::string& dbname,
+                               const DBOptions& options,
-    const std::string& db_log_dir,
+                               std::shared_ptr<Logger>* logger);
-    Env* env,
-    const DBOptions& options,
-    std::shared_ptr<Logger>* logger);
 
 }  // namespace rocksdb

util/auto_roll_logger_test.cc

@@ -221,13 +221,13 @@ TEST_F(AutoRollLoggerTest, CreateLoggerFromOptions) {
   shared_ptr<Logger> logger;
 
   // Normal logger
-  ASSERT_OK(CreateLoggerFromOptions(kTestDir, "", env, options, &logger));
+  ASSERT_OK(CreateLoggerFromOptions(kTestDir, options, &logger));
   ASSERT_TRUE(dynamic_cast<PosixLogger*>(logger.get()));
 
   // Only roll by size
   InitTestDb();
   options.max_log_file_size = 1024;
-  ASSERT_OK(CreateLoggerFromOptions(kTestDir, "", env, options, &logger));
+  ASSERT_OK(CreateLoggerFromOptions(kTestDir, options, &logger));
   AutoRollLogger* auto_roll_logger =
     dynamic_cast<AutoRollLogger*>(logger.get());
   ASSERT_TRUE(auto_roll_logger);
@@ -239,7 +239,7 @@ TEST_F(AutoRollLoggerTest, CreateLoggerFromOptions) {
   InitTestDb();
   options.max_log_file_size = 0;
   options.log_file_time_to_roll = 2;
-  ASSERT_OK(CreateLoggerFromOptions(kTestDir, "", env, options, &logger));
+  ASSERT_OK(CreateLoggerFromOptions(kTestDir, options, &logger));
   auto_roll_logger =
     dynamic_cast<AutoRollLogger*>(logger.get());
   RollLogFileByTimeTest(
@@ -250,7 +250,7 @@ TEST_F(AutoRollLoggerTest, CreateLoggerFromOptions) {
   InitTestDb();
   options.max_log_file_size = 1024 * 5;
   options.log_file_time_to_roll = 2;
-  ASSERT_OK(CreateLoggerFromOptions(kTestDir, "", env, options, &logger));
+  ASSERT_OK(CreateLoggerFromOptions(kTestDir, options, &logger));
   auto_roll_logger =
     dynamic_cast<AutoRollLogger*>(logger.get());
   RollLogFileBySizeTest(

utilities/backupable/backupable_db_test.cc

@@ -424,9 +424,14 @@ class BackupableDBTest : public testing::Test {
     options_.write_buffer_size = 1 << 17; // 128KB
     options_.env = test_db_env_.get();
     options_.wal_dir = dbname_;
+
+    // Create logger
+    DBOptions logger_options;
+    logger_options.env = env_;
+    logger_options.db_log_dir = backupdir_;
+    CreateLoggerFromOptions(dbname_, logger_options, &logger_);
+
     // set up backup db options
-    CreateLoggerFromOptions(dbname_, backupdir_, env_,
-                            DBOptions(), &logger_);
     backupable_options_.reset(new BackupableDBOptions(
         backupdir_, test_backup_env_.get(), true, logger_.get(), true));