From 4e48753b735692071beaae3cfea300c78a80ed73 Mon Sep 17 00:00:00 2001
From: sdong <siying.d@fb.com>
Date: Thu, 22 Jan 2015 11:43:38 -0800
Subject: [PATCH] Sync manifest file when initializing it

Summary: Now we don't sync manifest file when initializing it, so DB cannot be safely reopened before the first mem table flush. Fix it by syncing it. This fixes fault_injection_test.

Test Plan: make all check

Reviewers: rven, yhchiang, igor

Reviewed By: igor

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D32001
---
 db/db_impl.cc              |  3 +++
 db/db_test.cc              |  1 +
 db/fault_injection_test.cc |  3 ---
 db/filename.cc             | 13 +++++++++++++
 db/filename.h              |  5 +++++
 db/version_set.cc          | 12 ++----------
 6 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/db/db_impl.cc b/db/db_impl.cc
index b50ea9561..34c3077fd 100644
--- a/db/db_impl.cc
+++ b/db/db_impl.cc
@@ -348,6 +348,9 @@ Status DBImpl::NewDB() {
     std::string record;
     new_db.EncodeTo(&record);
     s = log.AddRecord(record);
+    if (s.ok()) {
+      s = SyncManifest(env_, &db_options_, log.file());
+    }
   }
   if (s.ok()) {
     // Make "CURRENT" file that points to the new manifest file.
diff --git a/db/db_test.cc b/db/db_test.cc
index 4d3ebb51f..1d6962b58 100644
--- a/db/db_test.cc
+++ b/db/db_test.cc
@@ -9274,6 +9274,7 @@ TEST(DBTest, WriteSingleThreadEntry) {
 }
 
 TEST(DBTest, DisableDataSyncTest) {
+  env_->sync_counter_.store(0);
   // iter 0 -- no sync
   // iter 1 -- sync
   for (int iter = 0; iter < 2; ++iter) {
diff --git a/db/fault_injection_test.cc b/db/fault_injection_test.cc
index 0ef0be318..6861f1525 100644
--- a/db/fault_injection_test.cc
+++ b/db/fault_injection_test.cc
@@ -511,8 +511,6 @@ TEST(FaultInjectionTest, FaultTest) {
     int num_pre_sync = rnd.Uniform(kMaxNumValues);
     int num_post_sync = rnd.Uniform(kMaxNumValues);
 
-    // TODO(t6007549) Figure out why this fails and then re-enable the test.
-#if 0
     PartialCompactTestPreFault(num_pre_sync, num_post_sync);
     PartialCompactTestReopenWithFault(RESET_DROP_UNSYNCED_DATA,
                                       num_pre_sync,
@@ -520,7 +518,6 @@ TEST(FaultInjectionTest, FaultTest) {
 
     NoWriteTestPreFault();
     NoWriteTestReopenWithFault(RESET_DROP_UNSYNCED_DATA);
-#endif
 
     PartialCompactTestPreFault(num_pre_sync, num_post_sync);
     // No new files created so we expect all values since no files will be
diff --git a/db/filename.cc b/db/filename.cc
index e5d97bdf2..160005dda 100644
--- a/db/filename.cc
+++ b/db/filename.cc
@@ -19,6 +19,7 @@
 #include "db/dbformat.h"
 #include "rocksdb/env.h"
 #include "util/logging.h"
+#include "util/stop_watch.h"
 
 namespace rocksdb {
 
@@ -329,4 +330,16 @@ Status SetIdentityFile(Env* env, const std::string& dbname) {
   return s;
 }
 
+Status SyncManifest(Env* env, const DBOptions* db_options, WritableFile* file) {
+  if (db_options->disableDataSync) {
+    return Status::OK();
+  } else if (db_options->use_fsync) {
+    StopWatch sw(env, db_options->statistics.get(), MANIFEST_FILE_SYNC_MICROS);
+    return file->Fsync();
+  } else {
+    StopWatch sw(env, db_options->statistics.get(), MANIFEST_FILE_SYNC_MICROS);
+    return file->Sync();
+  }
+}
+
 }  // namespace rocksdb
diff --git a/db/filename.h b/db/filename.h
index fda873676..33f5ace20 100644
--- a/db/filename.h
+++ b/db/filename.h
@@ -25,6 +25,7 @@ namespace rocksdb {
 
 class Env;
 class Directory;
+class WritableFile;
 
 enum FileType {
   kLogFile,
@@ -137,4 +138,8 @@ extern Status SetCurrentFile(Env* env, const std::string& dbname,
 // Make the IDENTITY file for the db
 extern Status SetIdentityFile(Env* env, const std::string& dbname);
 
+// Sync manifest file `file`.
+extern Status SyncManifest(Env* env, const DBOptions* db_options,
+                           WritableFile* file);
+
 }  // namespace rocksdb
diff --git a/db/version_set.cc b/db/version_set.cc
index c5956a534..a0af2decc 100644
--- a/db/version_set.cc
+++ b/db/version_set.cc
@@ -1691,16 +1691,8 @@ Status VersionSet::LogAndApply(ColumnFamilyData* column_family_data,
           break;
         }
       }
-      if (s.ok() && db_options_->disableDataSync == false) {
-        if (db_options_->use_fsync) {
-          StopWatch sw(env_, db_options_->statistics.get(),
-                       MANIFEST_FILE_SYNC_MICROS);
-          s = descriptor_log_->file()->Fsync();
-        } else {
-          StopWatch sw(env_, db_options_->statistics.get(),
-                       MANIFEST_FILE_SYNC_MICROS);
-          s = descriptor_log_->file()->Sync();
-        }
+      if (s.ok()) {
+        s = SyncManifest(env_, db_options_, descriptor_log_->file());
       }
       if (!s.ok()) {
         Log(InfoLogLevel::ERROR_LEVEL, db_options_->info_log,