You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by la...@apache.org on 2021/09/01 07:54:23 UTC
[kudu] branch master updated: [tool] Support to dump and edit pbc
in JSON pretty format
This is an automated email from the ASF dual-hosted git repository.
laiyingchun pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new 4d50656 [tool] Support to dump and edit pbc in JSON pretty format
4d50656 is described below
commit 4d50656f867a6e68724e232609e4ff7d538af6eb
Author: Yingchun Lai <40...@qq.com>
AuthorDate: Sun Aug 22 22:18:49 2021 +0800
[tool] Support to dump and edit pbc in JSON pretty format
If using 'kudu pbc edit' tool to edit a large protobuf container file
which contains a single object, the editing content is in JSON compact
format which is hard for human reading.
This patch enhance 'pbc edit' tool to dump PBC content in JSON pretty
format by adding --json_pretty flag, and also for 'pbc dump' tool.
Change-Id: I6d404e6cd487a644fa848513670c4945bf789578
Reviewed-on: http://gerrit.cloudera.org:8080/17808
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
src/kudu/tools/kudu-tool-test.cc | 297 ++++++++++++++++++++++++++++++++++++--
src/kudu/tools/tool_action_pbc.cc | 108 +++++++++-----
src/kudu/util/pb_util.cc | 8 +-
src/kudu/util/pb_util.h | 6 +-
4 files changed, 370 insertions(+), 49 deletions(-)
diff --git a/src/kudu/tools/kudu-tool-test.cc b/src/kudu/tools/kudu-tool-test.cc
index acf978e..090ff11 100644
--- a/src/kudu/tools/kudu-tool-test.cc
+++ b/src/kudu/tools/kudu-tool-test.cc
@@ -70,6 +70,7 @@
#include "kudu/fs/block_manager.h"
#include "kudu/fs/fs_manager.h"
#include "kudu/fs/fs_report.h"
+#include "kudu/fs/log_block_manager.h"
#include "kudu/gutil/map-util.h"
#include "kudu/gutil/port.h"
#include "kudu/gutil/ref_counted.h"
@@ -178,6 +179,7 @@ using kudu::consensus::ReplicateMsg;
using kudu::consensus::ReplicateRefPtr;
using kudu::fs::BlockDeletionTransaction;
using kudu::fs::FsReport;
+using kudu::fs::LogBlockManager;
using kudu::fs::WritableBlock;
using kudu::hms::HmsCatalog;
using kudu::hms::HmsClient;
@@ -1543,7 +1545,7 @@ TEST_F(ToolTest, TestPbcTools) {
{
string stdout;
NO_FATALS(RunActionStdoutString(Substitute(
- "pbc dump $0/instance --oneline", kTestDir), &stdout));
+ "pbc dump $0 --oneline", instance_path), &stdout));
SCOPED_TRACE(stdout);
ASSERT_STR_MATCHES(stdout, Substitute(
"^0\tuuid: \"$0\" format_stamp: \"Formatted at .*\"$$", uuid));
@@ -1557,10 +1559,27 @@ TEST_F(ToolTest, TestPbcTools) {
string stdout;
NO_FATALS(RunActionStdoutString(Substitute(
- "pbc dump $0/instance --json", kTestDir), &stdout));
+ "pbc dump $0 --json", instance_path), &stdout));
SCOPED_TRACE(stdout);
ASSERT_STR_MATCHES(stdout, Substitute(
- "^\\{\"uuid\":\"$0\",\"formatStamp\":\"Formatted at .*\"\\}$$", uuid_b64));
+ R"(^\{"uuid":"$0","formatStamp":"Formatted at .*"\}$$)", uuid_b64));
+ }
+ // Test dump --json_pretty
+ {
+ // Since the UUID is listed as 'bytes' rather than 'string' in the PB, it dumps
+ // base64-encoded.
+ string uuid_b64;
+ Base64Encode(uuid, &uuid_b64);
+
+ vector<string> stdout;
+ NO_FATALS(RunActionStdoutLines(Substitute(
+ "pbc dump $0 --json_pretty", instance_path), &stdout));
+ SCOPED_TRACE(stdout);
+ ASSERT_EQ(4, stdout.size());
+ ASSERT_EQ("{", stdout[0]);
+ ASSERT_EQ(Substitute(R"( "uuid": "$0",)", uuid_b64), stdout[1]);
+ ASSERT_STR_MATCHES(stdout[2], R"( "formatStamp": "Formatted at .*")");
+ ASSERT_EQ("}", stdout[3]);
}
// Utility to set the editor up based on the given shell command.
@@ -1572,7 +1591,7 @@ TEST_F(ToolTest, TestPbcTools) {
editor_path));
chmod(editor_path.c_str(), 0755);
setenv("EDITOR", editor_path.c_str(), /* overwrite */1);
- return RunTool(Substitute("pbc edit $0 $1/instance", extra_flags, kTestDir),
+ return RunTool(Substitute("pbc edit $0 $1", extra_flags, instance_path),
stdout, stderr, nullptr, nullptr);
};
@@ -1585,7 +1604,7 @@ TEST_F(ToolTest, TestPbcTools) {
// Dump to make sure the edit took place.
NO_FATALS(RunActionStdoutString(Substitute(
- "pbc dump $0/instance --oneline", kTestDir), &stdout));
+ "pbc dump $0 --oneline", instance_path), &stdout));
ASSERT_STR_MATCHES(stdout, Substitute(
"^0\tuuid: \"$0\" format_stamp: \"Edited at .*\"$$", uuid));
@@ -1595,6 +1614,25 @@ TEST_F(ToolTest, TestPbcTools) {
ASSERT_FALSE(found_backup);
}
+ // Test 'edit' by setting --json_pretty flag.
+ {
+ string stdout;
+ ASSERT_OK(DoEdit("exec sed -i -e s/Edited/Formatted/ \"$@\"\n", &stdout, nullptr,
+ "--nobackup --json_pretty"));
+ ASSERT_EQ("", stdout);
+
+ // Dump to make sure the edit took place.
+ NO_FATALS(RunActionStdoutString(Substitute(
+ "pbc dump $0 --oneline", instance_path), &stdout));
+ ASSERT_STR_MATCHES(stdout, Substitute(
+ "^0\tuuid: \"$0\" format_stamp: \"Formatted 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;
@@ -1624,7 +1662,7 @@ TEST_F(ToolTest, TestPbcTools) {
Status s = DoEdit("echo {} > $@\n", &stdout, &stderr);
ASSERT_EQ("", stdout);
ASSERT_STR_MATCHES(stderr,
- "Invalid argument: Unable to parse JSON line: \\{\\}: "
+ "Invalid argument: Unable to parse JSON text: \\{\\}: "
": missing field .*");
// NOTE: the above extra ':' is due to an apparent bug in protobuf.
}
@@ -1634,23 +1672,258 @@ TEST_F(ToolTest, TestPbcTools) {
string stdout, stderr;
Status s = DoEdit("echo not-a-json-string > $@\n", &stdout, &stderr);
ASSERT_EQ("", stdout);
- ASSERT_STR_CONTAINS(
- stderr,
- "Invalid argument: Unable to parse JSON line: not-a-json-string: Unexpected token.\n"
- "not-a-json-string\n"
- "^");
+ ASSERT_STR_CONTAINS(stderr, "Corruption: JSON text is corrupt: Invalid value.");
}
// The file should be unchanged by the unsuccessful edits above.
{
string stdout;
NO_FATALS(RunActionStdoutString(Substitute(
- "pbc dump $0/instance --oneline", kTestDir), &stdout));
+ "pbc dump $0 --oneline", instance_path), &stdout));
ASSERT_STR_MATCHES(stdout, Substitute(
"^0\tuuid: \"$0\" format_stamp: \"Edited at .*\"$$", uuid));
}
}
+TEST_F(ToolTest, TestPbcToolsOnMultipleBlocks) {
+ const int kNumCFileBlocks = 1024;
+ const int kNumEntries = 1;
+ const string kTestDir = GetTestPath("test");
+ const string kDataDir = JoinPathSegments(kTestDir, "data");
+
+ // Generate a block container metadata file.
+ string metadata_path;
+ {
+ // Open FsManager to write file later.
+ FsManager fs(env_, FsManagerOpts(kTestDir));
+ ASSERT_OK(fs.CreateInitialFileSystemLayout());
+ ASSERT_OK(fs.Open());
+
+ // Write multiple CFile blocks to file.
+ for (int i = 0; i < kNumCFileBlocks; ++i) {
+ unique_ptr<WritableBlock> block;
+ ASSERT_OK(fs.CreateNewBlock({}, &block));
+ StringDataGenerator<false> generator("hello %04d");
+ WriterOptions opts;
+ opts.write_posidx = true;
+ CFileWriter writer(
+ opts, GetTypeInfo(generator.kDataType), generator.has_nulls(), std::move(block));
+ ASSERT_OK(writer.Start());
+ generator.Build(kNumEntries);
+ ASSERT_OK_FAST(writer.AppendEntries(generator.values(), kNumEntries));
+ ASSERT_OK(writer.Finish());
+ }
+
+ // Find the CFile metadata file.
+ vector<string> children;
+ ASSERT_OK(env_->GetChildren(kDataDir, &children));
+ vector<string> metadata_files;
+ for (const string& child : children) {
+ if (HasSuffixString(child, LogBlockManager::kContainerMetadataFileSuffix)) {
+ metadata_files.push_back(JoinPathSegments(kDataDir, child));
+ }
+ }
+ ASSERT_EQ(1, metadata_files.size());
+ metadata_path = metadata_files[0];
+ }
+
+ // Test default dump
+ {
+ vector<string> stdout;
+ NO_FATALS(RunActionStdoutLines(Substitute(
+ "pbc dump $0", metadata_path), &stdout));
+ ASSERT_EQ(kNumCFileBlocks * 10 - 1, stdout.size());
+ ASSERT_EQ("Message 0", stdout[0]);
+ ASSERT_EQ("-------", stdout[1]);
+ ASSERT_EQ("block_id {", stdout[2]);
+ ASSERT_STR_MATCHES(stdout[3], "^ id: [0-9]+$");
+ ASSERT_EQ("}", stdout[4]);
+ ASSERT_EQ("op_type: CREATE", stdout[5]);
+ ASSERT_STR_MATCHES(stdout[6], "^timestamp_us: [0-9]+$");
+ ASSERT_EQ("offset: 0", stdout[7]);
+ ASSERT_EQ("length: 153", stdout[8]);
+ ASSERT_EQ("", stdout[9]);
+ }
+
+ // Test dump --debug
+ {
+ vector<string> stdout;
+ NO_FATALS(RunActionStdoutLines(Substitute(
+ "pbc dump $0 --debug", metadata_path), &stdout));
+ ASSERT_EQ(kNumCFileBlocks * 12 + 5, stdout.size());
+ // Header
+ ASSERT_EQ("File header", stdout[0]);
+ ASSERT_EQ("-------", stdout[1]);
+ ASSERT_STR_MATCHES(stdout[2], "^Protobuf container version: [0-9]+$");
+ ASSERT_STR_MATCHES(stdout[3], "^Total container file size: [0-9]+$");
+ ASSERT_EQ("Entry PB type: kudu.BlockRecordPB", stdout[4]);
+ ASSERT_EQ("", stdout[5]);
+ // The first block
+ ASSERT_EQ("Message 0", stdout[6]);
+ ASSERT_STR_MATCHES(stdout[7], "^offset: [0-9]+$");
+ ASSERT_STR_MATCHES(stdout[8], "^length: [0-9]+$");
+ ASSERT_EQ("-------", stdout[9]);
+ ASSERT_EQ("block_id {", stdout[10]);
+ ASSERT_STR_MATCHES(stdout[11], "^ id: [0-9]+$");
+ ASSERT_EQ("}", stdout[12]);
+ ASSERT_EQ("op_type: CREATE", stdout[13]);
+ ASSERT_STR_MATCHES(stdout[14], "^timestamp_us: [0-9]+$");
+ ASSERT_EQ("offset: 0", stdout[15]);
+ ASSERT_EQ("length: 153", stdout[16]);
+ ASSERT_EQ("", stdout[17]);
+ }
+
+ // Test dump --oneline
+ {
+ vector<string> stdout;
+ NO_FATALS(RunActionStdoutLines(Substitute(
+ "pbc dump $0 --oneline", metadata_path), &stdout));
+ ASSERT_EQ(kNumCFileBlocks, stdout.size());
+ ASSERT_STR_MATCHES(stdout[0],
+ "^0\tblock_id \\{ id: [0-9]+ \\} op_type: CREATE "
+ "timestamp_us: [0-9]+ offset: 0 length: 153$");
+ }
+
+ // Test dump --json
+ {
+ vector<string> stdout;
+ NO_FATALS(RunActionStdoutLines(Substitute(
+ "pbc dump $0 --json", metadata_path), &stdout));
+ ASSERT_EQ(kNumCFileBlocks, stdout.size());
+ ASSERT_STR_MATCHES(stdout[0],
+ R"(^\{"blockId":\{"id":"[0-9]+"\},"opType":"CREATE",)"
+ R"("timestampUs":"[0-9]+","offset":"0","length":"153"\}$$)");
+ }
+
+ // Test dump --json_pretty
+ {
+ vector<string> stdout;
+ NO_FATALS(RunActionStdoutLines(Substitute(
+ "pbc dump $0 --json_pretty", metadata_path), &stdout));
+ ASSERT_EQ(kNumCFileBlocks * 10 - 1, stdout.size());
+ ASSERT_EQ("{", stdout[0]);
+ ASSERT_EQ(R"( "blockId": {)", stdout[1]);
+ ASSERT_STR_MATCHES(stdout[2], R"( "id": "[0-9]+")");
+ ASSERT_EQ(" },", stdout[3]);
+ ASSERT_EQ(R"( "opType": "CREATE",)", stdout[4]);
+ ASSERT_STR_MATCHES(stdout[5], R"( "timestampUs": "[0-9]+",)");
+ ASSERT_EQ(R"( "offset": "0",)", stdout[6]);
+ ASSERT_EQ(R"( "length": "153")", stdout[7]);
+ ASSERT_EQ(R"(})", stdout[8]);
+ ASSERT_EQ("", stdout[9]);
+ }
+
+ // Utility to set the editor up based on the given shell command.
+ 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 $1", extra_flags, metadata_path),
+ 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/CREATE/DELETE/ \"$@\"\n", &stdout, nullptr,
+ "--nobackup"));
+ ASSERT_EQ("", stdout);
+
+ // Dump to make sure the edit took place.
+ vector<string> stdout_lines;
+ NO_FATALS(RunActionStdoutLines(Substitute(
+ "pbc dump $0 --oneline", metadata_path), &stdout_lines));
+ ASSERT_EQ(kNumCFileBlocks, stdout_lines.size());
+ ASSERT_STR_MATCHES(stdout_lines[0],
+ "^0\tblock_id \\{ id: [0-9]+ \\} op_type: DELETE "
+ "timestamp_us: [0-9]+ offset: 0 length: 153$");
+
+ // Make sure no backup file was written.
+ bool found_backup;
+ ASSERT_OK(HasAtLeastOneBackupFile(kDataDir, &found_backup));
+ ASSERT_FALSE(found_backup);
+ }
+
+ // Test 'edit' by setting --json_pretty flag.
+ {
+ string stdout;
+ ASSERT_OK(DoEdit("exec sed -i -e s/DELETE/CREATE/ \"$@\"\n", &stdout, nullptr,
+ "--nobackup --json_pretty"));
+ ASSERT_EQ("", stdout);
+
+ // Dump to make sure the edit took place.
+ vector<string> stdout_lines;
+ NO_FATALS(RunActionStdoutLines(Substitute(
+ "pbc dump $0 --oneline", metadata_path), &stdout_lines));
+ ASSERT_EQ(kNumCFileBlocks, stdout_lines.size());
+ ASSERT_STR_MATCHES(stdout_lines[0],
+ "^0\tblock_id \\{ id: [0-9]+ \\} op_type: CREATE "
+ "timestamp_us: [0-9]+ offset: 0 length: 153$");
+
+ // Make sure no backup file was written.
+ bool found_backup;
+ ASSERT_OK(HasAtLeastOneBackupFile(kDataDir, &found_backup));
+ ASSERT_FALSE(found_backup);
+ }
+
+ // Test 'edit' with a backup.
+ {
+ string stdout;
+ ASSERT_OK(DoEdit("exec sed -i -e s/CREATE/DELETE/ \"$@\"\n", &stdout));
+ ASSERT_EQ("", stdout);
+
+ // Make sure a backup file was written.
+ bool found_backup;
+ ASSERT_OK(HasAtLeastOneBackupFile(kDataDir, &found_backup));
+ ASSERT_TRUE(found_backup);
+ }
+
+ // Test 'edit' with an unsuccessful edit.
+ {
+ string stdout, stderr;
+ string path;
+ ASSERT_OK(FindExecutable("false", {}, &path));
+ Status s = DoEdit(path, &stdout, &stderr);
+ ASSERT_FALSE(s.ok());
+ ASSERT_EQ("", stdout);
+ ASSERT_STR_CONTAINS(stderr, "Aborted: editor returned non-zero exit code");
+ }
+
+ // Test 'edit' with an edit which tries to write some invalid JSON (missing required fields).
+ {
+ string stdout, stderr;
+ Status s = DoEdit("echo {} > $@\n", &stdout, &stderr);
+ ASSERT_EQ("", stdout);
+ ASSERT_STR_MATCHES(stderr,
+ "Invalid argument: Unable to parse JSON text: \\{\\}: "
+ ": missing field .*");
+ // NOTE: the above extra ':' is due to an apparent bug in protobuf.
+ }
+
+ // Test 'edit' with an edit that writes some invalid JSON (bad syntax)
+ {
+ string stdout, stderr;
+ Status s = DoEdit("echo not-a-json-string > $@\n", &stdout, &stderr);
+ ASSERT_EQ("", stdout);
+ ASSERT_STR_CONTAINS(stderr, "Corruption: JSON text is corrupt: Invalid value.");
+ }
+
+ // The file should be unchanged by the unsuccessful edits above.
+ {
+ vector<string> stdout;
+ NO_FATALS(RunActionStdoutLines(Substitute(
+ "pbc dump $0 --oneline", metadata_path), &stdout));
+ ASSERT_EQ(kNumCFileBlocks, stdout.size());
+ ASSERT_STR_MATCHES(stdout[0],
+ "^0\tblock_id \\{ id: [0-9]+ \\} op_type: DELETE "
+ "timestamp_us: [0-9]+ offset: 0 length: 153$");
+ }
+}
+
TEST_F(ToolTest, TestFsDumpCFile) {
const int kNumEntries = 8192;
const string kTestDir = GetTestPath("test");
diff --git a/src/kudu/tools/tool_action_pbc.cc b/src/kudu/tools/tool_action_pbc.cc
index ecb3926..be5c636 100644
--- a/src/kudu/tools/tool_action_pbc.cc
+++ b/src/kudu/tools/tool_action_pbc.cc
@@ -15,8 +15,8 @@
// specific language governing permissions and limitations
// under the License.
+#include <cstdio>
#include <cstdlib>
-#include <exception>
#include <fstream> // IWYU pragma: keep
#include <functional>
#include <iostream>
@@ -32,6 +32,14 @@
#include <google/protobuf/stubs/status.h>
#include <google/protobuf/stubs/stringpiece.h>
#include <google/protobuf/util/json_util.h>
+#include <rapidjson/document.h>
+#include <rapidjson/encodings.h>
+#include <rapidjson/error/en.h>
+#include <rapidjson/error/error.h>
+#include <rapidjson/filereadstream.h>
+#include <rapidjson/reader.h>
+#include <rapidjson/stringbuffer.h>
+#include <rapidjson/writer.h>
#include "kudu/gutil/macros.h"
#include "kudu/gutil/map-util.h"
@@ -40,6 +48,7 @@
#include "kudu/tools/tool_action.h"
#include "kudu/util/env.h"
#include "kudu/util/flag_tags.h"
+#include "kudu/util/flag_validators.h"
#include "kudu/util/path_util.h"
#include "kudu/util/pb_util.h"
#include "kudu/util/scoped_cleanup.h"
@@ -60,11 +69,29 @@ TAG_FLAG(oneline, stable);
DEFINE_bool(json, false, "Print protobufs in JSON format");
TAG_FLAG(json, stable);
+DEFINE_bool(json_pretty, false, "Print or edit protobufs in JSON pretty format");
+TAG_FLAG(json_pretty, evolving);
+
DEFINE_bool(debug, false, "Print extra debugging information about each protobuf");
TAG_FLAG(debug, stable);
DEFINE_bool(backup, true, "Write a backup file");
+bool ValidatePBCFlags() {
+ int count = 0;
+ if (FLAGS_debug) count++;
+ if (FLAGS_oneline) count++;
+ if (FLAGS_json) count++;
+ if (FLAGS_json_pretty) count++;
+ if (count > 1) {
+ LOG(ERROR) << "only one of --debug, --oneline, --json or --json_pretty "
+ "can be provided at most";
+ return false;
+ }
+ return true;
+}
+GROUP_FLAG_VALIDATOR(validate_pbc_flags, ValidatePBCFlags);
+
namespace kudu {
using pb_util::ReadablePBContainerFile;
@@ -77,18 +104,12 @@ namespace {
const char* const kPathArg = "path";
Status DumpPBContainerFile(const RunnerContext& context) {
- if (FLAGS_oneline && FLAGS_json) {
- return Status::InvalidArgument("only one of --json or --oneline may be provided");
- }
-
- if (FLAGS_debug && (FLAGS_oneline || FLAGS_json)) {
- return Status::InvalidArgument("--debug is not compatible with --json or --oneline");
- }
-
const string& path = FindOrDie(context.required_args, kPathArg);
auto format = ReadablePBContainerFile::Format::DEFAULT;
if (FLAGS_json) {
format = ReadablePBContainerFile::Format::JSON;
+ } else if (FLAGS_json_pretty) {
+ format = ReadablePBContainerFile::Format::JSON_PRETTY;
} else if (FLAGS_oneline) {
format = ReadablePBContainerFile::Format::ONELINE;
} else if (FLAGS_debug) {
@@ -124,21 +145,6 @@ Status RunEditor(const string& path) {
return Status::OK();
}
-Status LoadFileToLines(const string& path,
- vector<string>* lines) {
- try {
- string line;
- std::ifstream f(path);
- while (std::getline(f, line)) {
- lines->push_back(line);
- }
- } catch (const std::exception& e) {
- return Status::IOError(e.what());
- }
- return Status::OK();
-
-}
-
Status EditFile(const RunnerContext& context) {
Env* env = Env::Default();
const string& path = FindOrDie(context.required_args, kPathArg);
@@ -179,7 +185,10 @@ Status EditFile(const RunnerContext& context) {
// It is quite difficult to get a C++ ostream pointed at a temporary file,
// so we just dump to a string and then write it to a file.
std::ostringstream stream;
- RETURN_NOT_OK(pb_reader.Dump(&stream, ReadablePBContainerFile::Format::JSON));
+ ReadablePBContainerFile::Format format =
+ FLAGS_json_pretty ? ReadablePBContainerFile::Format::JSON_PRETTY :
+ ReadablePBContainerFile::Format::JSON;
+ RETURN_NOT_OK(pb_reader.Dump(&stream, format));
RETURN_NOT_OK_PREPEND(tmp_json_file->Append(stream.str()), "couldn't write to temporary file");
RETURN_NOT_OK_PREPEND(tmp_json_file->Close(), "couldn't close temporary file");
}
@@ -198,23 +207,53 @@ Status EditFile(const RunnerContext& context) {
// Parse the edited file.
unique_ptr<google::protobuf::Message> m(prototype->New());
- vector<string> lines;
- RETURN_NOT_OK(LoadFileToLines(tmp_json_path, &lines));
+
+ FILE* fp = nullptr;
+ POINTER_RETRY_ON_EINTR(fp, fopen(tmp_json_path.c_str(), "r"));
+ if (fp == nullptr) {
+ return Status::IOError(Substitute("open file ($0) failed", tmp_json_path));
+ }
+ SCOPED_CLEANUP({ fclose(fp); });
+
+ char read_buffer[65536];
+ rapidjson::FileReadStream in_stream(fp, read_buffer, sizeof(read_buffer));
+
JsonParseOptions opts;
opts.case_insensitive_enum_parsing = true;
- for (const string& l : lines) {
+ do {
+ // The file may contains multiple JSON objects, parse one object once.
+ rapidjson::Document document;
+ document.ParseStream<rapidjson::kParseStopWhenDoneFlag>(in_stream);
+ if (document.HasParseError()) {
+ auto code = document.GetParseError();
+ if (code != rapidjson::kParseErrorDocumentEmpty) {
+ return Status::Corruption("JSON text is corrupt",
+ rapidjson::GetParseError_En(code));
+ }
+ // No more JSON object left.
+ break;
+ }
+
+ // Convert one JSON object to protobuf object once.
+ rapidjson::StringBuffer buffer;
+ rapidjson::Writer<rapidjson::StringBuffer> writer(buffer);
+ document.Accept(writer);
m->Clear();
- const auto& google_status = JsonStringToMessage(l, m.get(), opts);
+ auto str = buffer.GetString();
+ const auto& google_status = JsonStringToMessage(str, m.get(), opts);
if (!google_status.ok()) {
return Status::InvalidArgument(
- Substitute("Unable to parse JSON line: $0", l),
+ Substitute("Unable to parse JSON text: $0", str),
google_status.error_message().ToString());
}
+
+ // Append the protobuf object to writer.
RETURN_NOT_OK_PREPEND(pb_writer.Append(*m), "unable to append PB to output");
- }
+ } while (true);
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.
if (FLAGS_backup) {
// Move the old file to a backup location.
@@ -238,17 +277,20 @@ unique_ptr<Mode> BuildPbcMode() {
unique_ptr<Action> dump =
ActionBuilder("dump", &DumpPBContainerFile)
.Description("Dump a PBC (protobuf container) file")
+ .AddRequiredParameter({kPathArg, "path to PBC file"})
.AddOptionalParameter("debug")
.AddOptionalParameter("oneline")
.AddOptionalParameter("json")
- .AddRequiredParameter({kPathArg, "path to PBC file"})
+ .AddOptionalParameter("json_pretty")
.Build();
unique_ptr<Action> edit =
ActionBuilder("edit", &EditFile)
.Description("Edit a PBC (protobuf container) file")
- .AddOptionalParameter("backup")
.AddRequiredParameter({kPathArg, "path to PBC file"})
+ .AddOptionalParameter("backup")
+ .AddOptionalParameter("json")
+ .AddOptionalParameter("json_pretty")
.Build();
return ModeBuilder("pbc")
diff --git a/src/kudu/util/pb_util.cc b/src/kudu/util/pb_util.cc
index 5a3fb8a..6e1f1a3 100644
--- a/src/kudu/util/pb_util.cc
+++ b/src/kudu/util/pb_util.cc
@@ -994,9 +994,13 @@ Status ReadablePBContainerFile::Dump(ostream* os, ReadablePBContainerFile::Forma
*os << SecureDebugString(*msg) << endl;
break;
case Format::JSON:
+ case Format::JSON_PRETTY:
buf.clear();
- const auto& google_status = google::protobuf::util::MessageToJsonString(
- *msg, &buf, google::protobuf::util::JsonPrintOptions());
+ auto opt = google::protobuf::util::JsonPrintOptions();
+ if (format == Format::JSON_PRETTY) {
+ opt.add_whitespace = true;
+ }
+ const auto& google_status = google::protobuf::util::MessageToJsonString(*msg, &buf, opt);
if (!google_status.ok()) {
return Status::RuntimeError("could not convert PB to JSON", google_status.ToString());
}
diff --git a/src/kudu/util/pb_util.h b/src/kudu/util/pb_util.h
index 13e496f..87bf4e2 100644
--- a/src/kudu/util/pb_util.h
+++ b/src/kudu/util/pb_util.h
@@ -406,8 +406,10 @@ class ReadablePBContainerFile {
DEBUG,
// Print each message on its own line.
ONELINE,
- // Dump in JSON.
- JSON
+ // Dump in JSON compact format.
+ JSON,
+ // Dump in JSON pretty format.
+ JSON_PRETTY
};
Status Dump(std::ostream* os, Format format);