Don't sync manifest when disableDataSync = true

Summary: As we discussed offline

Test Plan: compiles

Reviewers: yhchiang, sdong, ljin, dhruba

Reviewed By: sdong

Subscribers: leveldb

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

HISTORY.md

@@ -6,6 +6,7 @@
 
 ### Behavior changes
 * We have refactored our system of stalling writes.  Any stall-related statistics' meanings are changed. Instead of per-write stall counts, we now count stalls per-epoch, where epochs are periods between flushes and compactions. You'll find more information in our Tuning Perf Guide once we release RocksDB 3.6.
+* When disableDataSync=true, we no longer sync the MANIFEST file.
 
 ----- Past Releases -----
 

db/db_test.cc

@@ -151,6 +151,8 @@ class SpecialEnv : public EnvWrapper {
 
   std::atomic<int64_t> bytes_written_;
 
+  std::atomic<int> sync_counter_;
+
   explicit SpecialEnv(Env* base) : EnvWrapper(base) {
     delay_sstable_sync_.Release_Store(nullptr);
     drop_writes_.Release_Store(nullptr);
@@ -162,6 +164,7 @@ class SpecialEnv : public EnvWrapper {
     manifest_write_error_.Release_Store(nullptr);
     log_write_error_.Release_Store(nullptr);
     bytes_written_ = 0;
+    sync_counter_ = 0;
   }
 
   Status NewWritableFile(const std::string& f, unique_ptr<WritableFile>* r,
@@ -190,6 +193,7 @@ class SpecialEnv : public EnvWrapper {
       Status Close() { return base_->Close(); }
       Status Flush() { return base_->Flush(); }
       Status Sync() {
+        ++env_->sync_counter_;
         while (env_->delay_sstable_sync_.Acquire_Load() != nullptr) {
           env_->SleepForMicroseconds(100000);
         }
@@ -216,6 +220,7 @@ class SpecialEnv : public EnvWrapper {
       Status Close() { return base_->Close(); }
       Status Flush() { return base_->Flush(); }
       Status Sync() {
+        ++env_->sync_counter_;
         if (env_->manifest_sync_error_.Acquire_Load() != nullptr) {
           return Status::IOError("simulated sync error");
         } else {
@@ -239,7 +244,10 @@ class SpecialEnv : public EnvWrapper {
       }
       Status Close() { return base_->Close(); }
       Status Flush() { return base_->Flush(); }
-      Status Sync() { return base_->Sync(); }
+      Status Sync() {
+        ++env_->sync_counter_;
+        return base_->Sync();
+      }
     };
 
     if (non_writable_.Acquire_Load() != nullptr) {
@@ -8379,6 +8387,28 @@ TEST(DBTest, WriteSingleThreadEntry) {
   }
 }
 
+TEST(DBTest, DisableDataSyncTest) {
+  // iter 0 -- no sync
+  // iter 1 -- sync
+  for (int iter = 0; iter < 2; ++iter) {
+    Options options = CurrentOptions();
+    options.disableDataSync = iter == 0;
+    options.create_if_missing = true;
+    options.env = env_;
+    Reopen(&options);
+    CreateAndReopenWithCF({"pikachu"}, &options);
+
+    MakeTables(10, "a", "z");
+    Compact("a", "z");
+
+    if (iter == 0) {
+      ASSERT_EQ(env_->sync_counter_.load(), 0);
+    } else {
+      ASSERT_GT(env_->sync_counter_.load(), 0);
+    }
+    Destroy(&options);
+  }
+}
 
 
 }  // namespace rocksdb

db/version_set.cc

@@ -1891,7 +1891,7 @@ Status VersionSet::LogAndApply(ColumnFamilyData* column_family_data,
           break;
         }
       }
-      if (s.ok()) {
+      if (s.ok() && db_options_->disableDataSync == false) {
         if (db_options_->use_fsync) {
           StopWatch sw(env_, db_options_->statistics.get(),
                        MANIFEST_FILE_SYNC_MICROS);
@@ -1928,7 +1928,7 @@ Status VersionSet::LogAndApply(ColumnFamilyData* column_family_data,
     // new CURRENT file that points to it.
     if (s.ok() && new_descriptor_log) {
       s = SetCurrentFile(env_, dbname_, pending_manifest_file_number_,
-                         db_directory);
+                         db_options_->disableDataSync ? nullptr : db_directory);
       if (s.ok() && pending_manifest_file_number_ > manifest_file_number_) {
         // delete old manifest file
         Log(db_options_->info_log,

include/rocksdb/options.h

@@ -610,7 +610,7 @@ struct DBOptions {
   // it does not use any locks to prevent concurrent updates.
   std::shared_ptr<Statistics> statistics;
 
-  // If true, then the contents of data files are not synced
+  // If true, then the contents of manifest and data files are not synced
   // to stable storage. Their contents remain in the OS buffers till the
   // OS decides to flush them. This option is good for bulk-loading
   // of data. Once the bulk-loading is complete, please issue a