[kudu] 01/02: tools: don't open block manager when dumping UUID

[ad...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

adar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit f0b21498fb20a08724abcf8ecae62c2a7ed57507
Author: Andrew <an...@gmail.com>
AuthorDate: Mon Sep 16 22:29:30 2019 -0700

    tools: don't open block manager when dumping UUID
    
    The block manager can be expensive to open, even when opening it in
    read-only mode. If all a user wants is the server's UUID, all we really
    need is to read the instance metadata files.
    
    This patch updates the `kudu fs dump uuid` tool to do just that.
    
    This also moves some words around in the header to try clarifying what
    functions are important for startup.
    
    Change-Id: I0acf3b08fc13c169a52d4cfe847efd62bd30cc0a
    Reviewed-on: http://gerrit.cloudera.org:8080/14243
    Reviewed-by: Adar Dembo <ad...@cloudera.com>
    Tested-by: Kudu Jenkins
---
 src/kudu/fs/fs_manager.cc        | 24 +++++++++++--------
 src/kudu/fs/fs_manager.h         | 50 +++++++++++++++++++++++++++++-----------
 src/kudu/tools/kudu-tool-test.cc |  7 +++---
 src/kudu/tools/tool_action_fs.cc |  2 +-
 4 files changed, 55 insertions(+), 28 deletions(-)

diff --git a/src/kudu/fs/fs_manager.cc b/src/kudu/fs/fs_manager.cc
index b3b2503..605fd72 100644
--- a/src/kudu/fs/fs_manager.cc
+++ b/src/kudu/fs/fs_manager.cc
@@ -38,7 +38,6 @@
 #include "kudu/fs/log_block_manager.h"
 #include "kudu/gutil/bind.h"
 #include "kudu/gutil/bind_helpers.h"
-#include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/gutil/map-util.h"
 #include "kudu/gutil/port.h"
 #include "kudu/gutil/stringprintf.h"
@@ -312,24 +311,21 @@ void FsManager::InitBlockManager() {
   }
 }
 
-Status FsManager::Open(FsReport* report) {
+Status FsManager::PartialOpen(CanonicalizedRootsList* missing_roots) {
   RETURN_NOT_OK(Init());
 
-  // Load and verify the instance metadata files.
-  //
-  // 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;
     }
-    gscoped_ptr<InstanceMetadataPB> pb(new InstanceMetadataPB);
+    unique_ptr<InstanceMetadataPB> pb(new InstanceMetadataPB);
     Status s = pb_util::ReadPBContainerFromPath(env_, GetInstanceMetadataPath(root.path),
                                                 pb.get());
     if (PREDICT_FALSE(!s.ok())) {
       if (s.IsNotFound()) {
-        missing_roots.emplace_back(root);
+        if (missing_roots) {
+          missing_roots->emplace_back(root);
+        }
         continue;
       }
       if (s.IsDiskFailure()) {
@@ -352,6 +348,16 @@ Status FsManager::Open(FsReport* report) {
   if (!metadata_) {
     return Status::NotFound("could not find a healthy instance file");
   }
+  return Status::OK();
+}
+
+Status FsManager::Open(FsReport* report) {
+  // Load and verify the instance metadata files.
+  //
+  // Done first to minimize side effects in the case that the configured roots
+  // are not yet initialized on disk.
+  CanonicalizedRootsList missing_roots;
+  RETURN_NOT_OK(PartialOpen(&missing_roots));
 
   // Ensure all of the ancillary directories exist.
   vector<string> ancillary_dirs = { GetWalsRootDir(),
diff --git a/src/kudu/fs/fs_manager.h b/src/kudu/fs/fs_manager.h
index c71cb99..27eb0a0 100644
--- a/src/kudu/fs/fs_manager.h
+++ b/src/kudu/fs/fs_manager.h
@@ -136,7 +136,21 @@ class FsManager {
   FsManager(Env* env, FsManagerOpts opts);
   ~FsManager();
 
-  // Initialize and load the basic filesystem metadata, checking it for
+  // ==========================================================================
+  //  Initialization
+  // ==========================================================================
+
+  // Initializes and loads the instance metadata files, and verifies that they
+  // are all matching, returning any root paths that do not have metadata
+  // files. Sets 'metadata_' on success, and returns NotFound if none of the
+  // metadata files could be read. This must be called before calling uuid().
+  //
+  // This only partially initialize the FsManager to expose the file
+  // system's UUID. To do anything more than that, call Open() or
+  // CreateInitialFileSystemLayout().
+  Status PartialOpen(CanonicalizedRootsList* missing_roots = nullptr);
+
+  // Initializes and loads the basic filesystem metadata, checking it for
   // inconsistencies. If found, and if the FsManager was not constructed in
   // read-only mode, an attempt will be made to repair them.
   //
@@ -150,6 +164,17 @@ class FsManager {
   // on-disk and in-memory structures.
   Status Open(fs::FsReport* report = nullptr);
 
+  // Create the initial filesystem layout. If 'uuid' is provided, uses it as
+  // uuid of the filesystem. Otherwise generates one at random.
+  //
+  // Returns an error if the file system is already initialized.
+  Status CreateInitialFileSystemLayout(
+      boost::optional<std::string> uuid = boost::none);
+
+  // ==========================================================================
+  //  Error handling helpers
+  // ==========================================================================
+
   // Registers an error-handling callback with the FsErrorManager.
   //
   // If a disk failure is detected, this callback will be invoked with the
@@ -162,19 +187,6 @@ class FsManager {
   // this are idempotent and are safe even if a callback has not been set.
   void UnsetErrorNotificationCb(fs::ErrorHandlerType e);
 
-  // Create the initial filesystem layout. If 'uuid' is provided, uses it as
-  // uuid of the filesystem. Otherwise generates one at random.
-  //
-  // Returns an error if the file system is already initialized.
-  Status CreateInitialFileSystemLayout(
-      boost::optional<std::string> uuid = boost::none);
-
-  void DumpFileSystemTree(std::ostream& out);
-
-  // Return the UUID persisted in the local filesystem. If Open()
-  // has not been called, this will crash.
-  const std::string& uuid() const;
-
   // ==========================================================================
   //  Data read/write interfaces
   // ==========================================================================
@@ -240,6 +252,10 @@ class FsManager {
     return opts_.read_only;
   }
 
+  // Return the UUID persisted in the local filesystem. If PartialOpen() or
+  // Open() have not been called, this will crash.
+  const std::string& uuid() const;
+
   // ==========================================================================
   //  file-system helpers
   // ==========================================================================
@@ -259,6 +275,9 @@ class FsManager {
     return block_manager_.get();
   }
 
+  // Prints the file system trees under the file system roots.
+  void DumpFileSystemTree(std::ostream& out);
+
  private:
   FRIEND_TEST(FsManagerTestBase, TestDuplicatePaths);
   FRIEND_TEST(FsManagerTestBase, TestMetadataDirInWALRoot);
@@ -300,6 +319,9 @@ class FsManager {
   // ==========================================================================
   //  file-system helpers
   // ==========================================================================
+
+  // Prints the file system tree for the objects in 'objects' under the given
+  // 'path'. Prints lines with the given 'prefix'.
   void DumpFileSystemTree(std::ostream& out,
                           const std::string& prefix,
                           const std::string& path,
diff --git a/src/kudu/tools/kudu-tool-test.cc b/src/kudu/tools/kudu-tool-test.cc
index 1d59fef..f44d2ab 100644
--- a/src/kudu/tools/kudu-tool-test.cc
+++ b/src/kudu/tools/kudu-tool-test.cc
@@ -4933,7 +4933,7 @@ TEST_F(ToolTest, TestFsAddRemoveDataDirEndToEnd) {
   ASSERT_OK(mts->WaitStarted());
 }
 
-TEST_F(ToolTest, TestDumpFSWithNonDefaultMetadataDir) {
+TEST_F(ToolTest, TestCheckFSWithNonDefaultMetadataDir) {
   const string kTestDir = GetTestPath("test");
   ASSERT_OK(env_->CreateDir(kTestDir));
   string uuid;
@@ -4950,7 +4950,7 @@ TEST_F(ToolTest, TestDumpFSWithNonDefaultMetadataDir) {
   // enough arguments to open the FsManager, or else FS tools will not work.
   // The tool will fail in its own process. Catch its output.
   string stderr;
-  Status s = RunTool(Substitute("fs dump uuid --fs_wal_dir=$0", opts.wal_root),
+  Status s = RunTool(Substitute("fs check --fs_wal_dir=$0", opts.wal_root),
                     nullptr, &stderr, {}, {});
   ASSERT_TRUE(s.IsRuntimeError());
   ASSERT_STR_CONTAINS(s.ToString(), "process exited with non-zero status");
@@ -4960,10 +4960,9 @@ TEST_F(ToolTest, TestDumpFSWithNonDefaultMetadataDir) {
   // Providing the necessary arguments, the tool should work.
   string stdout;
   NO_FATALS(RunActionStdoutString(Substitute(
-      "fs dump uuid --fs_wal_dir=$0 --fs_metadata_dir=$1",
+      "fs check --fs_wal_dir=$0 --fs_metadata_dir=$1",
       opts.wal_root, opts.metadata_root), &stdout));
   SCOPED_TRACE(stdout);
-  ASSERT_EQ(uuid, stdout);
 }
 
 TEST_F(ToolTest, TestReplaceTablet) {
diff --git a/src/kudu/tools/tool_action_fs.cc b/src/kudu/tools/tool_action_fs.cc
index 6abf2de..9dc7e8f 100644
--- a/src/kudu/tools/tool_action_fs.cc
+++ b/src/kudu/tools/tool_action_fs.cc
@@ -233,7 +233,7 @@ Status DumpUuid(const RunnerContext& /*context*/) {
   FsManagerOpts fs_opts;
   fs_opts.read_only = true;
   FsManager fs_manager(Env::Default(), std::move(fs_opts));
-  RETURN_NOT_OK(fs_manager.Open());
+  RETURN_NOT_OK(fs_manager.PartialOpen());
   cout << fs_manager.uuid() << endl;
   return Status::OK();
 }