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 <[email protected]> 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 <[email protected]> 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(); }
