You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by to...@apache.org on 2017/11/15 20:58:15 UTC

[2/3] kudu git commit: tool: new action for adding to the set of data directories

tool: new action for adding to the set of data directories

This patch includes support for adding to the set of data directories in
an existing Kudu filesystem. The only user-facing bit is a new FsManager
option that, when set, augments Open() to also look for missing fs
roots. If any are found, they will be created, and the existing data
directory instance files updated to recognize these new roots. Also
included is a new tool action that opens an FsManager with this option
set.

Updating the set of data directories is a complex, multi-step operation,
and a single error could leave the filesystem in a difficult-to-repair
state. As such, there is some fairly gnarly rollback code that attempts
to undo the changes made in the event of an error.

This logic can be extended to remove data directories. This patch only
addresses adding.

Change-Id: I6ddbdd6cc6231996e7802a622a8b4691527a0643
Reviewed-on: http://gerrit.cloudera.org:8080/8352
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>


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

Branch: refs/heads/master
Commit: 044d5fe684b07a66749188168701ff7b5b2b0789
Parents: a47b040
Author: Adar Dembo <ad...@cloudera.com>
Authored: Thu Oct 19 17:53:39 2017 -0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Tue Nov 14 07:40:35 2017 +0000

----------------------------------------------------------------------
 src/kudu/fs/block_manager_util-test.cc |   6 +-
 src/kudu/fs/block_manager_util.cc      |  15 +-
 src/kudu/fs/block_manager_util.h       |   6 +-
 src/kudu/fs/data_dirs-test.cc          |   7 +-
 src/kudu/fs/data_dirs.cc               | 273 +++++++++++++++++++++-------
 src/kudu/fs/data_dirs.h                |  39 +++-
 src/kudu/fs/fs_manager-test.cc         | 222 +++++++++++++++++++---
 src/kudu/fs/fs_manager.cc              | 201 +++++++++++++-------
 src/kudu/fs/fs_manager.h               |  21 +++
 src/kudu/tools/kudu-tool-test.cc       |  92 +++++++++-
 src/kudu/tools/tool_action_fs.cc       |  17 ++
 11 files changed, 730 insertions(+), 169 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/044d5fe6/src/kudu/fs/block_manager_util-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/block_manager_util-test.cc b/src/kudu/fs/block_manager_util-test.cc
index 800aac1..cad2eb7 100644
--- a/src/kudu/fs/block_manager_util-test.cc
+++ b/src/kudu/fs/block_manager_util-test.cc
@@ -27,7 +27,6 @@
 
 #include "kudu/fs/block_manager_util.h"
 #include "kudu/fs/fs.pb.h"
-#include "kudu/gutil/stl_util.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/util/env.h"
 #include "kudu/util/status.h"
@@ -109,8 +108,7 @@ TEST_F(KuduTest, Locking) {
 static void RunCheckIntegrityTest(Env* env,
                                   const vector<PathSetPB>& path_sets,
                                   const string& expected_status_string) {
-  vector<PathInstanceMetadataFile*> instances;
-  ElementDeleter deleter(&instances);
+  vector<unique_ptr<PathInstanceMetadataFile>> instances;
 
   int i = 0;
   for (const PathSetPB& ps : path_sets) {
@@ -121,7 +119,7 @@ static void RunCheckIntegrityTest(Env* env,
     metadata->set_filesystem_block_size_bytes(1);
     metadata->mutable_path_set()->CopyFrom(ps);
     instance->SetMetadataForTests(std::move(metadata));
-    instances.push_back(instance.release());
+    instances.emplace_back(std::move(instance));
     i++;
   }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/044d5fe6/src/kudu/fs/block_manager_util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/block_manager_util.cc b/src/kudu/fs/block_manager_util.cc
index baa064f..1920d2a 100644
--- a/src/kudu/fs/block_manager_util.cc
+++ b/src/kudu/fs/block_manager_util.cc
@@ -159,7 +159,7 @@ void PathInstanceMetadataFile::SetMetadataForTests(
 }
 
 Status PathInstanceMetadataFile::CheckIntegrity(
-    const vector<PathInstanceMetadataFile*>& instances) {
+    const vector<unique_ptr<PathInstanceMetadataFile>>& instances) {
   CHECK(!instances.empty());
 
   // Note: although this verification works at the level of UUIDs and instance
@@ -183,7 +183,7 @@ Status PathInstanceMetadataFile::CheckIntegrity(
                    instances.size(), all_uuids.size()));
   }
 
-  for (PathInstanceMetadataFile* instance : instances) {
+  for (const auto& instance : instances) {
     // If the instance has failed (e.g. due to disk failure), there's no
     // telling what its metadata looks like. Ignore it, and continue checking
     // integrity across the healthy instances.
@@ -193,11 +193,12 @@ Status PathInstanceMetadataFile::CheckIntegrity(
     const PathSetPB& path_set = instance->metadata()->path_set();
 
     // Check that the instance's UUID has not been claimed by another instance.
-    PathInstanceMetadataFile** other = InsertOrReturnExisting(&uuids, path_set.uuid(), instance);
+    PathInstanceMetadataFile** other = InsertOrReturnExisting(
+        &uuids, path_set.uuid(), instance.get());
     if (other) {
       return Status::IOError(
           Substitute("Data directories $0 and $1 have duplicate instance metadata UUIDs",
-                     (*other)->path(), instance->path()),
+                     (*other)->dir(), instance->dir()),
           path_set.uuid());
     }
 
@@ -205,7 +206,7 @@ Status PathInstanceMetadataFile::CheckIntegrity(
     if (!ContainsKey(all_uuids, path_set.uuid())) {
       return Status::IOError(
           Substitute("Data directory $0 instance metadata contains unexpected UUID",
-                     instance->path()),
+                     instance->dir()),
           path_set.uuid());
     }
 
@@ -216,7 +217,7 @@ Status PathInstanceMetadataFile::CheckIntegrity(
     if (deduplicated_uuids.size() != path_set.all_uuids_size()) {
       return Status::IOError(
           Substitute("Data directory $0 instance metadata path set contains duplicate UUIDs",
-                     instance->path()),
+                     instance->dir()),
           JoinStrings(path_set.all_uuids(), ","));
     }
 
@@ -224,7 +225,7 @@ Status PathInstanceMetadataFile::CheckIntegrity(
     if (deduplicated_uuids != all_uuids) {
       return Status::IOError(
           Substitute("Data directories $0 and $1 have different instance metadata UUID sets",
-                     instances[0]->path(), instance->path()),
+                     instances[0]->dir(), instance->dir()),
           Substitute("$0 vs $1", JoinStrings(all_uuids, ","), all_uuids_str));
     }
   }

http://git-wip-us.apache.org/repos/asf/kudu/blob/044d5fe6/src/kudu/fs/block_manager_util.h
----------------------------------------------------------------------
diff --git a/src/kudu/fs/block_manager_util.h b/src/kudu/fs/block_manager_util.h
index c8fd2cf..a9b75aa 100644
--- a/src/kudu/fs/block_manager_util.h
+++ b/src/kudu/fs/block_manager_util.h
@@ -89,12 +89,14 @@ class PathInstanceMetadataFile {
     return health_status_;
   }
 
-  std::string path() const { return DirName(filename_); }
+  std::string dir() const { return DirName(filename_); }
+  const std::string& path() const { return filename_; }
   PathInstanceMetadataPB* const metadata() const { return metadata_.get(); }
 
   // Check the integrity of the provided instances' path sets, ignoring any
   // unhealthy instances.
-  static Status CheckIntegrity(const std::vector<PathInstanceMetadataFile*>& instances);
+  static Status CheckIntegrity(
+      const std::vector<std::unique_ptr<PathInstanceMetadataFile>>& instances);
 
  private:
   Env* env_;

http://git-wip-us.apache.org/repos/asf/kudu/blob/044d5fe6/src/kudu/fs/data_dirs-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/data_dirs-test.cc b/src/kudu/fs/data_dirs-test.cc
index 79a13fc..e658157 100644
--- a/src/kudu/fs/data_dirs-test.cc
+++ b/src/kudu/fs/data_dirs-test.cc
@@ -38,6 +38,7 @@
 #include "kudu/gutil/strings/join.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/util/env.h"
+#include "kudu/util/env_util.h"
 #include "kudu/util/metrics.h"
 #include "kudu/util/path_util.h"
 #include "kudu/util/status.h"
@@ -57,7 +58,6 @@ DECLARE_int32(fs_target_data_dirs_per_tablet);
 DECLARE_int64(disk_reserved_bytes_free_for_testing);
 DECLARE_int64(fs_data_dirs_reserved_bytes);
 DECLARE_string(env_inject_eio_globs);
-DECLARE_string(block_manager);
 
 METRIC_DECLARE_gauge_uint64(data_dirs_failed);
 
@@ -91,7 +91,8 @@ class DataDirsTest : public KuduTest {
     for (int i = 0; i < num_dirs; i++) {
       string dir_name = Substitute("$0-$1", kDirNamePrefix, i);
       ret.push_back(GetTestPath(dir_name));
-      CHECK_OK(env_->CreateDir(ret[i]));
+      bool created;
+      CHECK_OK(env_util::CreateDirIfMissing(env_, ret[i], &created));
     }
     return ret;
   }
@@ -422,7 +423,7 @@ TEST_F(DataDirManagerTest, TestOpenWithFailedDirs) {
   FLAGS_env_inject_eio_globs = JoinStrings(JoinPathSegmentsV(test_roots_, "**"), ",");
   Status s = DataDirManager::OpenExistingForTests(env_, test_roots_,
       DataDirManagerOptions(), &dd_manager_);
-  ASSERT_STR_CONTAINS(s.ToString(), "Could not open directory manager");
+  ASSERT_STR_CONTAINS(s.ToString(), "could not open directory manager");
   ASSERT_TRUE(s.IsIOError());
   FLAGS_env_inject_eio = 0;
 }

http://git-wip-us.apache.org/repos/asf/kudu/blob/044d5fe6/src/kudu/fs/data_dirs.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/data_dirs.cc b/src/kudu/fs/data_dirs.cc
index f3892c8..7860caa 100644
--- a/src/kudu/fs/data_dirs.cc
+++ b/src/kudu/fs/data_dirs.cc
@@ -28,6 +28,7 @@
 #include <random>
 #include <string>
 #include <unordered_map>
+#include <unordered_set>
 #include <utility>
 #include <vector>
 
@@ -52,6 +53,7 @@
 #include "kudu/util/monotime.h"
 #include "kudu/util/oid_generator.h"
 #include "kudu/util/path_util.h"
+#include "kudu/util/pb_util.h"
 #include "kudu/util/random_util.h"
 #include "kudu/util/scoped_cleanup.h"
 #include "kudu/util/status.h"
@@ -113,10 +115,13 @@ namespace fs {
 using internal::DataDirGroup;
 using std::default_random_engine;
 using std::iota;
+using std::pair;
 using std::set;
 using std::shuffle;
 using std::string;
 using std::unique_ptr;
+using std::unordered_map;
+using std::unordered_set;
 using std::vector;
 using strings::Substitute;
 
@@ -275,11 +280,10 @@ Status DataDir::RefreshIsFull(RefreshMode mode) {
   return Status::OK();
 }
 
-const char* DataDirManager::kDataDirName = "data";
-
 DataDirManagerOptions::DataDirManagerOptions()
   : block_manager_type(FLAGS_block_manager),
-    read_only(false) {}
+    read_only(false),
+    update_on_disk(false) {}
 
 vector<string> DataDirManager::GetRootNames(const CanonicalizedRootsList& root_list) {
   vector<string> roots;
@@ -296,6 +300,7 @@ DataDirManager::DataDirManager(Env* env,
       canonicalized_data_fs_roots_(std::move(canonicalized_data_roots)),
       rng_(GetRandomSeed32()) {
   DCHECK_GT(canonicalized_data_fs_roots_.size(), 0);
+  DCHECK(!opts_.update_on_disk || !opts_.read_only);
 
   if (opts_.metric_entity) {
     metrics_.reset(new DataDirMetrics(opts_.metric_entity));
@@ -327,8 +332,8 @@ Status DataDirManager::OpenExistingForTests(Env* env, vector<string> data_fs_roo
                                             DataDirManagerOptions opts,
                                             unique_ptr<DataDirManager>* dd_manager) {
   CanonicalizedRootsList roots;
-  for (string& root : data_fs_roots) {
-    roots.push_back({ root, Status::OK() });
+  for (const auto& r : data_fs_roots) {
+    roots.push_back({ r, Status::OK() });
   }
   return DataDirManager::OpenExisting(env, std::move(roots), std::move(opts), dd_manager);
 }
@@ -347,8 +352,8 @@ Status DataDirManager::CreateNewForTests(Env* env, vector<string> data_fs_roots,
                                          DataDirManagerOptions opts,
                                          unique_ptr<DataDirManager>* dd_manager) {
   CanonicalizedRootsList roots;
-  for (string& root : data_fs_roots) {
-    roots.push_back({ root, Status::OK() });
+  for (const auto& r : data_fs_roots) {
+    roots.push_back({ r, Status::OK() });
   }
   return DataDirManager::CreateNew(env, std::move(roots), std::move(opts), dd_manager);
 }
@@ -367,37 +372,50 @@ Status DataDirManager::CreateNew(Env* env, CanonicalizedRootsList data_fs_roots,
 Status DataDirManager::Create() {
   CHECK(!opts_.read_only);
 
-  vector<string> dirs_to_delete;
-  vector<string> files_to_delete;
+  // Generate a new UUID for each data directory.
+  ObjectIdGenerator gen;
+  vector<string> all_uuids;
+  vector<pair<string, string>> root_uuid_pairs_to_create;
+  for (const auto& r : canonicalized_data_fs_roots_) {
+    RETURN_NOT_OK_PREPEND(r.status, "Could not create directory manager with disks failed");
+    string uuid = gen.Next();
+    all_uuids.emplace_back(uuid);
+    root_uuid_pairs_to_create.emplace_back(r.path, std::move(uuid));
+  }
+  RETURN_NOT_OK_PREPEND(CreateNewDataDirectoriesAndUpdateExistingOnes(
+      std::move(root_uuid_pairs_to_create), {}, std::move(all_uuids)),
+                        "could not create new data directories");
+  return Status::OK();
+}
+
+Status DataDirManager::CreateNewDataDirectoriesAndUpdateExistingOnes(
+    vector<pair<string, string>> root_uuid_pairs_to_create,
+    vector<unique_ptr<PathInstanceMetadataFile>> instances_to_update,
+    vector<string> all_uuids) {
+  CHECK(!opts_.read_only);
+
+  vector<string> created_dirs;
+  vector<string> created_files;
   auto deleter = MakeScopedCleanup([&]() {
     // Delete files first so that the directories will be empty when deleted.
-    for (const auto& f : files_to_delete) {
+    for (const auto& f : created_files) {
       WARN_NOT_OK(env_->DeleteFile(f), "Could not delete file " + f);
     }
     // Delete directories in reverse order since parent directories will have
     // been added before child directories.
-    for (auto it = dirs_to_delete.rbegin(); it != dirs_to_delete.rend(); it++) {
+    for (auto it = created_dirs.rbegin(); it != created_dirs.rend(); it++) {
       WARN_NOT_OK(env_->DeleteDir(*it), "Could not delete dir " + *it);
     }
   });
 
-  // The UUIDs and indices will be included in every instance file.
-  ObjectIdGenerator gen;
-  vector<string> all_uuids(canonicalized_data_fs_roots_.size());
-  for (string& u : all_uuids) {
-    u = gen.Next();
-  }
-  int idx = 0;
-
   // Ensure the data dirs exist and create the instance files.
-  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);
+  for (const auto& p : root_uuid_pairs_to_create) {
+    string data_dir = JoinPathSegments(p.first, kDataDirName);
     bool created;
     RETURN_NOT_OK_PREPEND(env_util::CreateDirIfMissing(env_, data_dir, &created),
         Substitute("Could not create directory $0", data_dir));
     if (created) {
-      dirs_to_delete.emplace_back(data_dir);
+      created_dirs.emplace_back(data_dir);
     }
 
     if (opts_.block_manager_type == "log") {
@@ -407,16 +425,19 @@ Status DataDirManager::Create() {
     string instance_filename = JoinPathSegments(data_dir, kInstanceMetadataFileName);
     PathInstanceMetadataFile metadata(env_, opts_.block_manager_type,
                                       instance_filename);
-    RETURN_NOT_OK_PREPEND(metadata.Create(all_uuids[idx], all_uuids), instance_filename);
-    files_to_delete.emplace_back(instance_filename);
-
-    idx++;
+    RETURN_NOT_OK_PREPEND(metadata.Create(p.second, all_uuids), instance_filename);
+    created_files.emplace_back(instance_filename);
   }
 
+  // Update existing instances, if any.
+  RETURN_NOT_OK_PREPEND(UpdateExistingInstances(
+      std::move(instances_to_update), std::move(all_uuids)),
+                        "could not update existing data directories");
+
   // Ensure newly created directories are synchronized to disk.
   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");
+    WARN_NOT_OK(env_util::SyncAllParentDirs(env_, created_dirs, created_files),
+                "could not sync newly created data directories");
   }
 
   // Success: don't delete any files.
@@ -424,9 +445,70 @@ Status DataDirManager::Create() {
   return Status::OK();
 }
 
-Status DataDirManager::Open() {
-  vector<PathInstanceMetadataFile*> instances;
-  vector<unique_ptr<DataDir>> dds;
+Status DataDirManager::UpdateExistingInstances(
+    vector<unique_ptr<PathInstanceMetadataFile>> instances_to_update,
+    vector<string> new_all_uuids) {
+  // Prepare a scoped cleanup for managing instance metadata copies.
+  unordered_map<string, string> copies_to_restore;
+  unordered_set<string> copies_to_delete;
+  auto copy_cleanup = MakeScopedCleanup([&]() {
+    for (const auto& f : copies_to_delete) {
+      WARN_NOT_OK(env_->DeleteFile(f), "Could not delete file " + f);
+    }
+    for (const auto& f : copies_to_restore) {
+      WARN_NOT_OK(env_->RenameFile(f.first, f.second),
+                  Substitute("Could not restore file $0 from $1", f.second, f.first));
+    }
+  });
+
+  // Make a copy of every existing instance metadata file.
+  //
+  // This is done before performing any updates, so that if there's a failure
+  // while copying, there's no metadata to restore.
+  WritableFileOptions opts;
+  opts.sync_on_close = true;
+  for (const auto& instance : instances_to_update) {
+    const string& instance_filename = instance->path();
+    string copy_filename = instance_filename + kTmpInfix;
+    RETURN_NOT_OK_PREPEND(env_util::CopyFile(
+        env_, instance_filename, copy_filename, opts),
+                          "unable to backup existing data directory instance metadata");
+    InsertOrDie(&copies_to_delete, copy_filename);
+  }
+
+  // Update existing instance metadata files with the new value of all_uuids.
+  for (const auto& instance : instances_to_update) {
+    const string& instance_filename = instance->path();
+    string copy_filename = instance_filename + kTmpInfix;
+
+    // We've made enough progress on this instance that we should restore its
+    // copy on failure, even if the update below fails. That's because it's a
+    // multi-step process and it's possible for it to return failure despite
+    // the update taking place (e.g. synchronization failure).
+    CHECK_EQ(1, copies_to_delete.erase(copy_filename));
+    InsertOrDie(&copies_to_restore, copy_filename, instance_filename);
+
+    // Perform the update.
+    PathInstanceMetadataPB new_pb = *instance->metadata();
+    new_pb.mutable_path_set()->mutable_all_uuids()->Clear();
+    for (const auto& uuid : new_all_uuids) {
+      new_pb.mutable_path_set()->add_all_uuids(uuid);
+    }
+    RETURN_NOT_OK_PREPEND(pb_util::WritePBContainerToPath(
+        env_, instance_filename, new_pb, pb_util::OVERWRITE,
+        FLAGS_enable_data_block_fsync ? pb_util::SYNC : pb_util::NO_SYNC),
+                          "unable to overwrite existing data directory instance metadata");
+  }
+
+  // Success; instance metadata copies will be deleted by 'copy_cleanup'.
+  InsertKeysFromMap(copies_to_restore, &copies_to_delete);
+  copies_to_restore.clear();
+  return Status::OK();
+}
+
+Status DataDirManager::LoadInstances(
+    vector<string>* missing_roots,
+    vector<unique_ptr<PathInstanceMetadataFile>>* loaded_instances) {
   LockMode lock_mode;
   if (!FLAGS_fs_lock_data_dirs) {
     lock_mode = LockMode::NONE;
@@ -435,26 +517,34 @@ Status DataDirManager::Open() {
   } else {
     lock_mode = LockMode::MANDATORY;
   }
-  const int kMaxDataDirs = opts_.block_manager_type == "file" ? (1 << 16) - 1 : kint32max;
-
-  int i = 0;
-  // Create a directory for all data dirs specified by the user.
-  for (const auto& root : canonicalized_data_fs_roots_) {
+  vector<string> missing_roots_tmp;
+  vector<unique_ptr<PathInstanceMetadataFile>> loaded_instances_tmp;
+  for (int i = 0; i < canonicalized_data_fs_roots_.size(); i++) {
+    const auto& root = canonicalized_data_fs_roots_[i];
     string data_dir = JoinPathSegments(root.path, kDataDirName);
     string instance_filename = JoinPathSegments(data_dir, kInstanceMetadataFileName);
-    // Open and lock the data dir's metadata instance file.
-    gscoped_ptr<PathInstanceMetadataFile> instance(
-        new PathInstanceMetadataFile(env_, opts_.block_manager_type,
-                                     instance_filename));
+
+    unique_ptr<PathInstanceMetadataFile> instance(
+        new PathInstanceMetadataFile(env_, opts_.block_manager_type, instance_filename));
     if (PREDICT_FALSE(!root.status.ok())) {
+      DCHECK_GT(i, 0);  // Guaranteed by FsManager::Init().
       instance->SetInstanceFailed(root.status);
     } else {
-      RETURN_NOT_OK_PREPEND(instance->LoadFromDisk(),
-                            Substitute("Could not open $0", instance_filename));
+      // This may return OK and mark 'instance' as failed.
+      Status s = instance->LoadFromDisk();
+      if (s.IsNotFound() && opts_.update_on_disk && i > 0) {
+        // A missing instance is only tolerated if we've been asked to add new
+        // data directories and this isn't the first one (i.e. the data
+        // directory used for metadata).
+        missing_roots_tmp.emplace_back(root.path);
+        continue;
+      }
+      RETURN_NOT_OK_PREPEND(s, Substitute("could not load $0", instance_filename));
     }
 
-    // Try locking the directory.
-    if (lock_mode != LockMode::NONE) {
+    // Try locking the instance.
+    if (instance->healthy() && lock_mode != LockMode::NONE) {
+      // This may return OK and mark 'instance' as failed.
       Status s = instance->Lock();
       if (!s.ok()) {
         Status new_status = s.CloneAndPrepend(Substitute(
@@ -469,13 +559,83 @@ Status DataDirManager::Open() {
       }
     }
 
-    // Crash if the metadata directory, i.e. the first specified by the user, failed.
-    if (!instance->healthy() && i == 0) {
-      return Status::IOError(Substitute("Could not open directory manager; "
-          "metadata directory failed: $0", instance->health_status().ToString()));
+    loaded_instances_tmp.emplace_back(std::move(instance));
+  }
+
+  // Check that the first data directory (used for metadata) exists and is healthy.
+  CHECK(!loaded_instances_tmp.empty());  // enforced above
+  if (!loaded_instances_tmp[0]->healthy()) {
+    return Status::IOError(Substitute(
+        "could not open directory manager; metadata directory failed: $0",
+        loaded_instances_tmp[0]->health_status().ToString()));
+  }
+  missing_roots->swap(missing_roots_tmp);
+  loaded_instances->swap(loaded_instances_tmp);
+  return Status::OK();
+}
+
+Status DataDirManager::Open() {
+  const int kMaxDataDirs = opts_.block_manager_type == "file" ? (1 << 16) - 1 : kint32max;
+
+  // Find and load existing data directory instances.
+  vector<string> missing_roots;
+  vector<unique_ptr<PathInstanceMetadataFile>> loaded_instances;
+  RETURN_NOT_OK(LoadInstances(&missing_roots, &loaded_instances));
+
+  // Check the integrity of all loaded instances.
+  RETURN_NOT_OK_PREPEND(
+      PathInstanceMetadataFile::CheckIntegrity(loaded_instances),
+      Substitute("could not verify integrity of files: $0",
+                 JoinStrings(GetDataDirs(), ",")));
+
+  // Create any missing data directories, if desired.
+  if (!missing_roots.empty()) {
+    if (opts_.block_manager_type == "file") {
+      return Status::InvalidArgument(
+          "file block manager may not add new data directories");
+    }
+
+    // Prepare to create new directories and update existing instances. We
+    // must generate a new UUID for each missing root, and update all_uuids in
+    // all existing instances to include those new UUIDs.
+    //
+    // Note: all data directories must be healthy to perform this operation.
+    ObjectIdGenerator gen;
+    vector<string> new_all_uuids;
+    vector<pair<string, string>> root_uuid_pairs_to_create;
+    for (const auto& i : loaded_instances) {
+      RETURN_NOT_OK_PREPEND(
+          i->health_status(),
+          "found failed data directory while adding new data directories");
+      new_all_uuids.emplace_back(i->metadata()->path_set().uuid());
+    }
+    for (const auto& m : missing_roots) {
+      string uuid = gen.Next();
+      new_all_uuids.emplace_back(uuid);
+      root_uuid_pairs_to_create.emplace_back(m, std::move(uuid));
     }
+    RETURN_NOT_OK_PREPEND(
+        CreateNewDataDirectoriesAndUpdateExistingOnes(
+            std::move(root_uuid_pairs_to_create),
+            std::move(loaded_instances),
+            std::move(new_all_uuids)),
+            "could not add new data directories");
+
+    // Now that we've created the missing directories, try loading the
+    // directories again.
+    // Note: 'loaded_instances' must be cleared to unlock the instance files.
+    loaded_instances.clear();
+    missing_roots.clear();
+    RETURN_NOT_OK(LoadInstances(&missing_roots, &loaded_instances));
+    DCHECK(missing_roots.empty());
+  }
 
-    instances.push_back(instance.get());
+  // All instances are present and accounted for. Time to create the in-memory
+  // data directory structures.
+  int i = 0;
+  vector<unique_ptr<DataDir>> dds;
+  for (auto& instance : loaded_instances) {
+    const string data_dir = instance->dir();
 
     // Create a per-dir thread pool.
     gscoped_ptr<ThreadPool> pool;
@@ -499,22 +659,13 @@ Status DataDirManager::Open() {
       }
     }
 
-    // Create the data directory in-memory structure itself.
     unique_ptr<DataDir> dd(new DataDir(
-        env_, metrics_.get(), fs_type, data_dir,
-        unique_ptr<PathInstanceMetadataFile>(instance.release()),
+        env_, metrics_.get(), fs_type, data_dir, std::move(instance),
         unique_ptr<ThreadPool>(pool.release())));
-
     dds.emplace_back(std::move(dd));
     i++;
   }
 
-  // Check integrity and determine which instances need updating.
-  RETURN_NOT_OK_PREPEND(
-      PathInstanceMetadataFile::CheckIntegrity(instances),
-      Substitute("Could not verify integrity of files: $0",
-                 JoinStrings(GetDataDirs(), ",")));
-
   // Use the per-dir thread pools to delete temporary files in parallel.
   for (const auto& dd : dds) {
     if (dd->instance()->healthy()) {
@@ -562,7 +713,7 @@ Status DataDirManager::Open() {
     DCHECK_NE(idx, -1); // Guaranteed by CheckIntegrity().
     if (idx > kMaxDataDirs) {
       return Status::NotSupported(
-          Substitute("Block manager supports a maximum of $0 paths", kMaxDataDirs));
+          Substitute("block manager supports a maximum of $0 paths", kMaxDataDirs));
     }
     insert_to_maps(idx, path_set.uuid(), dd.get());
   }

http://git-wip-us.apache.org/repos/asf/kudu/blob/044d5fe6/src/kudu/fs/data_dirs.h
----------------------------------------------------------------------
diff --git a/src/kudu/fs/data_dirs.h b/src/kudu/fs/data_dirs.h
index 0671fae..d886f91 100644
--- a/src/kudu/fs/data_dirs.h
+++ b/src/kudu/fs/data_dirs.h
@@ -64,6 +64,7 @@ class PathInstanceMetadataFile;
 struct CreateBlockOptions;
 
 const char kInstanceMetadataFileName[] = "block_manager_instance";
+const char kDataDirName[] = "data";
 
 namespace internal {
 
@@ -216,6 +217,15 @@ struct DataDirManagerOptions {
   //
   // Defaults to false.
   bool read_only;
+
+  // Whether or not the provided directories should be considered the new
+  // canonical set. If true, on-disk data structures will be updated
+  // accordingly when the DataDirManager is opened.
+  //
+  // Defaults to false. If true, 'read_only' must be false.
+  //
+  // Currently only supports adding new data directories.
+  bool update_on_disk;
 };
 
 // Encapsulates knowledge of data directory management on behalf of block
@@ -381,9 +391,6 @@ class DataDirManager {
   FRIEND_TEST(DataDirsTest, TestLoadBalancingDistribution);
   FRIEND_TEST(DataDirsTest, TestFailedDirNotAddedToGroup);
 
-  // The base name of a data directory.
-  static const char* kDataDirName;
-
   // Constructs a directory manager.
   DataDirManager(Env* env,
                  DataDirManagerOptions opts,
@@ -402,6 +409,32 @@ class DataDirManager {
   // metadata directory (i.e. the first one) fails to load.
   Status Open();
 
+  // Loads the instance files for each directory, populating 'missing_roots'
+  // with the data roots that are missing instance files, and
+  // 'loaded_instances' with successfully loaded instances.
+  //
+  // Returns an error if the metadata directory fails to load.
+  Status LoadInstances(std::vector<std::string>* missing_roots,
+      std::vector<std::unique_ptr<PathInstanceMetadataFile>>* loaded_instances);
+
+  // Initializes new data directories specified by 'root_uuid_pairs_to_create'
+  // and updates the on-disk instance files of existing data directories
+  // specified by 'instances_to_update' using the contents of 'all_uuids'.
+  //
+  // Returns an error if any disk operations fail.
+  Status CreateNewDataDirectoriesAndUpdateExistingOnes(
+      std::vector<std::pair<std::string, std::string>> root_uuid_pairs_to_create,
+      std::vector<std::unique_ptr<PathInstanceMetadataFile>> instances_to_update,
+      std::vector<std::string> all_uuids);
+
+  // Updates the on-disk instance files specified by 'instances_to_update'
+  // using the contents of 'new_all_uuids'.
+  //
+  // Returns an error if any disk operations fail.
+  Status UpdateExistingInstances(
+      std::vector<std::unique_ptr<PathInstanceMetadataFile>> instances_to_update,
+      std::vector<std::string> new_all_uuids);
+
   // Repeatedly selects directories from those available to put into a new
   // DataDirGroup until 'group_indices' reaches 'target_size' elements.
   // Selection is based on "The Power of Two Choices in Randomized Load

http://git-wip-us.apache.org/repos/asf/kudu/blob/044d5fe6/src/kudu/fs/fs_manager-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/fs_manager-test.cc b/src/kudu/fs/fs_manager-test.cc
index 9261457..5df0f57 100644
--- a/src/kudu/fs/fs_manager-test.cc
+++ b/src/kudu/fs/fs_manager-test.cc
@@ -28,6 +28,7 @@
 #include <utility>
 #include <vector>
 
+#include <gflags/gflags.h>
 #include <gflags/gflags_declare.h>
 #include <glog/logging.h>
 #include <glog/stl_logging.h>
@@ -36,7 +37,6 @@
 #include "kudu/fs/block_manager.h"
 #include "kudu/fs/data_dirs.h"
 #include "kudu/fs/fs_manager.h"
-#include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/gutil/map-util.h"
 #include "kudu/gutil/port.h"
 #include "kudu/gutil/stringprintf.h"
@@ -61,6 +61,7 @@ using strings::Substitute;
 
 DECLARE_bool(crash_on_eio);
 DECLARE_double(env_inject_eio);
+DECLARE_string(block_manager);
 DECLARE_string(env_inject_eio_globs);
 DECLARE_string(umask);
 
@@ -68,6 +69,10 @@ namespace kudu {
 
 class FsManagerTestBase : public KuduTest {
  public:
+  FsManagerTestBase()
+     : fs_root_(GetTestPath("fs_root")) {
+  }
+
   void SetUp() OVERRIDE {
     KuduTest::SetUp();
 
@@ -78,13 +83,17 @@ class FsManagerTestBase : public KuduTest {
   }
 
   void ReinitFsManager() {
-    ReinitFsManager(GetTestPath("fs_root"), { GetTestPath("fs_root")} );
+    ReinitFsManagerWithPaths(fs_root_, { fs_root_ });
   }
 
-  void ReinitFsManager(const string& wal_path, const vector<string>& data_paths) {
+  void ReinitFsManagerWithPaths(string wal_path, vector<string> data_paths) {
     FsManagerOpts opts;
-    opts.wal_root = wal_path;
-    opts.data_roots = data_paths;
+    opts.wal_root = std::move(wal_path);
+    opts.data_roots = std::move(data_paths);
+    ReinitFsManagerWithOpts(std::move(opts));
+  }
+
+  void ReinitFsManagerWithOpts(FsManagerOpts opts) {
     fs_manager_.reset(new FsManager(env_, std::move(opts)));
   }
 
@@ -108,8 +117,11 @@ class FsManagerTestBase : public KuduTest {
 
   FsManager *fs_manager() const { return fs_manager_.get(); }
 
+ protected:
+  const string fs_root_;
+
  private:
-  gscoped_ptr<FsManager> fs_manager_;
+  unique_ptr<FsManager> fs_manager_;
 };
 
 TEST_F(FsManagerTestBase, TestBaseOperations) {
@@ -124,7 +136,7 @@ TEST_F(FsManagerTestBase, TestBaseOperations) {
 TEST_F(FsManagerTestBase, TestIllegalPaths) {
   vector<string> illegal = { "", "asdf", "/foo\n\t" };
   for (const string& path : illegal) {
-    ReinitFsManager(path, { path });
+    ReinitFsManagerWithPaths(path, { path });
     ASSERT_TRUE(fs_manager()->CreateInitialFileSystemLayout().IsIOError());
   }
 }
@@ -132,7 +144,7 @@ TEST_F(FsManagerTestBase, TestIllegalPaths) {
 TEST_F(FsManagerTestBase, TestMultiplePaths) {
   string wal_path = GetTestPath("a");
   vector<string> data_paths = { GetTestPath("a"), GetTestPath("b"), GetTestPath("c") };
-  ReinitFsManager(wal_path, data_paths);
+  ReinitFsManagerWithPaths(wal_path, data_paths);
   ASSERT_OK(fs_manager()->CreateInitialFileSystemLayout());
   ASSERT_OK(fs_manager()->Open());
 }
@@ -140,13 +152,13 @@ TEST_F(FsManagerTestBase, TestMultiplePaths) {
 TEST_F(FsManagerTestBase, TestMatchingPathsWithMismatchedSlashes) {
   string wal_path = GetTestPath("foo");
   vector<string> data_paths = { wal_path + "/" };
-  ReinitFsManager(wal_path, data_paths);
+  ReinitFsManagerWithPaths(wal_path, data_paths);
   ASSERT_OK(fs_manager()->CreateInitialFileSystemLayout());
 }
 
 TEST_F(FsManagerTestBase, TestDuplicatePaths) {
   string path = GetTestPath("foo");
-  ReinitFsManager(path, { path, path, path });
+  ReinitFsManagerWithPaths(path, { path, path, path });
   ASSERT_OK(fs_manager()->CreateInitialFileSystemLayout());
   ASSERT_EQ(vector<string>({ JoinPathSegments(path, fs_manager()->kDataDirName) }),
             fs_manager()->GetDataRootDirs());
@@ -182,12 +194,12 @@ TEST_F(FsManagerTestBase, TestCannotUseNonEmptyFsRoot) {
   }
 
   // Try to create the FS layout. It should fail.
-  ReinitFsManager(path, { path });
+  ReinitFsManagerWithPaths(path, { path });
   ASSERT_TRUE(fs_manager()->CreateInitialFileSystemLayout().IsAlreadyPresent());
 }
 
 TEST_F(FsManagerTestBase, TestEmptyWALPath) {
-  ReinitFsManager("", {});
+  ReinitFsManagerWithPaths("", {});
   Status s = fs_manager()->CreateInitialFileSystemLayout();
   ASSERT_TRUE(s.IsIOError());
   ASSERT_STR_CONTAINS(s.ToString(), "directory (fs_wal_dir) not provided");
@@ -197,7 +209,7 @@ TEST_F(FsManagerTestBase, TestOnlyWALPath) {
   string path = GetTestPath("new_fs_root");
   ASSERT_OK(env_->CreateDir(path));
 
-  ReinitFsManager(path, {});
+  ReinitFsManagerWithPaths(path, {});
   ASSERT_OK(fs_manager()->CreateInitialFileSystemLayout());
   ASSERT_TRUE(HasPrefixString(fs_manager()->GetWalsRootDir(), path));
   ASSERT_TRUE(HasPrefixString(fs_manager()->GetConsensusMetadataDir(), path));
@@ -209,7 +221,7 @@ TEST_F(FsManagerTestBase, TestOnlyWALPath) {
 
 TEST_F(FsManagerTestBase, TestFormatWithSpecificUUID) {
   string path = GetTestPath("new_fs_root");
-  ReinitFsManager(path, {});
+  ReinitFsManagerWithPaths(path, {});
 
   // Use an invalid uuid at first.
   string uuid = "not_a_valid_uuid";
@@ -276,9 +288,9 @@ TEST_F(FsManagerTestBase, TestCreateWithFailedDirs) {
 
   // Fail a directory, avoiding the metadata directory.
   FLAGS_env_inject_eio_globs = data_paths[1];
-  ReinitFsManager(wal_path, data_roots);
+  ReinitFsManagerWithPaths(wal_path, data_roots);
   Status s = fs_manager()->CreateInitialFileSystemLayout();
-  ASSERT_STR_MATCHES(s.ToString(), "Cannot create FS layout; at least one directory "
+  ASSERT_STR_MATCHES(s.ToString(), "cannot create FS layout; at least one directory "
                                    "failed to canonicalize");
   FLAGS_env_inject_eio = 0;
 }
@@ -292,14 +304,14 @@ TEST_F(FsManagerTestBase, TestOpenWithFailedDirs) {
   }
   vector<string> data_roots = { GetTestPath("data1/roots"), GetTestPath("data2/roots"),
                                 GetTestPath("data3/roots") };
-  ReinitFsManager(wal_path, data_roots);
+  ReinitFsManagerWithPaths(wal_path, data_roots);
   ASSERT_OK(fs_manager()->CreateInitialFileSystemLayout());
 
   // Now fail one of the directories.
   FLAGS_crash_on_eio = false;
   FLAGS_env_inject_eio = 1.0;
   FLAGS_env_inject_eio_globs = JoinPathSegments(Substitute("$0,$0", data_paths[1]), "**");
-  ReinitFsManager(wal_path, data_roots);
+  ReinitFsManagerWithPaths(wal_path, data_roots);
   ASSERT_OK(fs_manager()->Open(nullptr));
   ASSERT_EQ(1, fs_manager()->dd_manager()->GetFailedDataDirs().size());
 
@@ -309,7 +321,7 @@ TEST_F(FsManagerTestBase, TestOpenWithFailedDirs) {
 TEST_F(FsManagerTestBase, TestTmpFilesCleanup) {
   string wal_path = GetTestPath("wals");
   vector<string> data_paths = { GetTestPath("data1"), GetTestPath("data2"), GetTestPath("data3") };
-  ReinitFsManager(wal_path, data_paths);
+  ReinitFsManagerWithPaths(wal_path, data_paths);
   ASSERT_OK(fs_manager()->CreateInitialFileSystemLayout());
 
   // Create a few tmp files here
@@ -352,7 +364,7 @@ TEST_F(FsManagerTestBase, TestTmpFilesCleanup) {
   ASSERT_EQ(6, n_tmp_files);
 
   // Opening fs_manager should remove tmp files
-  ReinitFsManager(wal_path, data_paths);
+  ReinitFsManagerWithPaths(wal_path, data_paths);
   ASSERT_OK(fs_manager()->Open());
 
   n_tmp_files = 0;
@@ -385,7 +397,7 @@ TEST_F(FsManagerTestBase, TestUmask) {
   HandleCommonFlags();
   ASSERT_EQ(007, g_parsed_umask);
   root = GetTestPath("new_root");
-  ReinitFsManager({ root }, { root });
+  ReinitFsManagerWithPaths(root, { root });
   ASSERT_OK(fs_manager()->CreateInitialFileSystemLayout());
   EXPECT_EQ("770", FilePermsAsString(root));
   EXPECT_EQ("770", FilePermsAsString(fs_manager()->GetConsensusMetadataDir()));
@@ -396,7 +408,7 @@ TEST_F(FsManagerTestBase, TestUmask) {
   FLAGS_umask = "077";
   HandleCommonFlags();
   ASSERT_EQ(077, g_parsed_umask);
-  ReinitFsManager({ root }, { root });
+  ReinitFsManagerWithPaths(root, { root });
   ASSERT_OK(fs_manager()->Open());
   EXPECT_EQ("700", FilePermsAsString(root));
 }
@@ -417,4 +429,170 @@ TEST_F(FsManagerTestBase, TestOpenFailsWhenMissingImportantDir) {
   ASSERT_STR_CONTAINS(s.ToString(), "exists but is not a directory");
 }
 
+
+TEST_F(FsManagerTestBase, TestAddDataDirs) {
+  if (FLAGS_block_manager == "file") {
+    LOG(INFO) << "Skipping test, file block manager not supported";
+    return;
+  }
+
+  // Try to open with a new data dir in the list to be opened; this should fail.
+  const string new_path1 = GetTestPath("new_path1");
+  FsManagerOpts opts;
+  opts.wal_root = fs_root_;
+  opts.data_roots = { fs_root_, new_path1 };
+  ReinitFsManagerWithOpts(opts);
+  Status s = fs_manager()->Open();
+  ASSERT_TRUE(s.IsNotFound());
+  ASSERT_STR_CONTAINS(s.ToString(), fs_manager()->GetInstanceMetadataPath(new_path1));
+
+  // This time allow new data directories to be created.
+  opts.update_on_disk = true;
+  ReinitFsManagerWithOpts(opts);
+  ASSERT_OK(fs_manager()->Open());
+  ASSERT_EQ(2, fs_manager()->dd_manager()->GetDataDirs().size());
+
+  // Try to open with a data dir removed; this should fail.
+  opts.data_roots = { fs_root_ };
+  ReinitFsManagerWithOpts(opts);
+  s = fs_manager()->Open();
+  ASSERT_TRUE(s.IsIOError());
+  ASSERT_STR_CONTAINS(s.ToString(), "could not verify integrity of files");
+
+  // Try to open with a new data dir at the front of the list; this should fail.
+  const string new_path2 = GetTestPath("new_path2");
+  opts.data_roots = { new_path2, fs_root_, new_path1 };
+  ReinitFsManagerWithOpts(opts);
+  s = fs_manager()->Open();
+  ASSERT_TRUE(s.IsNotFound());
+  ASSERT_STR_CONTAINS(s.ToString(), "could not verify required directory");
+
+  // But adding a new data dir elsewhere in the list is OK.
+  opts.data_roots = { fs_root_, new_path2, new_path1 };
+  ReinitFsManagerWithOpts(opts);
+  ASSERT_OK(fs_manager()->Open());
+  ASSERT_EQ(3, fs_manager()->dd_manager()->GetDataDirs().size());
+
+  // Try to open with a new data dir although an existing data dir has failed;
+  // this should fail.
+  {
+    google::FlagSaver saver;
+    FLAGS_crash_on_eio = false;
+    FLAGS_env_inject_eio = 1.0;
+    FLAGS_env_inject_eio_globs = JoinPathSegments(new_path2, "**");
+
+    const string new_path3 = GetTestPath("new_path3");
+    opts.data_roots = { fs_root_, new_path2, new_path1, new_path3 };
+    ReinitFsManagerWithOpts(opts);
+    Status s = fs_manager()->Open();
+    ASSERT_TRUE(s.IsIOError());
+    ASSERT_STR_CONTAINS(s.ToString(), "found failed data directory");
+  }
+}
+
+TEST_F(FsManagerTestBase, TestAddDataDirsFuzz) {
+  const int kNumAttempts = AllowSlowTests() ? 1000 : 100;
+
+  if (FLAGS_block_manager == "file") {
+    LOG(INFO) << "Skipping test, file block manager not supported";
+    return;
+  }
+
+  FsManagerOpts fs_opts;
+  fs_opts.wal_root = fs_root_;
+  fs_opts.data_roots = { fs_root_ };
+  for (int i = 0; i < kNumAttempts; i++) {
+    // Create a new fs root.
+    string new_data_root = GetTestPath(Substitute("new_data_$0", i));
+    fs_opts.data_roots.emplace_back(new_data_root);
+
+    // Try to add it with failure injection enabled.
+    bool add_succeeded;
+    {
+      google::FlagSaver saver;
+      FLAGS_crash_on_eio = false;
+
+      // This value isn't arbitrary: most attempts fail and only some succeed.
+      FLAGS_env_inject_eio = 0.01;
+
+      fs_opts.update_on_disk = true;
+      ReinitFsManagerWithOpts(fs_opts);
+      add_succeeded = fs_manager()->Open().ok();
+    }
+
+    // Reopen regardless, to ensure that failures didn't corrupt anything.
+    fs_opts.update_on_disk = false;
+    ReinitFsManagerWithOpts(fs_opts);
+    Status open_status = fs_manager()->Open();
+    if (add_succeeded) {
+      ASSERT_OK(open_status);
+    }
+
+    // The rollback logic built into the add fs root operation isn't robust
+    // enough to handle every possible sequence of injected failures. Let's see
+    // if we need to apply a "workaround" in order to fix the filesystem.
+    if (!open_status.ok()) {
+      // Failed? Perhaps the new fs root and data directory were created, but
+      // there was an injected failure later on, which led to a rollback and
+      // the removal of the new fs instance file.
+      //
+      // Fix this as a user might (by copying the original fs instance file
+      // into the new fs root) then retry.
+      string source_instance = fs_manager()->GetInstanceMetadataPath(fs_root_);
+      bool is_dir;
+      Status s = env_->IsDirectory(new_data_root, &is_dir);
+      if (s.ok()) {
+        ASSERT_TRUE(is_dir);
+        string new_instance = fs_manager()->GetInstanceMetadataPath(new_data_root);
+        if (!env_->FileExists(new_instance)) {
+          WritableFileOptions wr_opts;
+          wr_opts.mode = Env::CREATE_NON_EXISTING;
+          ASSERT_OK(env_util::CopyFile(env_, source_instance, new_instance, wr_opts));
+          ReinitFsManagerWithOpts(fs_opts);
+          open_status = fs_manager()->Open();
+        }
+      }
+    }
+    if (!open_status.ok()) {
+      // Still failing? Unfortunately, there's not enough information to know
+      // whether the injected failure occurred during creation of the new data
+      // directory or just afterwards, when the DataDirManager reopened itself.
+      // If the former, the add itself failed and the failure should resolve if
+      // we drop the new fs root and retry.
+      fs_opts.data_roots.pop_back();
+      ReinitFsManagerWithOpts(fs_opts);
+      open_status = fs_manager()->Open();
+    }
+    if (!open_status.ok()) {
+      // We're still failing? Okay, there's only one legitimate case left, and
+      // that's if an error was injected during the update of existing data
+      // directory instance files AND during the restoration phase of cleanup.
+      //
+      // Fix this as a user might (by completing the restoration phase
+      // manually), then retry.
+      ASSERT_TRUE(open_status.IsIOError());
+      bool repaired = false;
+      for (const auto& root : fs_opts.data_roots) {
+        string data_dir = JoinPathSegments(root, fs::kDataDirName);
+        string instance = JoinPathSegments(data_dir,
+                                           fs::kInstanceMetadataFileName);
+        ASSERT_TRUE(env_->FileExists(instance));
+        string copy = instance + kTmpInfix;
+        if (env_->FileExists(copy)) {
+          ASSERT_OK(env_->RenameFile(copy, instance));
+          repaired = true;
+        }
+      }
+      if (repaired) {
+        ReinitFsManagerWithOpts(fs_opts);
+        open_status = fs_manager()->Open();
+      }
+    }
+
+    // We've exhausted all of our manual repair options; if this still fails,
+    // something else is wrong.
+    ASSERT_OK(open_status);
+  }
+}
+
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/044d5fe6/src/kudu/fs/fs_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/fs_manager.cc b/src/kudu/fs/fs_manager.cc
index 4f1679c..34b81ae 100644
--- a/src/kudu/fs/fs_manager.cc
+++ b/src/kudu/fs/fs_manager.cc
@@ -20,8 +20,9 @@
 #include <cinttypes>
 #include <ctime>
 #include <iostream>
-#include <map>
 #include <set>
+#include <unordered_map>
+#include <unordered_set>
 #include <utility>
 
 #include <boost/optional/optional.hpp>
@@ -101,11 +102,11 @@ using kudu::fs::LogBlockManager;
 using kudu::fs::ReadableBlock;
 using kudu::fs::WritableBlock;
 using kudu::pb_util::SecureDebugString;
-using std::map;
 using std::ostream;
-using std::set;
 using std::string;
 using std::unique_ptr;
+using std::unordered_map;
+using std::unordered_set;
 using std::vector;
 using strings::Substitute;
 
@@ -126,7 +127,8 @@ const char *FsManager::kConsensusMetadataDirName = "consensus-meta";
 FsManagerOpts::FsManagerOpts()
   : wal_root(FLAGS_fs_wal_dir),
     block_manager_type(FLAGS_block_manager),
-    read_only(false) {
+    read_only(false),
+    update_on_disk(false) {
   data_roots = strings::Split(FLAGS_fs_data_dirs, ",", strings::SkipEmpty());
 }
 
@@ -134,19 +136,23 @@ FsManagerOpts::FsManagerOpts(const string& root)
   : wal_root(root),
     data_roots({ root }),
     block_manager_type(FLAGS_block_manager),
-    read_only(false) {}
+    read_only(false),
+    update_on_disk(false) {}
 
 FsManager::FsManager(Env* env, const string& root_path)
   : env_(DCHECK_NOTNULL(env)),
     opts_(FsManagerOpts(root_path)),
     error_manager_(new FsErrorManager()),
-    initted_(false) {}
+    initted_(false) {
+}
 
 FsManager::FsManager(Env* env, FsManagerOpts opts)
   : env_(DCHECK_NOTNULL(env)),
     opts_(std::move(opts)),
     error_manager_(new FsErrorManager()),
-    initted_(false) {}
+    initted_(false) {
+  DCHECK(!opts_.update_on_disk || !opts_.read_only);
+}
 
 FsManager::~FsManager() {}
 
@@ -169,12 +175,12 @@ Status FsManager::Init() {
   }
 
   // Deduplicate all of the roots.
-  set<string> all_roots = { opts_.wal_root };
+  unordered_set<string> all_roots = { opts_.wal_root };
   all_roots.insert(opts_.data_roots.begin(), opts_.data_roots.end());
 
   // Build a map of original root --> canonicalized root, sanitizing each
   // root as we go and storing the canonicalization status.
-  typedef map<string, CanonicalizedRootAndStatus> RootMap;
+  typedef unordered_map<string, CanonicalizedRootAndStatus> RootMap;
   RootMap canonicalized_roots;
   for (const string& root : all_roots) {
     if (root.empty()) {
@@ -209,9 +215,10 @@ Status FsManager::Init() {
   }
 
   // All done, use the map to set the canonicalized state.
+
   canonicalized_wal_fs_root_ = FindOrDie(canonicalized_roots, opts_.wal_root);
   if (!opts_.data_roots.empty()) {
-    set<string> unique_roots;
+    unordered_set<string> unique_roots;
     for (const string& data_fs_root : opts_.data_roots) {
       const auto& root = FindOrDie(canonicalized_roots, data_fs_root);
       if (InsertIfNotPresent(&unique_roots, root.path)) {
@@ -274,6 +281,7 @@ Status FsManager::Open(FsReport* report) {
   //
   // Done first to minimize side effects in the case that the configured roots
   // are not yet initialized on disk.
+  CanonicalizedRootsList missing_roots;
   for (auto& root : canonicalized_all_fs_roots_) {
     if (!root.status.ok()) {
       continue;
@@ -282,6 +290,10 @@ Status FsManager::Open(FsReport* report) {
     Status s = pb_util::ReadPBContainerFromPath(env_, GetInstanceMetadataPath(root.path),
                                                 pb.get());
     if (PREDICT_FALSE(!s.ok())) {
+      if (s.IsNotFound() && opts_.update_on_disk) {
+        missing_roots.emplace_back(root);
+        continue;
+      }
       if (s.IsDiskFailure()) {
         root.status = s.CloneAndPrepend("Failed to open instance file");
         continue;
@@ -299,6 +311,10 @@ Status FsManager::Open(FsReport* report) {
     }
   }
 
+  if (!metadata_) {
+    return Status::Corruption("All instance files are missing");
+  }
+
   // Ensure all of the ancillary directories exist.
   vector<string> ancillary_dirs = { GetWalsRootDir(),
                                     GetTabletMetadataDir(),
@@ -313,6 +329,28 @@ Status FsManager::Open(FsReport* report) {
     }
   }
 
+  // In the event of failure, delete everything we created.
+  vector<string> created_dirs;
+  vector<string> created_files;
+  auto deleter = MakeScopedCleanup([&]() {
+    // Delete files first so that the directories will be empty when deleted.
+    for (const auto& f : created_dirs) {
+      WARN_NOT_OK(env_->DeleteFile(f), "Could not delete file " + f);
+    }
+    // Delete directories in reverse order since parent directories will have
+    // been added before child directories.
+    for (auto it = created_dirs.rbegin(); it != created_dirs.rend(); it++) {
+      WARN_NOT_OK(env_->DeleteDir(*it), "Could not delete dir " + *it);
+    }
+  });
+
+  // Create any missing roots.
+  if (!missing_roots.empty()) {
+    RETURN_NOT_OK_PREPEND(CreateFileSystemRoots(
+        missing_roots, *metadata_, &created_dirs, &created_files),
+                          "unable to create missing filesystem roots");
+  }
+
   // Remove leftover temporary files from the WAL root and fix permissions.
   //
   // Temporary files in the data directory roots will be removed by the block
@@ -328,6 +366,7 @@ Status FsManager::Open(FsReport* report) {
     dm_opts.metric_entity = opts_.metric_entity;
     dm_opts.block_manager_type = opts_.block_manager_type;
     dm_opts.read_only = opts_.read_only;
+    dm_opts.update_on_disk = opts_.update_on_disk;
     LOG_TIMING(INFO, "opening directory manager") {
       RETURN_NOT_OK(DataDirManager::OpenExisting(env_,
           canonicalized_data_fs_roots_, std::move(dm_opts), &dd_manager_));
@@ -353,9 +392,28 @@ Status FsManager::Open(FsReport* report) {
     return Status::IOError("Cannot open the FS layout; metadata directory failed");
   }
 
+  if (FLAGS_enable_data_block_fsync) {
+    // Files/directories created by the directory manager in the fs roots have
+    // been synchronized, so now is a good time to sync the roots themselves.
+    WARN_NOT_OK(env_util::SyncAllParentDirs(env_, created_dirs, created_dirs),
+                "could not sync newly created fs roots");
+  }
+
   LOG(INFO) << "Opened local filesystem: " <<
-    JoinStrings(DataDirManager::GetRootNames(canonicalized_all_fs_roots_), ",")
+      JoinStrings(DataDirManager::GetRootNames(canonicalized_all_fs_roots_), ",")
             << std::endl << SecureDebugString(*metadata_);
+
+  if (!created_dirs.empty()) {
+    LOG(INFO) << "New directories created while opening local filesystem: " <<
+        JoinStrings(created_dirs, ", ");
+  }
+  if (!created_dirs.empty()) {
+    LOG(INFO) << "New files created while opening local filesystem: " <<
+        JoinStrings(created_files, ", ");
+  }
+
+  // Success: do not delete any missing roots created.
+  deleter.cancel();
   return Status::OK();
 }
 
@@ -364,63 +422,32 @@ Status FsManager::CreateInitialFileSystemLayout(boost::optional<string> uuid) {
 
   RETURN_NOT_OK(Init());
 
-  // It's OK if a root already exists as long as there's nothing in it.
-  for (const auto& root : canonicalized_all_fs_roots_) {
-    if (!root.status.ok()) {
-      return Status::IOError("Cannot create FS layout; at least one directory "
-                             "failed to canonicalize", root.path);
-    }
-    if (!env_->FileExists(root.path)) {
-      // We'll create the directory below.
-      continue;
-    }
-    bool is_empty;
-    RETURN_NOT_OK_PREPEND(env_util::IsDirectoryEmpty(env_, root.path, &is_empty),
-                          "Unable to check if FSManager root is empty");
-    if (!is_empty) {
-      return Status::AlreadyPresent(
-          Substitute("FSManager root is not empty. See $0", KuduDocsTroubleshootingUrl()),
-          root.path);
-    }
-  }
-
-  // All roots are either empty or non-existent. Create missing roots and all
-  // subdirectories.
-  //
   // In the event of failure, delete everything we created.
-  vector<string> dirs_to_delete;
-  vector<string> files_to_delete;
+  vector<string> created_dirs;
+  vector<string> created_files;
   auto deleter = MakeScopedCleanup([&]() {
     // Delete files first so that the directories will be empty when deleted.
-    for (const auto& f : files_to_delete) {
+    for (const auto& f : created_files) {
       WARN_NOT_OK(env_->DeleteFile(f), "Could not delete file " + f);
     }
     // Delete directories in reverse order since parent directories will have
     // been added before child directories.
-    for (auto it = dirs_to_delete.rbegin(); it != dirs_to_delete.rend(); it++) {
+    for (auto it = created_dirs.rbegin(); it != created_dirs.rend(); it++) {
       WARN_NOT_OK(env_->DeleteDir(*it), "Could not delete dir " + *it);
     }
   });
 
+  // Create the filesystem roots.
+  //
+  // Files/directories created will NOT be synchronized to disk.
   InstanceMetadataPB metadata;
-  RETURN_NOT_OK(CreateInstanceMetadata(std::move(uuid), &metadata));
-  for (const auto& root : canonicalized_all_fs_roots_) {
-    if (!root.status.ok()) {
-      continue;
-    }
-    string root_name = root.path;
-    bool created;
-    RETURN_NOT_OK_PREPEND(env_util::CreateDirIfMissing(env_, root_name, &created),
-                          "Unable to create FSManager root");
-    if (created) {
-      dirs_to_delete.emplace_back(root_name);
-    }
-    RETURN_NOT_OK_PREPEND(WriteInstanceMetadata(metadata, root_name),
-                          "Unable to write instance metadata");
-    files_to_delete.emplace_back(GetInstanceMetadataPath(root_name));
-  }
+  RETURN_NOT_OK_PREPEND(CreateInstanceMetadata(std::move(uuid), &metadata),
+                        "unable to create instance metadata");
+  RETURN_NOT_OK_PREPEND(FsManager::CreateFileSystemRoots(
+      canonicalized_all_fs_roots_, metadata, &created_dirs, &created_files),
+                        "unable to create file system roots");
 
-  // Initialize ancillary directories.
+  // Create ancillary directories.
   vector<string> ancillary_dirs = { GetWalsRootDir(),
                                     GetTabletMetadataDir(),
                                     GetConsensusMetadataDir() };
@@ -429,17 +456,11 @@ Status FsManager::CreateInitialFileSystemLayout(boost::optional<string> uuid) {
     RETURN_NOT_OK_PREPEND(env_util::CreateDirIfMissing(env_, dir, &created),
                           Substitute("Unable to create directory $0", dir));
     if (created) {
-      dirs_to_delete.emplace_back(dir);
+      created_dirs.emplace_back(dir);
     }
   }
 
-  // Ensure newly created directories are synchronized to disk.
-  if (FLAGS_enable_data_block_fsync) {
-    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.
+  // Create the directory manager.
   //
   // All files/directories created will be synchronized to disk.
   DataDirManagerOptions dm_opts;
@@ -451,11 +472,65 @@ Status FsManager::CreateInitialFileSystemLayout(boost::optional<string> uuid) {
                           "Unable to create directory manager");
   }
 
+  if (FLAGS_enable_data_block_fsync) {
+    // Files/directories created by the directory manager in the fs roots have
+    // been synchronized, so now is a good time to sync the roots themselves.
+    WARN_NOT_OK(env_util::SyncAllParentDirs(env_, created_dirs, created_files),
+                "could not sync newly created fs roots");
+  }
+
   // Success: don't delete any files.
   deleter.cancel();
   return Status::OK();
 }
 
+Status FsManager::CreateFileSystemRoots(
+    CanonicalizedRootsList canonicalized_roots,
+    const InstanceMetadataPB& metadata,
+    vector<string>* created_dirs,
+    vector<string>* created_files) {
+  CHECK(!opts_.read_only);
+
+  // It's OK if a root already exists as long as there's nothing in it.
+  for (const auto& root : canonicalized_roots) {
+    if (!root.status.ok()) {
+      return Status::IOError("cannot create FS layout; at least one directory "
+                             "failed to canonicalize", root.path);
+    }
+    if (!env_->FileExists(root.path)) {
+      // We'll create the directory below.
+      continue;
+    }
+    bool is_empty;
+    RETURN_NOT_OK_PREPEND(env_util::IsDirectoryEmpty(env_, root.path, &is_empty),
+                          "unable to check if FSManager root is empty");
+    if (!is_empty) {
+      return Status::AlreadyPresent(
+          Substitute("FSManager root is not empty. See $0", KuduDocsTroubleshootingUrl()),
+          root.path);
+    }
+  }
+
+  // All roots are either empty or non-existent. Create missing roots and all
+  // subdirectories.
+  for (const auto& root : canonicalized_roots) {
+    if (!root.status.ok()) {
+      continue;
+    }
+    string root_name = root.path;
+    bool created;
+    RETURN_NOT_OK_PREPEND(env_util::CreateDirIfMissing(env_, root_name, &created),
+                          "unable to create FSManager root");
+    if (created) {
+      created_dirs->emplace_back(root_name);
+    }
+    RETURN_NOT_OK_PREPEND(WriteInstanceMetadata(metadata, root_name),
+                          "unable to write instance metadata");
+    created_files->emplace_back(GetInstanceMetadataPath(root_name));
+  }
+  return Status::OK();
+}
+
 Status FsManager::CreateInstanceMetadata(boost::optional<string> uuid,
                                          InstanceMetadataPB* metadata) {
   ObjectIdGenerator oid_generator;

http://git-wip-us.apache.org/repos/asf/kudu/blob/044d5fe6/src/kudu/fs/fs_manager.h
----------------------------------------------------------------------
diff --git a/src/kudu/fs/fs_manager.h b/src/kudu/fs/fs_manager.h
index a603a56..cbb08a9 100644
--- a/src/kudu/fs/fs_manager.h
+++ b/src/kudu/fs/fs_manager.h
@@ -91,6 +91,15 @@ struct FsManagerOpts {
 
   // Whether or not read-write operations should be allowed. Defaults to false.
   bool read_only;
+
+  // Whether or not the contents of 'data_roots' should be considered the new
+  // canonical set. If true, on-disk data structures will be updated
+  // accordingly when the FsManager is opened.
+  //
+  // If true, 'read_only' must be false.
+  //
+  // TODO(KUDU-2202): only supports adding new data directories.
+  bool update_on_disk;
 };
 
 // FsManager provides helpers to read data and metadata files,
@@ -249,6 +258,18 @@ class FsManager {
   // Does not actually perform any on-disk operations.
   void InitBlockManager();
 
+  // Creates filesystem roots from 'canonicalized_roots', writing new on-disk
+  // instances using 'metadata'.
+  //
+  //
+  // All created directories and files will be appended to 'created_dirs' and
+  // 'created_files' respectively. It is the responsibility of the caller to
+  // synchronize the directories containing these newly created file objects.
+  Status CreateFileSystemRoots(CanonicalizedRootsList canonicalized_roots,
+                               const InstanceMetadataPB& metadata,
+                               std::vector<std::string>* created_dirs,
+                               std::vector<std::string>* created_files);
+
   // Create a new InstanceMetadataPB.
   Status CreateInstanceMetadata(boost::optional<std::string> uuid,
                                 InstanceMetadataPB* metadata);

http://git-wip-us.apache.org/repos/asf/kudu/blob/044d5fe6/src/kudu/tools/kudu-tool-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/kudu-tool-test.cc b/src/kudu/tools/kudu-tool-test.cc
index b0b8355..f9f5f22 100644
--- a/src/kudu/tools/kudu-tool-test.cc
+++ b/src/kudu/tools/kudu-tool-test.cc
@@ -126,6 +126,8 @@ namespace tools {
 using cfile::CFileWriter;
 using cfile::StringDataGenerator;
 using cfile::WriterOptions;
+using client::KuduSchema;
+using client::KuduSchemaBuilder;
 using client::sp::shared_ptr;
 using cluster::ExternalMiniCluster;
 using cluster::ExternalMiniClusterOptions;
@@ -402,8 +404,9 @@ TEST_F(ToolTest, TestModeHelp) {
   {
     const vector<string> kFsModeRegexes = {
         "check.*Kudu filesystem for inconsistencies",
+        "dump.*Dump a Kudu filesystem",
         "format.*new Kudu filesystem",
-        "dump.*Dump a Kudu filesystem"
+        "update_dirs.*Updates the set of data directories"
     };
     NO_FATALS(RunTestHelp("fs", kFsModeRegexes));
     NO_FATALS(RunTestHelp("fs not_a_mode", kFsModeRegexes,
@@ -1271,7 +1274,7 @@ void ToolTest::RunLoadgen(int num_tservers,
 
     shared_ptr<client::KuduClient> client;
     ASSERT_OK(cluster_->CreateClient(nullptr, &client));
-    client::KuduSchema client_schema(client::KuduSchemaFromSchema(kSchema));
+    KuduSchema client_schema(client::KuduSchemaFromSchema(kSchema));
     unique_ptr<client::KuduTableCreator> table_creator(
         client->NewTableCreator());
     ASSERT_OK(table_creator->table_name(table_name)
@@ -1921,12 +1924,12 @@ TEST_P(ControlShellToolTest, TestControlShell) {
     }
     shared_ptr<client::KuduClient> client;
     ASSERT_OK(client_builder.Build(&client));
-    client::KuduSchemaBuilder schema_builder;
+    KuduSchemaBuilder schema_builder;
     schema_builder.AddColumn("foo")
               ->Type(client::KuduColumnSchema::INT32)
               ->NotNull()
               ->PrimaryKey();
-    client::KuduSchema schema;
+    KuduSchema schema;
     ASSERT_OK(schema_builder.Build(&schema));
     unique_ptr<client::KuduTableCreator> table_creator(
         client->NewTableCreator());
@@ -2058,5 +2061,86 @@ TEST_P(ControlShellToolTest, TestControlShell) {
   }
 }
 
+static void CreateTableWithFlushedData(const string& table_name,
+                                        InternalMiniCluster* cluster) {
+  // Use a schema with a high number of columns to encourage the creation of
+  // many data blocks.
+  KuduSchemaBuilder schema_builder;
+  schema_builder.AddColumn("key")
+      ->Type(client::KuduColumnSchema::INT32)
+      ->NotNull()
+      ->PrimaryKey();
+  for (int i = 0; i < 50; i++) {
+    schema_builder.AddColumn(Substitute("col$0", i))
+        ->Type(client::KuduColumnSchema::INT32)
+        ->NotNull();
+  }
+  KuduSchema schema;
+  ASSERT_OK(schema_builder.Build(&schema));
+
+  // Create a table and write some data to it.
+  TestWorkload workload(cluster);
+  workload.set_schema(schema);
+  workload.set_table_name(table_name);
+  workload.set_num_replicas(1);
+  workload.Setup();
+  workload.Start();
+  ASSERT_EVENTUALLY([&](){
+    ASSERT_GE(workload.rows_inserted(), 10000);
+  });
+  workload.StopAndJoin();
+
+  // Flush all tablets belonging to this table.
+  for (int i = 0; i < cluster->num_tablet_servers(); i++) {
+    vector<scoped_refptr<TabletReplica>> replicas;
+    cluster->mini_tablet_server(i)->server()->tablet_manager()->GetTabletReplicas(&replicas);
+    for (const auto& r : replicas) {
+      if (r->tablet_metadata()->table_name() == table_name) {
+        ASSERT_OK(r->tablet()->Flush());
+      }
+    }
+  }
+}
+
+TEST_F(ToolTest, TestFsAddDataDirEndToEnd) {
+  if (FLAGS_block_manager == "file") {
+    LOG(INFO) << "Skipping test, only log block manager is supported";
+    return;
+  }
+
+  // Start a cluster whose tserver has multiple data directories.
+  InternalMiniClusterOptions opts;
+  opts.num_data_dirs = 2;
+  NO_FATALS(StartMiniCluster(std::move(opts)));
+
+  // Create a table and flush some test data to it.
+  NO_FATALS(CreateTableWithFlushedData("foo", mini_cluster_.get()));
+
+  // Add a new data directory.
+  MiniTabletServer* mts = mini_cluster_->mini_tablet_server(0);
+  const string& wal_root = mts->options()->fs_opts.wal_root;
+  vector<string> data_roots = mts->options()->fs_opts.data_roots;
+  string to_add = JoinPathSegments(DirName(data_roots.back()), "data-new");
+  data_roots.emplace_back(to_add);
+  mts->Shutdown();
+  NO_FATALS(RunActionStdoutNone(Substitute(
+      "fs update_dirs --fs_wal_dir=$0 --fs_data_dirs=$1",
+      wal_root, JoinStrings(data_roots, ","))));
+
+  // Reconfigure the tserver to use the newly added data directory and restart it.
+  mts->options()->fs_opts.data_roots = data_roots;
+  ASSERT_OK(mts->Start());
+  ASSERT_OK(mts->WaitStarted());
+
+  // Create a second table and flush some data. The flush should write some
+  // data to the newly added data directory.
+  uint64_t disk_space_used_before;
+  ASSERT_OK(env_->GetFileSizeOnDiskRecursively(to_add, &disk_space_used_before));
+  NO_FATALS(CreateTableWithFlushedData("bar", mini_cluster_.get()));
+  uint64_t disk_space_used_after;
+  ASSERT_OK(env_->GetFileSizeOnDiskRecursively(to_add, &disk_space_used_after));
+  ASSERT_GT(disk_space_used_after, disk_space_used_before);
+}
+
 } // namespace tools
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/044d5fe6/src/kudu/tools/tool_action_fs.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/tool_action_fs.cc b/src/kudu/tools/tool_action_fs.cc
index 58da4dc..5584588 100644
--- a/src/kudu/tools/tool_action_fs.cc
+++ b/src/kudu/tools/tool_action_fs.cc
@@ -275,6 +275,14 @@ Status DumpFsTree(const RunnerContext& /*context*/) {
   return Status::OK();
 }
 
+Status Update(const RunnerContext& /*context*/) {
+  Env* env = Env::Default();
+  FsManagerOpts opts;
+  opts.update_on_disk = true;
+  FsManager fs(env, std::move(opts));
+  return fs.Open();
+}
+
 } // anonymous namespace
 
 static unique_ptr<Mode> BuildFsDumpMode() {
@@ -340,9 +348,18 @@ unique_ptr<Mode> BuildFsMode() {
       .AddOptionalParameter("uuid")
       .Build();
 
+  unique_ptr<Action> update =
+      ActionBuilder("update_dirs", &Update)
+      .Description("Updates the set of data directories in an existing Kudu filesystem")
+      .ExtraDescription("Cannot currently be used to remove data directories")
+      .AddOptionalParameter("fs_wal_dir")
+      .AddOptionalParameter("fs_data_dirs")
+      .Build();
+
   return ModeBuilder("fs")
       .Description("Operate on a local Kudu filesystem")
       .AddMode(BuildFsDumpMode())
+      .AddAction(std::move(update))
       .AddAction(std::move(check))
       .AddAction(std::move(format))
       .Build();