From 3bf9f9a8323239c30d835a89161ec2d015bc12e2 Mon Sep 17 00:00:00 2001
From: Mike Kolupaev <kolmike@fb.com>
Date: Wed, 22 Jul 2015 12:27:39 -0700
Subject: [PATCH] cleaned up PosixMmapFile a little

Summary: https://reviews.facebook.net/D42321 has left PosixMmapFile in some weird state. This diff removes pending_sync_ that was now unused, fixes indentation and prevents Fsync() from calling both fsync() and fdatasync().

Test Plan: `make -j check`

Reviewers: sdong

Reviewed By: sdong

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D42885
---
 util/env_posix.cc | 54 +++++++++++++++++++++--------------------------
 1 file changed, 24 insertions(+), 30 deletions(-)

diff --git a/util/env_posix.cc b/util/env_posix.cc
index 80ef54566..47ff7284c 100644
--- a/util/env_posix.cc
+++ b/util/env_posix.cc
@@ -350,8 +350,6 @@ class PosixMmapFile : public WritableFile {
   char* dst_;             // Where to write next  (in range [base_,limit_])
   char* last_sync_;       // Where have we synced up to
   uint64_t file_offset_;  // Offset of base_ in file
-  // Have we done an munmap of unsynced data?
-  bool pending_sync_;
 #ifdef ROCKSDB_FALLOCATE_PRESENT
   bool fallocate_with_keep_size_;
 #endif
@@ -370,10 +368,6 @@ class PosixMmapFile : public WritableFile {
   Status UnmapCurrentRegion() {
     TEST_KILL_RANDOM(rocksdb_kill_odds);
     if (base_ != nullptr) {
-      if (last_sync_ < limit_) {
-        // Defer syncing this data until next Sync() call, if any
-        pending_sync_ = true;
-      }
       int munmap_status = munmap(base_, limit_ - base_);
       if (munmap_status != 0) {
         return IOError(filename_, munmap_status);
@@ -429,6 +423,22 @@ class PosixMmapFile : public WritableFile {
 #endif
   }
 
+  Status Msync() {
+    if (dst_ == last_sync_) {
+      return Status::OK();
+    }
+    // Find the beginnings of the pages that contain the first and last
+    // bytes to be synced.
+    size_t p1 = TruncateToPageBoundary(last_sync_ - base_);
+    size_t p2 = TruncateToPageBoundary(dst_ - base_ - 1);
+    last_sync_ = dst_;
+    TEST_KILL_RANDOM(rocksdb_kill_odds);
+    if (msync(base_ + p1, p2 - p1 + page_size_, MS_SYNC) < 0) {
+      return IOError(filename_, errno);
+    }
+    return Status::OK();
+  }
+
  public:
   PosixMmapFile(const std::string& fname, int fd, size_t page_size,
                 const EnvOptions& options)
@@ -514,38 +524,22 @@ class PosixMmapFile : public WritableFile {
   }
 
   virtual Status Sync() override {
-    Status s;
-
-      if (fdatasync(fd_) < 0) {
-        s = IOError(filename_, errno);
-      }
-
-    if (dst_ > last_sync_) {
-      // Find the beginnings of the pages that contain the first and last
-      // bytes to be synced.
-      size_t p1 = TruncateToPageBoundary(last_sync_ - base_);
-      size_t p2 = TruncateToPageBoundary(dst_ - base_ - 1);
-      last_sync_ = dst_;
-      TEST_KILL_RANDOM(rocksdb_kill_odds);
-      if (msync(base_ + p1, p2 - p1 + page_size_, MS_SYNC) < 0) {
-        s = IOError(filename_, errno);
-      }
+    if (fdatasync(fd_) < 0) {
+      return IOError(filename_, errno);
     }
 
-    return s;
+    return Msync();
   }
 
   /**
    * Flush data as well as metadata to stable storage.
    */
   virtual Status Fsync() override {
-      // Some unmapped data was not synced
-      if (fsync(fd_) < 0) {
-        return IOError(filename_, errno);
-      }
-    // This invocation to Sync will not issue the call to
-    // fdatasync because pending_sync_ has already been cleared.
-    return Sync();
+    if (fsync(fd_) < 0) {
+      return IOError(filename_, errno);
+    }
+
+    return Msync();
   }
 
   /**
-- 
GitLab