You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by ad...@apache.org on 2019/09/18 16:12:28 UTC

[kudu] branch master updated (ff8ee5d -> c55b6b5)

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

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


    from ff8ee5d  [util] Fix the bug of MaybeGCAfterRelease()
     new f0b2149  tools: don't open block manager when dumping UUID
     new c55b6b5  [tserver] include ip:port in the tserver name

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 src/kudu/fs/fs_manager.cc         | 24 ++++++++++++-------
 src/kudu/fs/fs_manager.h          | 50 ++++++++++++++++++++++++++++-----------
 src/kudu/master/master.cc         | 13 +++-------
 src/kudu/master/master.h          |  3 ---
 src/kudu/server/rpc_server.cc     | 10 ++++++--
 src/kudu/tools/kudu-tool-test.cc  |  7 +++---
 src/kudu/tools/tool_action_fs.cc  |  2 +-
 src/kudu/tserver/tablet_server.cc | 24 +++++++++----------
 src/kudu/tserver/tablet_server.h  |  8 ++++++-
 9 files changed, 84 insertions(+), 57 deletions(-)


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

Posted by 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();
 }


[kudu] 02/02: [tserver] include ip:port in the tserver name

Posted by 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 c55b6b5f79eb15943ac9e49ae935acd5fe28f161
Author: helifu <hz...@corp.netease.com>
AuthorDate: Mon Sep 16 14:54:52 2019 +0800

    [tserver] include ip:port in the tserver name
    
    Consolidate implementation of ToString() for Master and TabletServer
    into RpcServer. And include IP:Port info in the TabletServer name at
    the same time.
    
    Change-Id: Ic6b0d8e29b6cac5a900923b881f0d7a74facbd6e
    Reviewed-on: http://gerrit.cloudera.org:8080/14232
    Tested-by: Kudu Jenkins
    Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
 src/kudu/master/master.cc         | 13 +++----------
 src/kudu/master/master.h          |  3 ---
 src/kudu/server/rpc_server.cc     | 10 ++++++++--
 src/kudu/tserver/tablet_server.cc | 24 +++++++++++-------------
 src/kudu/tserver/tablet_server.h  |  8 +++++++-
 5 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/src/kudu/master/master.cc b/src/kudu/master/master.cc
index 4fb8b8f..053b00a 100644
--- a/src/kudu/master/master.cc
+++ b/src/kudu/master/master.cc
@@ -127,13 +127,6 @@ Master::~Master() {
   CHECK_NE(kRunning, state_);
 }
 
-string Master::ToString() const {
-  if (state_ != kRunning) {
-    return "Master (stopped)";
-  }
-  return strings::Substitute("Master@$0", first_rpc_address().ToString());
-}
-
 Status Master::Init() {
   CHECK_EQ(kStopped, state_);
 
@@ -243,8 +236,8 @@ Status Master::WaitUntilCatalogManagerIsLeaderAndReadyForTests(const MonoDelta&
 
 void Master::Shutdown() {
   if (state_ == kRunning) {
-    string name = ToString();
-    LOG(INFO) << name << " shutting down...";
+    const string name = rpc_server_->ToString();
+    LOG(INFO) << "Master@" << name << " shutting down...";
 
     // 1. Stop accepting new RPCs.
     UnregisterAllServices();
@@ -255,7 +248,7 @@ void Master::Shutdown() {
 
     // 3. Shut down generic subsystems.
     KuduServer::Shutdown();
-    LOG(INFO) << name << " shutdown complete.";
+    LOG(INFO) << "Master@" << name << " shutdown complete.";
   }
   state_ = kStopped;
 }
diff --git a/src/kudu/master/master.h b/src/kudu/master/master.h
index 077aabd..0b3c66b 100644
--- a/src/kudu/master/master.h
+++ b/src/kudu/master/master.h
@@ -20,7 +20,6 @@
 #include <atomic>
 #include <cstdint>
 #include <memory>
-#include <string>
 #include <vector>
 
 #include "kudu/common/wire_protocol.pb.h"
@@ -74,8 +73,6 @@ class Master : public kserver::KuduServer {
   Status WaitUntilCatalogManagerIsLeaderAndReadyForTests(const MonoDelta& timeout)
       WARN_UNUSED_RESULT;
 
-  std::string ToString() const;
-
   MasterCertAuthority* cert_authority() { return cert_authority_.get(); }
 
   security::TokenSigner* token_signer() { return token_signer_.get(); }
diff --git a/src/kudu/server/rpc_server.cc b/src/kudu/server/rpc_server.cc
index 1c6eeab..ec5a135 100644
--- a/src/kudu/server/rpc_server.cc
+++ b/src/kudu/server/rpc_server.cc
@@ -30,6 +30,7 @@
 #include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/gutil/macros.h"
 #include "kudu/gutil/ref_counted.h"
+#include "kudu/gutil/strings/join.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/rpc/acceptor_pool.h"
 #include "kudu/rpc/messenger.h"
@@ -104,8 +105,13 @@ RpcServer::~RpcServer() {
 }
 
 string RpcServer::ToString() const {
-  // TODO: include port numbers, etc.
-  return "RpcServer";
+  if (PREDICT_FALSE(UNINITIALIZED == server_state_)) {
+    return options_.rpc_bind_addresses;
+  }
+
+  return JoinMapped(rpc_bind_addresses_,
+                    [] (const Sockaddr& sock_addr) { return sock_addr.ToString(); },
+                    ",");
 }
 
 Status RpcServer::Init(const shared_ptr<Messenger>& messenger) {
diff --git a/src/kudu/tserver/tablet_server.cc b/src/kudu/tserver/tablet_server.cc
index b7a5356..c6513e7 100644
--- a/src/kudu/tserver/tablet_server.cc
+++ b/src/kudu/tserver/tablet_server.cc
@@ -31,6 +31,7 @@
 #include "kudu/gutil/bind_helpers.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/rpc/service_if.h"
+#include "kudu/server/rpc_server.h"
 #include "kudu/tserver/heartbeater.h"
 #include "kudu/tserver/scanners.h"
 #include "kudu/tserver/tablet_copy_service.h"
@@ -52,7 +53,7 @@ namespace tserver {
 
 TabletServer::TabletServer(const TabletServerOptions& opts)
   : KuduServer("TabletServer", opts, "kudu.tabletserver"),
-    initted_(false),
+    state_(kStopped),
     fail_heartbeats_for_tests_(false),
     opts_(opts),
     tablet_manager_(new TSTabletManager(this)),
@@ -64,13 +65,8 @@ TabletServer::~TabletServer() {
   Shutdown();
 }
 
-string TabletServer::ToString() const {
-  // TODO: include port numbers, etc.
-  return "TabletServer";
-}
-
 Status TabletServer::Init() {
-  CHECK(!initted_);
+  CHECK_EQ(kStopped, state_);
 
   cfile::BlockCache::GetSingleton()->StartInstrumentation(metric_entity());
 
@@ -113,7 +109,7 @@ Status TabletServer::Init() {
   RETURN_NOT_OK_PREPEND(scanner_manager_->StartRemovalThread(),
                         "Could not start expired Scanner removal thread");
 
-  initted_ = true;
+  state_ = kInitialized;
   return Status::OK();
 }
 
@@ -122,7 +118,7 @@ Status TabletServer::WaitInited() {
 }
 
 Status TabletServer::Start() {
-  CHECK(initted_);
+  CHECK_EQ(kInitialized, state_);
 
   fs_manager_->SetErrorNotificationCb(ErrorHandlerType::DISK_ERROR,
       Bind(&TSTabletManager::FailTabletsInDataDir, Unretained(tablet_manager_.get())));
@@ -146,13 +142,14 @@ Status TabletServer::Start() {
 
   google::FlushLogFiles(google::INFO); // Flush the startup messages.
 
+  state_ = kRunning;
   return Status::OK();
 }
 
 void TabletServer::Shutdown() {
-  if (initted_) {
-    string name = ToString();
-    LOG(INFO) << name << " shutting down...";
+  if (kInitialized == state_ || kRunning == state_) {
+    const string name = rpc_server_->ToString();
+    LOG(INFO) << "TabletServer@" << name << " shutting down...";
 
     // 1. Stop accepting new RPCs.
     UnregisterAllServices();
@@ -166,8 +163,9 @@ void TabletServer::Shutdown() {
 
     // 3. Shut down generic subsystems.
     KuduServer::Shutdown();
-    LOG(INFO) << name << " shutdown complete.";
+    LOG(INFO) << "TabletServer@" << name << " shutdown complete.";
   }
+  state_ = kStopped;
 }
 
 } // namespace tserver
diff --git a/src/kudu/tserver/tablet_server.h b/src/kudu/tserver/tablet_server.h
index 9af1b2f..343a1de 100644
--- a/src/kudu/tserver/tablet_server.h
+++ b/src/kudu/tserver/tablet_server.h
@@ -85,7 +85,13 @@ class TabletServer : public kserver::KuduServer {
  private:
   friend class TabletServerTestBase;
 
-  bool initted_;
+  enum TabletServerState {
+    kStopped,
+    kInitialized,
+    kRunning
+  };
+
+  TabletServerState state_;
 
   // If true, all heartbeats will be seen as failed.
   Atomic32 fail_heartbeats_for_tests_;