Flush before Fsync()/Sync()

Summary: Calling Fsync()/Sync() on a file should give the guarantee that whatever you written to the file is now persisted. This is currently not the case, since we might have some data left in application cache as we do Fsync()/Sync(). For example, BuildTable() calls Fsync() without the flush, assuming all sst data is now persisted, but it's actually not. This may result in big inconsistencies.

Test Plan: no test

Reviewers: sdong, dhruba, haobo, ljin, yhchiang

Reviewed By: sdong

CC: leveldb
Differential Revision: https://reviews.facebook.net/D18159

diff --git a/util/env_posix.cc b/util/env_posix.cc
index bce9526a63b5c2522ff48503324e54d70876582d..9e76a126dcd4cbb8db685f2d705b3f02726dc655 100644 (file)
--- a/util/env_posix.cc
+++ b/util/env_posix.cc
@@ -761,6 +761,10 @@ class PosixWritableFile : public WritableFile {
   }
 
   virtual Status Sync() {
+    Status s = Flush();
+    if (!s.ok()) {
+      return s;
+    }
     TEST_KILL_RANDOM(rocksdb_kill_odds);
     if (pending_sync_ && fdatasync(fd_) < 0) {
       return IOError(filename_, errno);
@@ -771,6 +775,10 @@ class PosixWritableFile : public WritableFile {
   }
 
   virtual Status Fsync() {
+    Status s = Flush();
+    if (!s.ok()) {
+      return s;
+    }
     TEST_KILL_RANDOM(rocksdb_kill_odds);
     if (pending_fsync_ && fsync(fd_) < 0) {
       return IOError(filename_, errno);