kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ale...@apache.org
Subject [1/2] kudu git commit: fs: adjust directory sync behavior
Date Thu, 19 Oct 2017 21:48:07 GMT
Repository: kudu
Updated Branches:
  refs/heads/master 741bdfa41 -> a6f2322b2


fs: adjust directory sync behavior

This patch centralizes the synchronization of fs roots and data directories
in a new env_util helper method, which uses the set of all created files and
directories as input to synchronization.

There are a few related fixes:
- DataDirManager::Create wasn't considering enable_data_block_fsync when
  synchronizing. It does now.
- It was also synchronizing the directories themselves instead of their
  parent directories; the latter is what's needed for durability.

Change-Id: I27e7bf6a5a31720cf80a002c3553536ee0975758
Reviewed-on: http://gerrit.cloudera.org:8080/8290
Reviewed-by: Andrew Wong <awong@cloudera.com>
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <todd@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/4d84eeea
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/4d84eeea
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/4d84eeea

Branch: refs/heads/master
Commit: 4d84eeea1695734f2723af0e200f134b9fe72005
Parents: 741bdfa
Author: Adar Dembo <adar@cloudera.com>
Authored: Fri Oct 13 17:38:57 2017 -0700
Committer: Todd Lipcon <todd@apache.org>
Committed: Thu Oct 19 18:49:33 2017 +0000

----------------------------------------------------------------------
 src/kudu/fs/data_dirs.cc  | 11 ++++-------
 src/kudu/fs/fs_manager.cc | 13 ++++---------
 src/kudu/util/env_util.cc | 22 +++++++++++++++++++++-
 src/kudu/util/env_util.h  |  7 +++++++
 4 files changed, 36 insertions(+), 17 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/4d84eeea/src/kudu/fs/data_dirs.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/data_dirs.cc b/src/kudu/fs/data_dirs.cc
index 9ece90d..635a4a8 100644
--- a/src/kudu/fs/data_dirs.cc
+++ b/src/kudu/fs/data_dirs.cc
@@ -28,7 +28,6 @@
 #include <random>
 #include <string>
 #include <unordered_map>
-#include <unordered_set>
 #include <utility>
 #include <vector>
 
@@ -104,6 +103,7 @@ METRIC_DEFINE_gauge_uint64(server, data_dirs_full,
                            kudu::MetricUnit::kDataDirectories,
                            "Number of data directories whose disks are currently full");
 
+DECLARE_bool(enable_data_block_fsync);
 DECLARE_string(block_manager);
 
 namespace kudu {
@@ -117,7 +117,6 @@ using std::set;
 using std::shuffle;
 using std::string;
 using std::unique_ptr;
-using std::unordered_set;
 using std::vector;
 using strings::Substitute;
 
@@ -393,7 +392,6 @@ Status DataDirManager::Create() {
   int idx = 0;
 
   // Ensure the data dirs exist and create the instance files.
-  unordered_set<string> to_sync;
   for (const auto& root : canonicalized_data_fs_roots_) {
     RETURN_NOT_OK_PREPEND(root.status, "Could not create directory manager with disks failed");
     string data_dir = JoinPathSegments(root.path, kDataDirName);
@@ -402,7 +400,6 @@ Status DataDirManager::Create() {
         Substitute("Could not create directory $0", data_dir));
     if (created) {
       dirs_to_delete.emplace_back(data_dir);
-      to_sync.insert(root.path);
     }
 
     if (block_manager_type_ == "log") {
@@ -419,9 +416,9 @@ Status DataDirManager::Create() {
   }
 
   // Ensure newly created directories are synchronized to disk.
-  for (const string& dir : to_sync) {
-    RETURN_NOT_OK_PREPEND(env_->SyncDir(dir),
-                          Substitute("Unable to synchronize directory $0", dir));
+  if (FLAGS_enable_data_block_fsync) {
+    RETURN_NOT_OK_PREPEND(env_util::SyncAllParentDirs(
+        env_, dirs_to_delete, files_to_delete), "could not sync data directories");
   }
 
   // Success: don't delete any files.

http://git-wip-us.apache.org/repos/asf/kudu/blob/4d84eeea/src/kudu/fs/fs_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/fs_manager.cc b/src/kudu/fs/fs_manager.cc
index bf88c63..5f62858 100644
--- a/src/kudu/fs/fs_manager.cc
+++ b/src/kudu/fs/fs_manager.cc
@@ -22,7 +22,6 @@
 #include <iostream>
 #include <map>
 #include <set>
-#include <unordered_set>
 #include <utility>
 
 #include <boost/optional/optional.hpp>
@@ -107,7 +106,6 @@ using std::ostream;
 using std::set;
 using std::string;
 using std::unique_ptr;
-using std::unordered_set;
 using std::vector;
 using strings::Substitute;
 
@@ -400,7 +398,6 @@ Status FsManager::CreateInitialFileSystemLayout(boost::optional<string>
uuid) {
 
   InstanceMetadataPB metadata;
   RETURN_NOT_OK(CreateInstanceMetadata(std::move(uuid), &metadata));
-  unordered_set<string> to_sync;
   for (const auto& root : canonicalized_all_fs_roots_) {
     if (!root.status.ok()) {
       continue;
@@ -411,7 +408,6 @@ Status FsManager::CreateInitialFileSystemLayout(boost::optional<string>
uuid) {
                           "Unable to create FSManager root");
     if (created) {
       dirs_to_delete.emplace_back(root_name);
-      to_sync.insert(DirName(root_name));
     }
     RETURN_NOT_OK_PREPEND(WriteInstanceMetadata(metadata, root_name),
                           "Unable to write instance metadata");
@@ -428,19 +424,18 @@ Status FsManager::CreateInitialFileSystemLayout(boost::optional<string>
uuid) {
                           Substitute("Unable to create directory $0", dir));
     if (created) {
       dirs_to_delete.emplace_back(dir);
-      to_sync.insert(DirName(dir));
     }
   }
 
   // Ensure newly created directories are synchronized to disk.
   if (FLAGS_enable_data_block_fsync) {
-    for (const string& dir : to_sync) {
-      RETURN_NOT_OK_PREPEND(env_->SyncDir(dir),
-                            Substitute("Unable to synchronize directory $0", dir));
-    }
+    RETURN_NOT_OK_PREPEND(env_util::SyncAllParentDirs(
+        env_, dirs_to_delete, files_to_delete), "could not sync fs roots");
   }
 
   // And lastly, create the directory manager.
+  //
+  // All files/directories created will be synchronized to disk.
   DataDirManagerOptions opts;
   opts.metric_entity = metric_entity_;
   opts.read_only = read_only_;

http://git-wip-us.apache.org/repos/asf/kudu/blob/4d84eeea/src/kudu/util/env_util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/env_util.cc b/src/kudu/util/env_util.cc
index 1a7a5bc..4dbe6d0 100644
--- a/src/kudu/util/env_util.cc
+++ b/src/kudu/util/env_util.cc
@@ -23,8 +23,9 @@
 #include <ctime>
 #include <memory>
 #include <string>
-#include <vector>
+#include <unordered_set>
 #include <utility>
+#include <vector>
 
 #include <gflags/gflags.h>
 #include <glog/logging.h>
@@ -73,6 +74,7 @@ using std::pair;
 using std::shared_ptr;
 using std::string;
 using std::unique_ptr;
+using std::unordered_set;
 using std::vector;
 using strings::Substitute;
 
@@ -281,5 +283,23 @@ Status IsDirectoryEmpty(Env* env, const string& path, bool* is_empty)
{
   return Status::OK();
 }
 
+Status SyncAllParentDirs(Env* env,
+                         const vector<string>& dirs,
+                         const vector<string>& files) {
+  // An unordered_set is used to deduplicate the set of directories.
+  unordered_set<string> to_sync;
+  for (const auto& d : dirs) {
+    to_sync.insert(DirName(d));
+  }
+  for (const auto& f : files) {
+    to_sync.insert(DirName(f));
+  }
+  for (const auto& d : to_sync) {
+    RETURN_NOT_OK_PREPEND(env->SyncDir(d),
+                          Substitute("unable to synchronize directory $0", d));
+  }
+  return Status::OK();
+}
+
 } // namespace env_util
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/4d84eeea/src/kudu/util/env_util.h
----------------------------------------------------------------------
diff --git a/src/kudu/util/env_util.h b/src/kudu/util/env_util.h
index faba197..bc630e0 100644
--- a/src/kudu/util/env_util.h
+++ b/src/kudu/util/env_util.h
@@ -21,6 +21,7 @@
 #include <cstdint>
 #include <memory>
 #include <string>
+#include <vector>
 
 #include "kudu/util/status.h"
 
@@ -94,6 +95,12 @@ Status DeleteTmpFilesRecursively(Env* env, const std::string& path);
 // accordingly.
 Status IsDirectoryEmpty(Env* env, const std::string& path, bool* is_empty);
 
+// Synchronize all of the parent directories belonging to 'dirs' and 'files'
+// to disk.
+Status SyncAllParentDirs(Env* env,
+                         const std::vector<std::string>& dirs,
+                         const std::vector<std::string>& files);
+
 } // namespace env_util
 } // namespace kudu
 


Mime
View raw message