You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by gr...@apache.org on 2018/08/21 03:32:19 UTC
[1/2] kudu git commit: KUDU-2531: (part 1) Ignore invalid tablet
metadata files
Repository: kudu
Updated Branches:
refs/heads/master 9199d1646 -> 368990140
KUDU-2531: (part 1) Ignore invalid tablet metadata files
Changes the behavior of FsManager::IsValidTabletId
to return false for all files that are named with invalid
tabet ids. This results in all files that are not valid
table ids being ignored, instead of just tmp files.
Change-Id: If9f0e6dd8925519aa3ed3e735f2987743f2ef1d1
Reviewed-on: http://gerrit.cloudera.org:8080/11259
Tested-by: Kudu Jenkins
Reviewed-by: Grant Henke <gr...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/6d7d8f55
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/6d7d8f55
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/6d7d8f55
Branch: refs/heads/master
Commit: 6d7d8f555e85bd2809fe9dea0630e2820ac2a892
Parents: 9199d16
Author: Grant Henke <gr...@apache.org>
Authored: Fri Aug 17 13:54:03 2018 -0500
Committer: Grant Henke <gr...@apache.org>
Committed: Tue Aug 21 03:31:53 2018 +0000
----------------------------------------------------------------------
src/kudu/fs/fs_manager-test.cc | 10 +++++++++-
src/kudu/fs/fs_manager.cc | 33 ++++++++++++++++++++-------------
src/kudu/fs/fs_manager.h | 8 +++++++-
3 files changed, 36 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/6d7d8f55/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 194ecef..df84259 100644
--- a/src/kudu/fs/fs_manager-test.cc
+++ b/src/kudu/fs/fs_manager-test.cc
@@ -181,9 +181,17 @@ TEST_F(FsManagerTestBase, TestListTablets) {
ASSERT_OK(env_->NewWritableFile(
JoinPathSegments(path, "foo.kudutmp.abc123"), &writer));
ASSERT_OK(env_->NewWritableFile(
+ JoinPathSegments(path, "foo.bak"), &writer));
+ ASSERT_OK(env_->NewWritableFile(
+ JoinPathSegments(path, "foo.bak.abc123"), &writer));
+ ASSERT_OK(env_->NewWritableFile(
JoinPathSegments(path, ".hidden"), &writer));
+ // An uncanonicalized id.
+ ASSERT_OK(env_->NewWritableFile(
+ JoinPathSegments(path, "6ba7b810-9dad-11d1-80b4-00c04fd430c8"), &writer));
+ // 1 valid tablet id.
ASSERT_OK(env_->NewWritableFile(
- JoinPathSegments(path, "a_tablet_sort_of"), &writer));
+ JoinPathSegments(path, "922ff7ed14c14dbca4ee16331dfda42a"), &writer));
ASSERT_OK(fs_manager()->ListTabletIds(&tablet_ids));
ASSERT_EQ(1, tablet_ids.size()) << tablet_ids;
http://git-wip-us.apache.org/repos/asf/kudu/blob/6d7d8f55/src/kudu/fs/fs_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/fs_manager.cc b/src/kudu/fs/fs_manager.cc
index 8c08f34..93fd7f3 100644
--- a/src/kudu/fs/fs_manager.cc
+++ b/src/kudu/fs/fs_manager.cc
@@ -57,6 +57,7 @@
#include "kudu/util/path_util.h"
#include "kudu/util/pb_util.h"
#include "kudu/util/scoped_cleanup.h"
+#include "kudu/util/slice.h"
#include "kudu/util/stopwatch.h"
DEFINE_bool(enable_data_block_fsync, true,
@@ -569,13 +570,12 @@ Status FsManager::CreateFileSystemRoots(
Status FsManager::CreateInstanceMetadata(boost::optional<string> uuid,
InstanceMetadataPB* metadata) {
- ObjectIdGenerator oid_generator;
if (uuid) {
string canonicalized_uuid;
- RETURN_NOT_OK(oid_generator.Canonicalize(uuid.get(), &canonicalized_uuid));
+ RETURN_NOT_OK(oid_generator_.Canonicalize(uuid.get(), &canonicalized_uuid));
metadata->set_uuid(canonicalized_uuid);
} else {
- metadata->set_uuid(oid_generator.Next());
+ metadata->set_uuid(oid_generator_.Next());
}
string time_str;
@@ -621,24 +621,31 @@ string FsManager::GetTabletMetadataPath(const string& tablet_id) const {
return JoinPathSegments(GetTabletMetadataDir(), tablet_id);
}
-namespace {
-// Return true if 'fname' is a valid tablet ID.
-bool IsValidTabletId(const string& fname) {
- if (fname.find(kTmpInfix) != string::npos ||
- fname.find(kOldTmpInfix) != string::npos) {
- LOG(WARNING) << "Ignoring tmp file in tablet metadata dir: " << fname;
+bool FsManager::IsValidTabletId(const string& fname) {
+ // Prevent warning logs for hidden files or ./..
+ if (HasPrefixString(fname, ".")) {
+ VLOG(1) << "Ignoring hidden file in tablet metadata dir: " << fname;
return false;
}
- if (HasPrefixString(fname, ".")) {
- // Hidden file or ./..
- VLOG(1) << "Ignoring hidden file in tablet metadata dir: " << fname;
+ string canonicalized_uuid;
+ Status s = oid_generator_.Canonicalize(fname, &canonicalized_uuid);
+
+ if (!s.ok()) {
+ LOG(WARNING) << "Ignoring file in tablet metadata dir: " << fname << ": " <<
+ s.message().ToString();
+ return false;
+ }
+
+ if (fname != canonicalized_uuid) {
+ LOG(WARNING) << "Ignoring file in tablet metadata dir: " << fname << ": " <<
+ Substitute("canonicalized uuid $0 does not match file name",
+ canonicalized_uuid);
return false;
}
return true;
}
-} // anonymous namespace
Status FsManager::ListTabletIds(vector<string>* tablet_ids) {
string dir = GetTabletMetadataDir();
http://git-wip-us.apache.org/repos/asf/kudu/blob/6d7d8f55/src/kudu/fs/fs_manager.h
----------------------------------------------------------------------
diff --git a/src/kudu/fs/fs_manager.h b/src/kudu/fs/fs_manager.h
index 9a29e57..c71cb99 100644
--- a/src/kudu/fs/fs_manager.h
+++ b/src/kudu/fs/fs_manager.h
@@ -34,6 +34,7 @@
#include "kudu/gutil/ref_counted.h"
#include "kudu/util/env.h"
#include "kudu/util/metrics.h"
+#include "kudu/util/oid_generator.h"
#include "kudu/util/path_util.h"
#include "kudu/util/status.h"
@@ -50,9 +51,9 @@ namespace fs {
class BlockManager;
class ReadableBlock;
class WritableBlock;
+struct CreateBlockOptions;
struct FsReport;
-struct CreateBlockOptions;
} // namespace fs
namespace itest {
@@ -314,6 +315,9 @@ class FsManager {
// configured umask, and tightens them as necessary if they do not.
void CheckAndFixPermissions();
+ // Returns true if 'fname' is a valid tablet ID.
+ bool IsValidTabletId(const std::string& fname);
+
static const char *kDataDirName;
static const char *kTabletMetadataDirName;
static const char *kWalDirName;
@@ -345,6 +349,8 @@ class FsManager {
std::unique_ptr<fs::DataDirManager> dd_manager_;
std::unique_ptr<fs::BlockManager> block_manager_;
+ ObjectIdGenerator oid_generator_;
+
bool initted_;
DISALLOW_COPY_AND_ASSIGN(FsManager);
[2/2] kudu git commit: KUDU-2531: (part 2) Add -nobackup flag to pbc
edit tool
Posted by gr...@apache.org.
KUDU-2531: (part 2) Add -nobackup flag to pbc edit tool
Adds a -nobackup flag to the pbc edit tool to
simplify usage when the backup file will
not be used and the user doesn’t want to worry about
cleanup.
Change-Id: If544f7682a30933077db0824452639a68507ba40
Reviewed-on: http://gerrit.cloudera.org:8080/11260
Tested-by: Kudu Jenkins
Reviewed-by: Grant Henke <gr...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/36899014
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/36899014
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/36899014
Branch: refs/heads/master
Commit: 36899014032ea05d1cd6177df0d9dd73f8d91b1e
Parents: 6d7d8f5
Author: Grant Henke <gr...@apache.org>
Authored: Fri Aug 17 15:13:22 2018 -0500
Committer: Grant Henke <gr...@apache.org>
Committed: Tue Aug 21 03:31:55 2018 +0000
----------------------------------------------------------------------
src/kudu/tools/kudu-tool-test.cc | 38 +++++++++++++++++++++++++++++++---
src/kudu/tools/tool_action_pbc.cc | 24 +++++++++++++--------
2 files changed, 50 insertions(+), 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/36899014/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 0027872..4a0be3a 100644
--- a/src/kudu/tools/kudu-tool-test.cc
+++ b/src/kudu/tools/kudu-tool-test.cc
@@ -361,6 +361,19 @@ class ToolTest : public KuduTest {
stdout, Substitute("Total orphaned blocks: $0", expected_num_orphaned));
}
+ Status HasAtLeastOneBackupFile(const string& dir, bool* found) {
+ vector<string> children;
+ RETURN_NOT_OK(env_->GetChildren(dir, &children));
+ *found = false;
+ for (const auto& child : children) {
+ if (child.find(".bak") != string::npos) {
+ *found = true;
+ break;
+ }
+ }
+ return Status::OK();
+ }
+
protected:
void RunLoadgen(int num_tservers = 1,
const vector<string>& tool_args = {},
@@ -835,21 +848,23 @@ TEST_F(ToolTest, TestPbcTools) {
}
// Utility to set the editor up based on the given shell command.
- auto DoEdit = [&](const string& editor_shell, string* stdout, string* stderr = nullptr) {
+ auto DoEdit = [&](const string& editor_shell, string* stdout, string* stderr = nullptr,
+ const string& extra_flags = "") {
const string editor_path = GetTestPath("editor");
CHECK_OK(WriteStringToFile(Env::Default(),
StrCat("#!/usr/bin/env bash\n", editor_shell),
editor_path));
chmod(editor_path.c_str(), 0755);
setenv("EDITOR", editor_path.c_str(), /* overwrite */1);
- return RunTool(Substitute("pbc edit $0/instance", kTestDir),
+ return RunTool(Substitute("pbc edit $0 $1/instance", extra_flags, kTestDir),
stdout, stderr, nullptr, nullptr);
};
// Test 'edit' by setting up EDITOR to be a sed script which performs a substitution.
{
string stdout;
- ASSERT_OK(DoEdit("exec sed -i -e s/Formatted/Edited/ \"$@\"\n", &stdout));
+ ASSERT_OK(DoEdit("exec sed -i -e s/Formatted/Edited/ \"$@\"\n", &stdout, nullptr,
+ "--nobackup"));
ASSERT_EQ("", stdout);
// Dump to make sure the edit took place.
@@ -857,6 +872,23 @@ TEST_F(ToolTest, TestPbcTools) {
"pbc dump $0/instance --oneline", kTestDir), &stdout));
ASSERT_STR_MATCHES(stdout, Substitute(
"^0\tuuid: \"$0\" format_stamp: \"Edited at .*\"$$", uuid));
+
+ // Make sure no backup file was written.
+ bool found_backup;
+ ASSERT_OK(HasAtLeastOneBackupFile(kTestDir, &found_backup));
+ ASSERT_FALSE(found_backup);
+ }
+
+ // Test 'edit' with a backup.
+ {
+ string stdout;
+ ASSERT_OK(DoEdit("exec sed -i -e s/Formatted/Edited/ \"$@\"\n", &stdout));
+ ASSERT_EQ("", stdout);
+
+ // Make sure a backup file was written.
+ bool found_backup;
+ ASSERT_OK(HasAtLeastOneBackupFile(kTestDir, &found_backup));
+ ASSERT_TRUE(found_backup);
}
// Test 'edit' with an unsuccessful edit.
http://git-wip-us.apache.org/repos/asf/kudu/blob/36899014/src/kudu/tools/tool_action_pbc.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/tool_action_pbc.cc b/src/kudu/tools/tool_action_pbc.cc
index ee3837d..cc0e3b6 100644
--- a/src/kudu/tools/tool_action_pbc.cc
+++ b/src/kudu/tools/tool_action_pbc.cc
@@ -52,15 +52,17 @@ using std::string;
using std::unique_ptr;
using std::vector;
-DEFINE_bool(oneline, false, "print each protobuf on a single line");
+DEFINE_bool(oneline, false, "Print each protobuf on a single line");
TAG_FLAG(oneline, stable);
-DEFINE_bool(json, false, "print protobufs in JSON format");
+DEFINE_bool(json, false, "Print protobufs in JSON format");
TAG_FLAG(json, stable);
-DEFINE_bool(debug, false, "print extra debugging information about each protobuf");
+DEFINE_bool(debug, false, "Print extra debugging information about each protobuf");
TAG_FLAG(debug, stable);
+DEFINE_bool(backup, true, "Write a backup file");
+
namespace kudu {
using pb_util::ReadablePBContainerFile;
@@ -209,12 +211,15 @@ Status EditFile(const RunnerContext& context) {
RETURN_NOT_OK_PREPEND(pb_writer.Sync(), "failed to sync output");
RETURN_NOT_OK_PREPEND(pb_writer.Close(), "failed to close output");
}
- // We successfully wrote the new file. Move the old file to a backup location,
- // and move the new one to the final location.
- string backup_path = Substitute("$0.bak.$1", path, GetCurrentTimeMicros());
- RETURN_NOT_OK_PREPEND(env->RenameFile(path, backup_path),
- "couldn't back up original file");
- LOG(INFO) << "Moved original file to " << backup_path;
+ // We successfully wrote the new file.
+ if (FLAGS_backup) {
+ // Move the old file to a backup location.
+ string backup_path = Substitute("$0.bak.$1", path, GetCurrentTimeMicros());
+ RETURN_NOT_OK_PREPEND(env->RenameFile(path, backup_path),
+ "couldn't back up original file");
+ LOG(INFO) << "Moved original file to " << backup_path;
+ }
+ // Move the new file to the final location.
RETURN_NOT_OK_PREPEND(env->RenameFile(tmp_out_path, path),
"couldn't move new file into place");
delete_tmp_output.cancel();
@@ -238,6 +243,7 @@ unique_ptr<Mode> BuildPbcMode() {
unique_ptr<Action> edit =
ActionBuilder("edit", &EditFile)
.Description("Edit a PBC (protobuf container) file")
+ .AddOptionalParameter("backup")
.AddRequiredParameter({kPathArg, "path to PBC file"})
.Build();