Mailing-List: contact commits-help@kudu.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@kudu.apache.org
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: todd@apache.org
To: commits@kudu.apache.org
Message-Id: <213fb160942d4eaf92dc227a9c5c2e4b@git.apache.org>
Subject: kudu git commit: tool: add a 'pbc edit' command
Date: Tue,  6 Jun 2017 05:50:34 +0000 (UTC)
archived-at: Tue, 06 Jun 2017 05:50:37 -0000

Repository: kudu
Updated Branches:
  refs/heads/master fc98b1ae7 -> bf378b4bd


tool: add a 'pbc edit' command

This adds a new command 'kudu pbc edit' which can be used to edit any
PBC file. It dumps the file in JSON format, then opens the user's
$EDITOR, and then re-encodes it back to PBC.

Change-Id: Ie9434935469df8978a4f3fb3719f0245499aece5
Reviewed-on: http://gerrit.cloudera.org:8080/7048
Reviewed-by: Adar Dembo <adar@cloudera.com>
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mpercy@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/bf378b4b
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/bf378b4b
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/bf378b4b

Branch: refs/heads/master
Commit: bf378b4bd139665ae08cabf7c31464ea07dd2fac
Parents: fc98b1a
Author: Todd Lipcon <todd@cloudera.com>
Authored: Thu Jun 1 17:23:06 2017 -0700
Committer: Todd Lipcon <todd@apache.org>
Committed: Tue Jun 6 05:48:15 2017 +0000

----------------------------------------------------------------------
 src/kudu/tools/kudu-tool-test.cc  |  82 ++++++++++++++++-
 src/kudu/tools/tool_action_pbc.cc | 155 ++++++++++++++++++++++++++++++++-
 src/kudu/util/pb_util-test.cc     |  18 ++--
 src/kudu/util/pb_util.cc          | 104 ++++++++++++++--------
 src/kudu/util/pb_util.h           |  31 +++++--
 5 files changed, 341 insertions(+), 49 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/bf378b4b/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 b7d4190..615ef1e 100644
--- a/src/kudu/tools/kudu-tool-test.cc
+++ b/src/kudu/tools/kudu-tool-test.cc
@@ -76,6 +76,7 @@
 #include "kudu/util/subprocess.h"
 #include "kudu/util/test_macros.h"
 #include "kudu/util/test_util.h"
+#include "kudu/util/url-coding.h"
 
 DECLARE_string(block_manager);
 
@@ -649,7 +650,7 @@ TEST_F(ToolTest, TestFsDumpUuid) {
   ASSERT_EQ(uuid, stdout);
 }
 
-TEST_F(ToolTest, TestPbcDump) {
+TEST_F(ToolTest, TestPbcTools) {
   const string kTestDir = GetTestPath("test");
   string uuid;
   string instance_path;
@@ -672,6 +673,7 @@ TEST_F(ToolTest, TestPbcDump) {
     ASSERT_EQ(Substitute("uuid: \"$0\"", uuid), stdout[2]);
     ASSERT_STR_MATCHES(stdout[3], "^format_stamp: \"Formatted at .*\"$");
   }
+  // Test dump --oneline
   {
     string stdout;
     NO_FATALS(RunActionStdoutString(Substitute(
@@ -680,6 +682,84 @@ TEST_F(ToolTest, TestPbcDump) {
     ASSERT_STR_MATCHES(stdout, Substitute(
         "^0\tuuid: \"$0\" format_stamp: \"Formatted at .*\"$$", uuid));
   }
+  // Test dump --json
+  {
+    // Since the UUID is listed as 'bytes' rather than 'string' in the PB, it dumps
+    // base64-encoded.
+    string uuid_b64;
+    Base64Encode(uuid, &uuid_b64);
+
+    string stdout;
+    NO_FATALS(RunActionStdoutString(Substitute(
+        "pbc dump $0/instance --json", kTestDir), &stdout));
+    SCOPED_TRACE(stdout);
+    ASSERT_STR_MATCHES(stdout, Substitute(
+        "^\\{\"uuid\":\"$0\",\"formatStamp\":\"Formatted at .*\"\\}$$", uuid_b64));
+  }
+
+  // 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 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),
+                   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_EQ("", stdout);
+
+    // Dump to make sure the edit took place.
+    NO_FATALS(RunActionStdoutString(Substitute(
+        "pbc dump $0/instance --oneline", kTestDir), &stdout));
+    ASSERT_STR_MATCHES(stdout, Substitute(
+        "^0\tuuid: \"$0\" format_stamp: \"Edited at .*\"$$", uuid));
+  }
+
+  // Test 'edit' with an unsuccessful edit.
+  {
+    string stdout, stderr;
+    Status s = DoEdit("/bin/false", &stdout, &stderr);
+    ASSERT_FALSE(s.ok());
+    ASSERT_EQ("", stdout);
+    ASSERT_EQ("Aborted: editor returned non-zero exit code", stderr);
+  }
+
+  // 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 line: \\{\\}: "
+                       ": 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_EQ("Invalid argument: Unable to parse JSON line: not-a-json-string: Unexpected token.\n"
+              "not-a-json-string\n"
+              "^", stderr);
+  }
+
+  // The file should be unchanged by the unsuccessful edits above.
+  {
+    string stdout;
+    NO_FATALS(RunActionStdoutString(Substitute(
+        "pbc dump $0/instance --oneline", kTestDir), &stdout));
+    ASSERT_STR_MATCHES(stdout, Substitute(
+        "^0\tuuid: \"$0\" format_stamp: \"Edited at .*\"$$", uuid));
+  }
 }
 
 TEST_F(ToolTest, TestFsDumpCFile) {

http://git-wip-us.apache.org/repos/asf/kudu/blob/bf378b4b/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 ed4afe9..5b6d8d8 100644
--- a/src/kudu/tools/tool_action_pbc.cc
+++ b/src/kudu/tools/tool_action_pbc.cc
@@ -18,25 +18,40 @@
 #include "kudu/tools/tool_action.h"
 
 #include <gflags/gflags.h>
+#include <google/protobuf/message.h>
+#include <google/protobuf/util/json_util.h>
+
+#include <fstream>
 #include <iostream>
 #include <memory>
 #include <string>
 
 #include "kudu/gutil/map-util.h"
+#include "kudu/gutil/strings/substitute.h"
 #include "kudu/util/env.h"
+#include "kudu/util/env_util.h"
 #include "kudu/util/flag_tags.h"
+#include "kudu/util/path_util.h"
 #include "kudu/util/pb_util.h"
+#include "kudu/util/scoped_cleanup.h"
 #include "kudu/util/status.h"
+#include "kudu/util/subprocess.h"
 
 using std::cout;
-using std::endl;
 using std::string;
 using std::unique_ptr;
 
 DEFINE_bool(oneline, false, "print each protobuf on a single line");
 TAG_FLAG(oneline, stable);
 
+DEFINE_bool(json, false, "print protobufs in JSON format");
+TAG_FLAG(json, stable);
+
 namespace kudu {
+
+using pb_util::ReadablePBContainerFile;
+using strings::Substitute;
+
 namespace tools {
 
 namespace {
@@ -44,14 +59,140 @@ 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");
+  }
+
   const string& path = FindOrDie(context.required_args, kPathArg);
+  auto format = ReadablePBContainerFile::Format::DEFAULT;
+  if (FLAGS_json) {
+    format = ReadablePBContainerFile::Format::JSON;
+  } else if (FLAGS_oneline) {
+    format = ReadablePBContainerFile::Format::ONELINE;
+  }
+
+  Env* env = Env::Default();
+  unique_ptr<RandomAccessFile> reader;
+  RETURN_NOT_OK(env->NewRandomAccessFile(path, &reader));
+  ReadablePBContainerFile pb_reader(std::move(reader));
+  RETURN_NOT_OK(pb_reader.Open());
+  RETURN_NOT_OK(pb_reader.Dump(&std::cout, format));
+
+  return Status::OK();
+}
+
+// Run the user's configured editor on 'path'.
+Status RunEditor(const string& path) {
+  const char* editor = getenv("EDITOR");
+  if (!editor) {
+    editor = "vi";
+  }
+  Subprocess editor_proc({editor, path});
+  editor_proc.ShareParentStdin();
+  editor_proc.ShareParentStdout();
+  editor_proc.ShareParentStderr();
+  RETURN_NOT_OK_PREPEND(editor_proc.Start(), "couldn't start editor");
+  int ret = 0;
+  RETURN_NOT_OK_PREPEND(editor_proc.Wait(&ret), "edit failed");
+  if (ret != 0) {
+    return Status::Aborted("editor returned non-zero exit code");
+  }
+  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);
+  const string& dir = DirName(path);
+
+  // Open the original file.
   unique_ptr<RandomAccessFile> reader;
   RETURN_NOT_OK(env->NewRandomAccessFile(path, &reader));
-  pb_util::ReadablePBContainerFile pb_reader(std::move(reader));
+  ReadablePBContainerFile pb_reader(std::move(reader));
   RETURN_NOT_OK(pb_reader.Open());
-  RETURN_NOT_OK(pb_reader.Dump(&std::cout, FLAGS_oneline));
+
+  // Make a new RWFile where we'll write the changed PBC file.
+  // Do this up front so that we fail early if the user doesn't have appropriate permissions.
+  const string tmp_out_path = path + ".new";
+  unique_ptr<RWFile> out_rwfile;
+  RETURN_NOT_OK_PREPEND(env->NewRWFile(tmp_out_path, &out_rwfile), "couldn't open output PBC file");
+  env_util::ScopedFileDeleter delete_tmp_output(env, tmp_out_path);
+
+  // Also make a tmp file where we'll write the PBC in JSON format for
+  // easy editing.
+  unique_ptr<WritableFile> tmp_json_file;
+  string tmp_json_path;
+  const string tmp_template = Substitute("pbc-edit$0.XXXXXX", kTmpInfix);
+  RETURN_NOT_OK_PREPEND(env->NewTempWritableFile(WritableFileOptions(),
+                                                 JoinPathSegments(dir, tmp_template),
+                                                 &tmp_json_path, &tmp_json_file),
+                        "couldn't create temporary file");
+  env_util::ScopedFileDeleter delete_tmp_json(env, tmp_json_path);
+
+  // Dump the contents in JSON to the temporary file.
+  {
+    // 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));
+    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");
+  }
+
+  // Open the temporary file in the editor for the user to edit, and load the content
+  // back into a list of lines.
+  RETURN_NOT_OK(RunEditor(tmp_json_path));
+
+  {
+    const google::protobuf::Message* prototype;
+    RETURN_NOT_OK_PREPEND(pb_reader.GetPrototype(&prototype),
+                          "couldn't load message prototype from file");
+
+    pb_util::WritablePBContainerFile pb_writer(std::shared_ptr<RWFile>(out_rwfile.release()));
+    RETURN_NOT_OK_PREPEND(pb_writer.CreateNew(*prototype), "couldn't init PBC writer");
+
+    // Parse the edited file.
+    unique_ptr<google::protobuf::Message> m(prototype->New());
+    vector<string> lines;
+    RETURN_NOT_OK(LoadFileToLines(tmp_json_path, &lines));
+    for (const string& l : lines) {
+      m->Clear();
+      const auto& google_status = google::protobuf::util::JsonStringToMessage(l, m.get());
+      if (!google_status.ok()) {
+        return Status::InvalidArgument(
+            Substitute("Unable to parse JSON line: $0", l),
+            google_status.error_message().ToString());
+      }
+      RETURN_NOT_OK_PREPEND(pb_writer.Append(*m), "unable to append PB to output");
+    }
+    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;
+  RETURN_NOT_OK_PREPEND(env->RenameFile(tmp_out_path, path),
+                        "couldn't move new file into place");
+  delete_tmp_output.Cancel();
+  WARN_NOT_OK(env->SyncDir(dir), "couldn't sync directory");
 
   return Status::OK();
 }
@@ -63,12 +204,20 @@ unique_ptr<Mode> BuildPbcMode() {
       ActionBuilder("dump", &DumpPBContainerFile)
       .Description("Dump a PBC (protobuf container) file")
       .AddOptionalParameter("oneline")
+      .AddOptionalParameter("json")
+      .AddRequiredParameter({kPathArg, "path to PBC file"})
+      .Build();
+
+  unique_ptr<Action> edit =
+      ActionBuilder("edit", &EditFile)
+      .Description("Edit a PBC (protobuf container) file")
       .AddRequiredParameter({kPathArg, "path to PBC file"})
       .Build();
 
   return ModeBuilder("pbc")
       .Description("Operate on PBC (protobuf container) files")
       .AddAction(std::move(dump))
+      .AddAction(std::move(edit))
       .Build();
 }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/bf378b4b/src/kudu/util/pb_util-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/pb_util-test.cc b/src/kudu/util/pb_util-test.cc
index 304e910..eddc799 100644
--- a/src/kudu/util/pb_util-test.cc
+++ b/src/kudu/util/pb_util-test.cc
@@ -80,7 +80,7 @@ class TestPBUtil : public KuduTest {
   // XORs the data in the specified range of the file at the given path.
   Status BitFlipFileByteRange(const string& path, uint64_t offset, uint64_t length);
 
-  void DumpPBCToString(const string& path, bool oneline_output, string* ret);
+  void DumpPBCToString(const string& path, ReadablePBContainerFile::Format format, string* ret);
 
   // Truncate the specified file to the specified length.
   Status TruncateFile(const string& path, uint64_t size);
@@ -509,14 +509,15 @@ TEST_F(TestPBUtil, TestPopulateDescriptorSet) {
   }
 }
 
-void TestPBUtil::DumpPBCToString(const string& path, bool oneline_output,
+void TestPBUtil::DumpPBCToString(const string& path,
+                                 ReadablePBContainerFile::Format format,
                                  string* ret) {
   unique_ptr<RandomAccessFile> reader;
   ASSERT_OK(env_->NewRandomAccessFile(path, &reader));
   ReadablePBContainerFile pb_reader(std::move(reader));
   ASSERT_OK(pb_reader.Open());
   ostringstream oss;
-  ASSERT_OK(pb_reader.Dump(&oss, oneline_output));
+  ASSERT_OK(pb_reader.Dump(&oss, format));
   ASSERT_OK(pb_reader.Close());
   *ret = oss.str();
 }
@@ -553,6 +554,10 @@ TEST_P(TestPBContainerVersions, TestDumpPBContainer) {
     "0\trecord_one { name: \"foo\" value: 0 } record_two { record { name: \"foo\" value: 0 } }\n"
     "1\trecord_one { name: \"foo\" value: 1 } record_two { record { name: \"foo\" value: 2 } }\n";
 
+  const char* kExpectedOutputJson =
+      "{\"recordOne\":{\"name\":\"foo\",\"value\":0},\"recordTwo\":{\"record\":{\"name\":\"foo\",\"value\":0}}}\n" // NOLINT
+      "{\"recordOne\":{\"name\":\"foo\",\"value\":1},\"recordTwo\":{\"record\":{\"name\":\"foo\",\"value\":2}}}\n"; // NOLINT
+
   ProtoContainerTest3PB pb;
   pb.mutable_record_one()->set_name("foo");
   pb.mutable_record_two()->mutable_record()->set_name("foo");
@@ -569,11 +574,14 @@ TEST_P(TestPBContainerVersions, TestDumpPBContainer) {
   ASSERT_OK(pb_writer->Close());
 
   string output;
-  NO_FATALS(DumpPBCToString(path_, false, &output));
+  NO_FATALS(DumpPBCToString(path_, ReadablePBContainerFile::Format::DEFAULT, &output));
   ASSERT_STREQ(kExpectedOutput, output.c_str());
 
-  NO_FATALS(DumpPBCToString(path_, true, &output));
+  NO_FATALS(DumpPBCToString(path_, ReadablePBContainerFile::Format::ONELINE, &output));
   ASSERT_STREQ(kExpectedOutputShort, output.c_str());
+
+  NO_FATALS(DumpPBCToString(path_, ReadablePBContainerFile::Format::JSON, &output));
+  ASSERT_STREQ(kExpectedOutputJson, output.c_str());
 }
 
 TEST_F(TestPBUtil, TestOverwriteExistingPB) {

http://git-wip-us.apache.org/repos/asf/kudu/blob/bf378b4b/src/kudu/util/pb_util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/pb_util.cc b/src/kudu/util/pb_util.cc
index a83c309..0c09712 100644
--- a/src/kudu/util/pb_util.cc
+++ b/src/kudu/util/pb_util.cc
@@ -44,6 +44,7 @@
 #include <google/protobuf/message.h>
 #include <google/protobuf/message_lite.h>
 #include <google/protobuf/text_format.h>
+#include <google/protobuf/util/json_util.h>
 
 #include "kudu/gutil/bind.h"
 #include "kudu/gutil/callback.h"
@@ -822,53 +823,88 @@ Status ReadablePBContainerFile::ReadNextPB(Message* msg) {
   return ReadFullPB(reader_.get(), version_, &offset_, msg);
 }
 
-Status ReadablePBContainerFile::Dump(ostream* os, bool oneline) {
-  DCHECK_EQ(FileState::OPEN, state_);
+Status ReadablePBContainerFile::GetPrototype(const Message** prototype) {
+  if (!prototype_) {
+    // Loading the schemas into a DescriptorDatabase (and not directly into
+    // a DescriptorPool) defers resolution until FindMessageTypeByName()
+    // below, allowing for schemas to be loaded in any order.
+    unique_ptr<SimpleDescriptorDatabase> db(new SimpleDescriptorDatabase());
+    for (int i = 0; i < protos()->file_size(); i++) {
+      if (!db->Add(protos()->file(i))) {
+        return Status::Corruption("Descriptor not loaded", Substitute(
+            "Could not load descriptor for PB type $0 referenced in container file",
+            pb_type()));
+      }
+    }
+    unique_ptr<DescriptorPool> pool(new DescriptorPool(db.get()));
+    const Descriptor* desc = pool->FindMessageTypeByName(pb_type());
+    if (!desc) {
+      return Status::NotFound("Descriptor not found", Substitute(
+          "Could not find descriptor for PB type $0 referenced in container file",
+          pb_type()));
+    }
 
-  // Use the embedded protobuf information from the container file to
-  // create the appropriate kind of protobuf Message.
-  //
-  // Loading the schemas into a DescriptorDatabase (and not directly into
-  // a DescriptorPool) defers resolution until FindMessageTypeByName()
-  // below, allowing for schemas to be loaded in any order.
-  SimpleDescriptorDatabase db;
-  for (int i = 0; i < protos()->file_size(); i++) {
-    if (!db.Add(protos()->file(i))) {
-      return Status::Corruption("Descriptor not loaded", Substitute(
-          "Could not load descriptor for PB type $0 referenced in container file",
+    unique_ptr<DynamicMessageFactory> factory(new DynamicMessageFactory());
+    const Message* p = factory->GetPrototype(desc);
+    if (!p) {
+      return Status::NotSupported("Descriptor not supported", Substitute(
+          "Descriptor $0 referenced in container file not supported",
           pb_type()));
     }
+
+    db_ = std::move(db);
+    descriptor_pool_ = std::move(pool);
+    message_factory_ = std::move(factory);
+    prototype_ = p;
   }
-  DescriptorPool pool(&db);
-  const Descriptor* desc = pool.FindMessageTypeByName(pb_type());
-  if (!desc) {
-    return Status::NotFound("Descriptor not found", Substitute(
-        "Could not find descriptor for PB type $0 referenced in container file",
-        pb_type()));
-  }
-  DynamicMessageFactory factory;
-  const Message* prototype = factory.GetPrototype(desc);
-  if (!prototype) {
-    return Status::NotSupported("Descriptor not supported", Substitute(
-        "Descriptor $0 referenced in container file not supported",
-        pb_type()));
+  *prototype = prototype_;
+  return Status::OK();
+}
+
+Status ReadablePBContainerFile::Dump(ostream* os, ReadablePBContainerFile::Format format) {
+  DCHECK_EQ(FileState::OPEN, state_);
+
+  // Since we use the protobuf library support for dumping JSON, there isn't any easy
+  // way to hook in our redaction support. Since this is only used by CLI tools,
+  // just refuse to dump JSON if redaction is enabled.
+  if (format == Format::JSON && KUDU_SHOULD_REDACT()) {
+    return Status::NotSupported("cannot dump PBC file in JSON format if redaction is enabled");
   }
-  unique_ptr<Message> msg(prototype->New());
+
+  // Use the embedded protobuf information from the container file to
+  // create the appropriate kind of protobuf Message.
+  const Message* prototype;
+  RETURN_NOT_OK(GetPrototype(&prototype));
+  unique_ptr<Message> msg(prototype_->New());
 
   // Dump each message in the container file.
   int count = 0;
   Status s;
+  string buf;
   for (s = ReadNextPB(msg.get());
       s.ok();
       s = ReadNextPB(msg.get())) {
-    if (oneline) {
-      *os << count++ << "\t" << SecureShortDebugString(*msg) << endl;
-    } else {
-      *os << "Message " << count << endl;
-      *os << "-------" << endl;
-      *os << SecureDebugString(*msg) << endl;
-      count++;
+    switch (format) {
+      case Format::ONELINE:
+        *os << count << "\t" << SecureShortDebugString(*msg) << endl;
+        break;
+      case Format::DEFAULT:
+        *os << "Message " << count << endl;
+        *os << "-------" << endl;
+        *os << SecureDebugString(*msg) << endl;
+        break;
+      case Format::JSON:
+        buf.clear();
+        const auto& google_status = google::protobuf::util::MessageToJsonString(
+            *msg, &buf, google::protobuf::util::JsonPrintOptions());
+        if (!google_status.ok()) {
+          return Status::RuntimeError("could not convert PB to JSON", google_status.ToString());
+        }
+        *os << buf << endl;
+        break;
     }
+    count++;
+
   }
   return s.IsEndOfFile() ? s.OK() : s;
 }

http://git-wip-us.apache.org/repos/asf/kudu/blob/bf378b4b/src/kudu/util/pb_util.h
----------------------------------------------------------------------
diff --git a/src/kudu/util/pb_util.h b/src/kudu/util/pb_util.h
index 24119c9..8e8a65e 100644
--- a/src/kudu/util/pb_util.h
+++ b/src/kudu/util/pb_util.h
@@ -32,12 +32,15 @@
 
 namespace google {
 namespace protobuf {
+class DescriptorPool;
 class FileDescriptor;
 class FileDescriptorSet;
-class MessageLite;
 class Message;
-}
-}
+class MessageFactory;
+class MessageLite;
+class SimpleDescriptorDatabase;
+} // namespace protobuf
+} // namespace google
 
 namespace kudu {
 
@@ -394,9 +397,15 @@ class ReadablePBContainerFile {
   // Dumps any unread protobuf messages in the container to 'os'. Each
   // message's DebugString() method is invoked to produce its textual form.
   // File must be open.
-  //
-  // If 'oneline' is true, prints each message on a single line.
-  Status Dump(std::ostream* os, bool oneline);
+  enum class Format {
+    // Print each message on multiple lines, with intervening headers.
+    DEFAULT,
+    // Print each message on its own line.
+    ONELINE,
+    // Dump in JSON.
+    JSON
+  };
+  Status Dump(std::ostream* os, Format format);
 
   // Closes the container.
   Status Close();
@@ -409,6 +418,10 @@ class ReadablePBContainerFile {
     return protos_.get();
   }
 
+  // Get the prototype instance for the type of messages stored in this
+  // file. The returned Message is owned by this ReadablePBContainerFile instance.
+  Status GetPrototype(const google::protobuf::Message** prototype);
+
   // Return the protobuf container file format version.
   // File must be open.
   int version() const;
@@ -428,6 +441,12 @@ class ReadablePBContainerFile {
   // Wrapped in a unique_ptr so that clients need not include PB headers.
   std::unique_ptr<google::protobuf::FileDescriptorSet> protos_;
 
+  // Protobuf infrastructure which owns the message prototype 'prototype_'.
+  std::unique_ptr<google::protobuf::SimpleDescriptorDatabase> db_;
+  std::unique_ptr<google::protobuf::DescriptorPool> descriptor_pool_;
+  std::unique_ptr<google::protobuf::MessageFactory> message_factory_;
+  const google::protobuf::Message* prototype_ = nullptr;
+
   std::shared_ptr<RandomAccessFile> reader_;
 };