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 <granthe...@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 <granthe...@apache.org> Authored: Fri Aug 17 15:13:22 2018 -0500 Committer: Grant Henke <granthe...@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();