You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by al...@apache.org on 2019/11/20 00:04:51 UTC

[kudu] branch master updated (93ba778 -> 68f9fbc)

This is an automated email from the ASF dual-hosted git repository.

alexey pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git.


    from 93ba778  [mini-cluster] another fix on building mini-cluster JAR
     new 8369380  [docs] gradle-related update on RELEASING.adoc
     new 68f9fbc  KUDU-2971 p1: add subprocess module

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 CMakeLists.txt                                  |   1 +
 RELEASING.adoc                                  | 176 ++++++++++++-------
 src/kudu/{kserver => subprocess}/CMakeLists.txt |  18 +-
 src/kudu/subprocess/subprocess_protocol.cc      | 217 ++++++++++++++++++++++++
 src/kudu/subprocess/subprocess_protocol.h       |  96 +++++++++++
 src/kudu/tools/CMakeLists.txt                   |   2 +
 src/kudu/tools/kudu-tool-test.cc                |  25 +--
 src/kudu/tools/tool_action_common.cc            | 173 -------------------
 src/kudu/tools/tool_action_common.h             |  65 -------
 src/kudu/tools/tool_action_test.cc              |  30 ++--
 10 files changed, 467 insertions(+), 336 deletions(-)
 copy src/kudu/{kserver => subprocess}/CMakeLists.txt (78%)
 create mode 100644 src/kudu/subprocess/subprocess_protocol.cc
 create mode 100644 src/kudu/subprocess/subprocess_protocol.h


[kudu] 02/02: KUDU-2971 p1: add subprocess module

Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit 68f9fbc420a7ade895ffa639971978713fbbee4f
Author: Hao Hao <ha...@cloudera.com>
AuthorDate: Mon Oct 7 16:06:12 2019 -0700

    KUDU-2971 p1: add subprocess module
    
    Utility classes exist that allow for IPC over stdin/stdout via protobuf
    and JSON-encoded protobuf. This commit moves those classes into their
    own directory so it can be reused by other subprocesses.
    
    Following commits can then extend it to support concurrent communications
    with subprocess. There are no functional changes in this patch.
    
    Change-Id: If73e27772e1897a04f04229c4906a24c61e361f2
    Reviewed-on: http://gerrit.cloudera.org:8080/14425
    Tested-by: Kudu Jenkins
    Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
 CMakeLists.txt                             |   1 +
 src/kudu/subprocess/CMakeLists.txt         |  30 ++++
 src/kudu/subprocess/subprocess_protocol.cc | 217 +++++++++++++++++++++++++++++
 src/kudu/subprocess/subprocess_protocol.h  |  96 +++++++++++++
 src/kudu/tools/CMakeLists.txt              |   2 +
 src/kudu/tools/kudu-tool-test.cc           |  25 ++--
 src/kudu/tools/tool_action_common.cc       | 173 -----------------------
 src/kudu/tools/tool_action_common.h        |  65 ---------
 src/kudu/tools/tool_action_test.cc         |  30 ++--
 9 files changed, 374 insertions(+), 265 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 692dbc8..775e39a 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -1490,6 +1490,7 @@ add_subdirectory(src/kudu/rpc)
 add_subdirectory(src/kudu/security)
 add_subdirectory(src/kudu/sentry)
 add_subdirectory(src/kudu/server)
+add_subdirectory(src/kudu/subprocess)
 add_subdirectory(src/kudu/tablet)
 add_subdirectory(src/kudu/thrift)
 add_subdirectory(src/kudu/tools)
diff --git a/src/kudu/subprocess/CMakeLists.txt b/src/kudu/subprocess/CMakeLists.txt
new file mode 100644
index 0000000..6637c82
--- /dev/null
+++ b/src/kudu/subprocess/CMakeLists.txt
@@ -0,0 +1,30 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+#######################################
+# kudu_subprocess
+#######################################
+
+add_library(kudu_subprocess
+  subprocess_protocol.cc
+)
+target_link_libraries(kudu_subprocess
+  gutil
+  kudu_util
+  tool_proto
+  ${KUDU_BASE_LIBS}
+)
diff --git a/src/kudu/subprocess/subprocess_protocol.cc b/src/kudu/subprocess/subprocess_protocol.cc
new file mode 100644
index 0000000..f4f8540
--- /dev/null
+++ b/src/kudu/subprocess/subprocess_protocol.cc
@@ -0,0 +1,217 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "kudu/subprocess/subprocess_protocol.h"
+
+#include <errno.h>
+#include <stddef.h>
+#include <stdint.h>
+#include <unistd.h>
+
+#include <ostream>
+#include <string>
+
+#include <glog/logging.h>
+#include <google/protobuf/util/json_util.h>
+
+#include "kudu/gutil/endian.h"
+#include "kudu/gutil/strings/substitute.h"
+#include "kudu/tools/tool.pb.h"  // IWYU pragma: keep
+#include "kudu/util/faststring.h"
+#include "kudu/util/pb_util.h"
+#include "kudu/util/status.h"
+
+using kudu::pb_util::SecureDebugString;
+using strings::Substitute;
+using std::string;
+
+namespace kudu {
+namespace subprocess {
+
+const int SubprocessProtocol::kMaxMessageBytes = 1024 * 1024;
+
+SubprocessProtocol::SubprocessProtocol(SerializationMode serialization_mode,
+                                       CloseMode close_mode,
+                                       int read_fd,
+                                       int write_fd,
+                                       int max_msg_bytes)
+    : serialization_mode_(serialization_mode),
+      close_mode_(close_mode),
+      read_fd_(read_fd),
+      write_fd_(write_fd),
+      max_msg_bytes_(max_msg_bytes) {
+}
+
+SubprocessProtocol::~SubprocessProtocol() {
+  if (close_mode_ == CloseMode::CLOSE_ON_DESTROY) {
+    int ret;
+    RETRY_ON_EINTR(ret, close(read_fd_));
+    RETRY_ON_EINTR(ret, close(write_fd_));
+  }
+}
+
+template <class M>
+Status SubprocessProtocol::ReceiveMessage(M* message) {
+  switch (serialization_mode_) {
+    case SerializationMode::JSON:
+    {
+      // Read and accumulate one byte at a time, looking for the newline.
+      //
+      // TODO(adar): it would be more efficient to read a chunk of data, look
+      // for a newline, and if found, store the remainder for the next message.
+      faststring buf;
+      faststring one_byte;
+      one_byte.resize(1);
+      while (true) {
+        RETURN_NOT_OK_PREPEND(DoRead(&one_byte), "unable to receive message byte");
+        if (one_byte[0] == '\n') {
+          break;
+        }
+        buf.push_back(one_byte[0]);
+      }
+
+      // Parse the JSON-encoded message.
+      const auto& google_status =
+          google::protobuf::util::JsonStringToMessage(buf.ToString(), message);
+      if (!google_status.ok()) {
+        return Status::InvalidArgument(
+            Substitute("unable to parse JSON: $0", buf.ToString()),
+            google_status.error_message().ToString());
+      }
+      break;
+    }
+    case SerializationMode::PB:
+    {
+      // Read four bytes of size (big-endian).
+      faststring size_buf;
+      size_buf.resize(sizeof(uint32_t));
+      RETURN_NOT_OK_PREPEND(DoRead(&size_buf), "unable to receive message size");
+      uint32_t body_size = NetworkByteOrder::Load32(size_buf.data());
+
+      if (body_size > max_msg_bytes_) {
+        return Status::IOError(
+            Substitute("message size ($0) exceeds maximum message size ($1)",
+                       body_size, max_msg_bytes_));
+      }
+
+      // Read the variable size body.
+      faststring body_buf;
+      body_buf.resize(body_size);
+      RETURN_NOT_OK_PREPEND(DoRead(&body_buf), "unable to receive message body");
+
+      // Parse the body into a PB request.
+      RETURN_NOT_OK_PREPEND(pb_util::ParseFromArray(
+          message, body_buf.data(), body_buf.length()),
+              Substitute("unable to parse PB: $0", body_buf.ToString()));
+      break;
+    }
+    default: LOG(FATAL) << "Unknown mode";
+  }
+
+  VLOG(1) << "Received message: " << pb_util::SecureDebugString(*message);
+  return Status::OK();
+}
+
+template <class M>
+Status SubprocessProtocol::SendMessage(const M& message) {
+  VLOG(1) << "Sending message: " << pb_util::SecureDebugString(message);
+
+  faststring buf;
+  switch (serialization_mode_) {
+    case SerializationMode::JSON:
+    {
+      string serialized;
+      const auto& google_status =
+          google::protobuf::util::MessageToJsonString(message, &serialized);
+      if (!google_status.ok()) {
+        return Status::InvalidArgument(Substitute(
+            "unable to serialize JSON: $0", pb_util::SecureDebugString(message)),
+                                       google_status.error_message().ToString());
+      }
+
+      buf.append(serialized);
+      buf.append("\n");
+      break;
+    }
+    case SerializationMode::PB:
+    {
+      size_t msg_size = message.ByteSizeLong();
+      buf.resize(sizeof(uint32_t) + msg_size);
+      NetworkByteOrder::Store32(buf.data(), msg_size);
+      if (!message.SerializeWithCachedSizesToArray(buf.data() + sizeof(uint32_t))) {
+        return Status::Corruption("failed to serialize PB to array");
+      }
+      break;
+    }
+    default:
+      break;
+  }
+  RETURN_NOT_OK_PREPEND(DoWrite(buf), "unable to send message");
+  return Status::OK();
+}
+
+Status SubprocessProtocol::DoRead(faststring* buf) {
+  uint8_t* pos = buf->data();
+  size_t rem = buf->length();
+  while (rem > 0) {
+    ssize_t r;
+    RETRY_ON_EINTR(r, read(read_fd_, pos, rem));
+    if (r == -1) {
+      return Status::IOError("Error reading from pipe", "", errno);
+    }
+    if (r == 0) {
+      return Status::EndOfFile("Other end of pipe was closed");
+    }
+    DCHECK_GE(rem, r);
+    rem -= r;
+    pos += r;
+  }
+  return Status::OK();
+}
+
+Status SubprocessProtocol::DoWrite(const faststring& buf) {
+  const uint8_t* pos = buf.data();
+  size_t rem = buf.length();
+  while (rem > 0) {
+    ssize_t r;
+    RETRY_ON_EINTR(r, write(write_fd_, pos, rem));
+    if (r == -1) {
+      if (errno == EPIPE) {
+        return Status::EndOfFile("Other end of pipe was closed");
+      }
+      return Status::IOError("Error writing to pipe", "", errno);
+    }
+    DCHECK_GE(rem, r);
+    rem -= r;
+    pos += r;
+  }
+  return Status::OK();
+}
+
+
+// Explicit specialization for callers outside this compilation unit.
+template
+Status SubprocessProtocol::ReceiveMessage(tools::ControlShellRequestPB* message);
+template
+Status SubprocessProtocol::ReceiveMessage(tools::ControlShellResponsePB* message);
+template
+Status SubprocessProtocol::SendMessage(const tools::ControlShellRequestPB& message);
+template
+Status SubprocessProtocol::SendMessage(const tools::ControlShellResponsePB& message);
+
+} // namespace subprocess
+} // namespace kudu
diff --git a/src/kudu/subprocess/subprocess_protocol.h b/src/kudu/subprocess/subprocess_protocol.h
new file mode 100644
index 0000000..22efce4
--- /dev/null
+++ b/src/kudu/subprocess/subprocess_protocol.h
@@ -0,0 +1,96 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include "kudu/gutil/macros.h"
+#include "kudu/util/status.h"
+
+namespace kudu {
+
+class faststring; // NOLINT
+
+namespace subprocess {
+// Facilitates sending and receiving messages with a subprocess via protobuf-based
+// protocol.
+//
+// May be used by a subprocess communicating with the parent process via pipe, or
+// by the parent process itself to read/write messages via stdin/stdout respectively.
+class SubprocessProtocol {
+ public:
+  enum class SerializationMode {
+    // Each message is serialized as a four byte big-endian size followed by
+    // the protobuf-encoded message itself.
+    PB,
+
+    // Each message is serialized into a protobuf-like JSON representation
+    // terminated with a newline character.
+    JSON,
+  };
+
+  // Whether the provided fds are closed at class destruction time.
+  enum class CloseMode {
+    CLOSE_ON_DESTROY,
+    NO_CLOSE_ON_DESTROY,
+  };
+
+  // Constructs a new protocol instance.
+  //
+  // If 'close_mode' is CLOSE_ON_DESTROY, the instance has effectively taken
+  // control of 'read_fd' and 'write_fd' and the caller shouldn't use them.
+  // 'max_msg_bytes' represents the maximum number of bytes per message.
+  SubprocessProtocol(SerializationMode serialization_mode,
+                     CloseMode close_mode,
+                     int read_fd,
+                     int write_fd,
+                     int max_msg_bytes = kMaxMessageBytes);
+
+  ~SubprocessProtocol();
+
+  // Receives a protobuf message, blocking if the pipe is empty.
+  //
+  // Returns EndOfFile if the writer on the other end of the pipe was closed.
+  //
+  // Returns an error if serialization_mode_ is PB and the received message
+  // sizes exceeds kMaxMessageBytes.
+  template <class M>
+  Status ReceiveMessage(M* message);
+
+  // Sends a protobuf message, blocking if the pipe is full.
+  //
+  // Returns EndOfFile if the reader on the other end of the pipe was closed.
+  template <class M>
+  Status SendMessage(const M& message);
+
+ private:
+  // Private helpers to drive actual pipe reading and writing.
+  Status DoRead(faststring* buf);
+  Status DoWrite(const faststring& buf);
+
+  static const int kMaxMessageBytes;
+
+  const SerializationMode serialization_mode_;
+  const CloseMode close_mode_;
+  const int read_fd_;
+  const int write_fd_;
+  const int max_msg_bytes_;
+
+  DISALLOW_COPY_AND_ASSIGN(SubprocessProtocol);
+};
+
+} // namespace subprocess
+} // namespace kudu
diff --git a/src/kudu/tools/CMakeLists.txt b/src/kudu/tools/CMakeLists.txt
index face315..84215c2 100644
--- a/src/kudu/tools/CMakeLists.txt
+++ b/src/kudu/tools/CMakeLists.txt
@@ -129,6 +129,7 @@ target_link_libraries(kudu
   kudu_client
   kudu_common
   kudu_fs
+  kudu_subprocess
   kudu_tools_rebalance
   kudu_util
   log
@@ -165,6 +166,7 @@ SET_KUDU_TEST_LINK_LIBS(
   itest_util
   ksck
   kudu_hms
+  kudu_subprocess
   kudu_tools_rebalance
   kudu_tools_test_util
   kudu_tools_util
diff --git a/src/kudu/tools/kudu-tool-test.cc b/src/kudu/tools/kudu-tool-test.cc
index bd4ec0b..5a77955 100644
--- a/src/kudu/tools/kudu-tool-test.cc
+++ b/src/kudu/tools/kudu-tool-test.cc
@@ -99,6 +99,7 @@
 #include "kudu/mini-cluster/internal_mini_cluster.h"
 #include "kudu/mini-cluster/mini_cluster.h"
 #include "kudu/rpc/rpc_controller.h"
+#include "kudu/subprocess/subprocess_protocol.h"
 #include "kudu/tablet/local_tablet_writer.h"
 #include "kudu/tablet/metadata.pb.h"
 #include "kudu/tablet/tablet-harness.h"
@@ -108,7 +109,6 @@
 #include "kudu/tablet/tablet_replica.h"
 #include "kudu/thrift/client.h"
 #include "kudu/tools/tool.pb.h"
-#include "kudu/tools/tool_action_common.h"
 #include "kudu/tools/tool_replica_util.h"
 #include "kudu/tools/tool_test_util.h"
 #include "kudu/tserver/mini_tablet_server.h"
@@ -185,6 +185,7 @@ using kudu::master::MiniMaster;
 using kudu::master::TServerStatePB;
 using kudu::master::TSManager;
 using kudu::rpc::RpcController;
+using kudu::subprocess::SubprocessProtocol;
 using kudu::tablet::LocalTabletWriter;
 using kudu::tablet::Tablet;
 using kudu::tablet::TabletDataState;
@@ -4446,7 +4447,7 @@ TEST_F(ToolTest, TestHmsList) {
 // This test is parameterized on the serialization mode and Kerberos.
 class ControlShellToolTest :
     public ToolTest,
-    public ::testing::WithParamInterface<std::tuple<ControlShellProtocol::SerializationMode,
+    public ::testing::WithParamInterface<std::tuple<SubprocessProtocol::SerializationMode,
                                                     bool>> {
  public:
   virtual void SetUp() override {
@@ -4455,8 +4456,8 @@ class ControlShellToolTest :
     // Start the control shell.
     string mode;
     switch (serde_mode()) {
-      case ControlShellProtocol::SerializationMode::JSON: mode = "json"; break;
-      case ControlShellProtocol::SerializationMode::PB: mode = "pb"; break;
+      case SubprocessProtocol::SerializationMode::JSON: mode = "json"; break;
+      case SubprocessProtocol::SerializationMode::PB: mode = "pb"; break;
       default: LOG(FATAL) << "Unknown serialization mode";
     }
     shell_.reset(new Subprocess({
@@ -4470,10 +4471,10 @@ class ControlShellToolTest :
     ASSERT_OK(shell_->Start());
 
     // Start the protocol interface.
-    proto_.reset(new ControlShellProtocol(serde_mode(),
-                                          ControlShellProtocol::CloseMode::CLOSE_ON_DESTROY,
-                                          shell_->ReleaseChildStdoutFd(),
-                                          shell_->ReleaseChildStdinFd()));
+    proto_.reset(new SubprocessProtocol(serde_mode(),
+                                        SubprocessProtocol::CloseMode::CLOSE_ON_DESTROY,
+                                        shell_->ReleaseChildStdoutFd(),
+                                        shell_->ReleaseChildStdinFd()));
   }
 
   virtual void TearDown() override {
@@ -4500,7 +4501,7 @@ class ControlShellToolTest :
     return Status::OK();
   }
 
-  ControlShellProtocol::SerializationMode serde_mode() const {
+  SubprocessProtocol::SerializationMode serde_mode() const {
     return ::testing::get<0>(GetParam());
   }
 
@@ -4509,13 +4510,13 @@ class ControlShellToolTest :
   }
 
   unique_ptr<Subprocess> shell_;
-  unique_ptr<ControlShellProtocol> proto_;
+  unique_ptr<SubprocessProtocol> proto_;
 };
 
 INSTANTIATE_TEST_CASE_P(SerializationModes, ControlShellToolTest,
                         ::testing::Combine(::testing::Values(
-                            ControlShellProtocol::SerializationMode::PB,
-                            ControlShellProtocol::SerializationMode::JSON),
+                            SubprocessProtocol::SerializationMode::PB,
+                            SubprocessProtocol::SerializationMode::JSON),
                                            ::testing::Bool()));
 
 TEST_P(ControlShellToolTest, TestControlShell) {
diff --git a/src/kudu/tools/tool_action_common.cc b/src/kudu/tools/tool_action_common.cc
index bd9855d..6b17cc8 100644
--- a/src/kudu/tools/tool_action_common.cc
+++ b/src/kudu/tools/tool_action_common.cc
@@ -18,10 +18,8 @@
 #include "kudu/tools/tool_action_common.h"
 
 #include <stdlib.h>
-#include <unistd.h>
 
 #include <algorithm>
-#include <cerrno>
 #include <iomanip>
 #include <iostream>
 #include <iterator>
@@ -35,7 +33,6 @@
 #include <boost/algorithm/string/predicate.hpp>
 #include <gflags/gflags.h>
 #include <glog/logging.h>
-#include <google/protobuf/util/json_util.h>
 // IWYU pragma: no_include <yaml-cpp/node/impl.h>
 // IWYU pragma: no_include <yaml-cpp/node/node.h>
 
@@ -52,7 +49,6 @@
 #include "kudu/consensus/log.pb.h"
 #include "kudu/consensus/log_util.h"
 #include "kudu/consensus/opid.pb.h"
-#include "kudu/gutil/endian.h"
 #include "kudu/gutil/map-util.h"
 #include "kudu/gutil/ref_counted.h"
 #include "kudu/gutil/strings/join.h"
@@ -76,7 +72,6 @@
 #include "kudu/tserver/tserver_service.proxy.h" // IWYU pragma: keep
 #include "kudu/util/async_util.h"
 #include "kudu/util/env.h"
-#include "kudu/util/faststring.h"
 #include "kudu/util/jsonwriter.h"
 #include "kudu/util/mem_tracker.pb.h"
 #include "kudu/util/memory/arena.h"
@@ -859,173 +854,5 @@ Status LeaderMasterProxy::SyncRpc(
                                RpcController*,
                                const ResponseCallback&)>& func);
 
-const int ControlShellProtocol::kMaxMessageBytes = 1024 * 1024;
-
-ControlShellProtocol::ControlShellProtocol(SerializationMode serialization_mode,
-                                           CloseMode close_mode,
-                                           int read_fd,
-                                           int write_fd)
-    : serialization_mode_(serialization_mode),
-      close_mode_(close_mode),
-      read_fd_(read_fd),
-      write_fd_(write_fd) {
-}
-
-ControlShellProtocol::~ControlShellProtocol() {
-  if (close_mode_ == CloseMode::CLOSE_ON_DESTROY) {
-    int ret;
-    RETRY_ON_EINTR(ret, close(read_fd_));
-    RETRY_ON_EINTR(ret, close(write_fd_));
-  }
-}
-
-template <class M>
-Status ControlShellProtocol::ReceiveMessage(M* message) {
-  switch (serialization_mode_) {
-    case SerializationMode::JSON:
-    {
-      // Read and accumulate one byte at a time, looking for the newline.
-      //
-      // TODO(adar): it would be more efficient to read a chunk of data, look
-      // for a newline, and if found, store the remainder for the next message.
-      faststring buf;
-      faststring one_byte;
-      one_byte.resize(1);
-      while (true) {
-        RETURN_NOT_OK_PREPEND(DoRead(&one_byte), "unable to receive message byte");
-        if (one_byte[0] == '\n') {
-          break;
-        }
-        buf.push_back(one_byte[0]);
-      }
-
-      // Parse the JSON-encoded message.
-      const auto& google_status =
-          google::protobuf::util::JsonStringToMessage(buf.ToString(), message);
-      if (!google_status.ok()) {
-        return Status::InvalidArgument(
-            Substitute("unable to parse JSON: $0", buf.ToString()),
-            google_status.error_message().ToString());
-      }
-      break;
-    }
-    case SerializationMode::PB:
-    {
-      // Read four bytes of size (big-endian).
-      faststring size_buf;
-      size_buf.resize(sizeof(uint32_t));
-      RETURN_NOT_OK_PREPEND(DoRead(&size_buf), "unable to receive message size");
-      uint32_t body_size = NetworkByteOrder::Load32(size_buf.data());
-
-      if (body_size > kMaxMessageBytes) {
-        return Status::IOError(
-            Substitute("message size ($0) exceeds maximum message size ($1)",
-                       body_size, kMaxMessageBytes));
-      }
-
-      // Read the variable size body.
-      faststring body_buf;
-      body_buf.resize(body_size);
-      RETURN_NOT_OK_PREPEND(DoRead(&body_buf), "unable to receive message body");
-
-      // Parse the body into a PB request.
-      RETURN_NOT_OK_PREPEND(pb_util::ParseFromArray(
-          message, body_buf.data(), body_buf.length()),
-                            Substitute("unable to parse PB: $0", body_buf.ToString()));
-      break;
-    }
-    default: LOG(FATAL) << "Unknown mode";
-  }
-
-  VLOG(1) << "Received message: " << pb_util::SecureDebugString(*message);
-  return Status::OK();
-}
-
-template <class M>
-Status ControlShellProtocol::SendMessage(const M& message) {
-  VLOG(1) << "Sending message: " << pb_util::SecureDebugString(message);
-
-  faststring buf;
-  switch (serialization_mode_) {
-    case SerializationMode::JSON:
-    {
-      string serialized;
-      const auto& google_status =
-          google::protobuf::util::MessageToJsonString(message, &serialized);
-      if (!google_status.ok()) {
-        return Status::InvalidArgument(Substitute(
-            "unable to serialize JSON: $0", pb_util::SecureDebugString(message)),
-                                       google_status.error_message().ToString());
-      }
-
-      buf.append(serialized);
-      buf.append("\n");
-      break;
-    }
-    case SerializationMode::PB:
-    {
-      size_t msg_size = message.ByteSizeLong();
-      buf.resize(sizeof(uint32_t) + msg_size);
-      NetworkByteOrder::Store32(buf.data(), msg_size);
-      if (!message.SerializeWithCachedSizesToArray(buf.data() + sizeof(uint32_t))) {
-        return Status::Corruption("failed to serialize PB to array");
-      }
-      break;
-    }
-    default:
-      break;
-  }
-  RETURN_NOT_OK_PREPEND(DoWrite(buf), "unable to send message");
-  return Status::OK();
-}
-
-Status ControlShellProtocol::DoRead(faststring* buf) {
-  uint8_t* pos = buf->data();
-  size_t rem = buf->length();
-  while (rem > 0) {
-    ssize_t r;
-    RETRY_ON_EINTR(r, read(read_fd_, pos, rem));
-    if (r == -1) {
-      return Status::IOError("Error reading from pipe", "", errno);
-    }
-    if (r == 0) {
-      return Status::EndOfFile("Other end of pipe was closed");
-    }
-    DCHECK_GE(rem, r);
-    rem -= r;
-    pos += r;
-  }
-  return Status::OK();
-}
-
-Status ControlShellProtocol::DoWrite(const faststring& buf) {
-  const uint8_t* pos = buf.data();
-  size_t rem = buf.length();
-  while (rem > 0) {
-    ssize_t r;
-    RETRY_ON_EINTR(r, write(write_fd_, pos, rem));
-    if (r == -1) {
-      if (errno == EPIPE) {
-        return Status::EndOfFile("Other end of pipe was closed");
-      }
-      return Status::IOError("Error writing to pipe", "", errno);
-    }
-    DCHECK_GE(rem, r);
-    rem -= r;
-    pos += r;
-  }
-  return Status::OK();
-}
-
-// Explicit specialization for callers outside this compilation unit.
-template
-Status ControlShellProtocol::ReceiveMessage(ControlShellRequestPB* message);
-template
-Status ControlShellProtocol::ReceiveMessage(ControlShellResponsePB* message);
-template
-Status ControlShellProtocol::SendMessage(const ControlShellRequestPB& message);
-template
-Status ControlShellProtocol::SendMessage(const ControlShellResponsePB& message);
-
 } // namespace tools
 } // namespace kudu
diff --git a/src/kudu/tools/tool_action_common.h b/src/kudu/tools/tool_action_common.h
index 8c5ba9a..c68e885 100644
--- a/src/kudu/tools/tool_action_common.h
+++ b/src/kudu/tools/tool_action_common.h
@@ -24,7 +24,6 @@
 #include <vector>
 
 #include "kudu/client/shared_ptr.h"
-#include "kudu/gutil/macros.h"
 #include "kudu/gutil/port.h"
 #include "kudu/gutil/ref_counted.h"
 #include "kudu/rpc/response_callback.h"
@@ -38,7 +37,6 @@ class function;
 namespace kudu {
 
 class MonoDelta;
-class faststring; // NOLINT
 
 namespace client {
 class KuduClient;
@@ -259,68 +257,5 @@ class LeaderMasterProxy {
   client::sp::shared_ptr<client::KuduClient> client_;
 };
 
-// Facilitates sending and receiving messages with the tool control shell.
-//
-// May be used by a subprocess communicating with the shell via pipe, or by the
-// shell itself to read/write messages via stdin/stdout respectively.
-class ControlShellProtocol {
- public:
-  enum class SerializationMode {
-    // Each message is serialized as a four byte big-endian size followed by
-    // the protobuf-encoded message itself.
-    PB,
-
-    // Each message is serialized into a protobuf-like JSON representation
-    // terminated with a newline character.
-    JSON,
-  };
-
-  // Whether the provided fds are closed at class destruction time.
-  enum class CloseMode {
-    CLOSE_ON_DESTROY,
-    NO_CLOSE_ON_DESTROY,
-  };
-
-  // Constructs a new protocol instance.
-  //
-  // If 'close_mode' is CLOSE_ON_DESTROY, the instance has effectively taken
-  // control of 'read_fd' and 'write_fd' and the caller shouldn't use them.
-  ControlShellProtocol(SerializationMode serialization_mode,
-                       CloseMode close_mode,
-                       int read_fd,
-                       int write_fd);
-
-  ~ControlShellProtocol();
-
-  // Receives a protobuf message, blocking if the pipe is empty.
-  //
-  // Returns EndOfFile if the writer on the other end of the pipe was closed.
-  //
-  // Returns an error if serialization_mode_ is PB and the received message
-  // sizes exceeds kMaxMessageBytes.
-  template <class M>
-  Status ReceiveMessage(M* message);
-
-  // Sends a protobuf message, blocking if the pipe is full.
-  //
-  // Returns EndOfFile if the reader on the other end of the pipe was closed.
-  template <class M>
-  Status SendMessage(const M& message);
-
- private:
-  // Private helpers to drive actual pipe reading and writing.
-  Status DoRead(faststring* buf);
-  Status DoWrite(const faststring& buf);
-
-  static const int kMaxMessageBytes;
-
-  const SerializationMode serialization_mode_;
-  const CloseMode close_mode_;
-  const int read_fd_;
-  const int write_fd_;
-
-  DISALLOW_COPY_AND_ASSIGN(ControlShellProtocol);
-};
-
 } // namespace tools
 } // namespace kudu
diff --git a/src/kudu/tools/tool_action_test.cc b/src/kudu/tools/tool_action_test.cc
index da0de8d..e775e9c 100644
--- a/src/kudu/tools/tool_action_test.cc
+++ b/src/kudu/tools/tool_action_test.cc
@@ -38,8 +38,8 @@
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/mini-cluster/external_mini_cluster.h"
 #include "kudu/security/test/mini_kdc.h"
+#include "kudu/subprocess/subprocess_protocol.h"
 #include "kudu/tools/tool.pb.h"
-#include "kudu/tools/tool_action_common.h"
 #include "kudu/util/env.h"
 #include "kudu/util/env_util.h"
 #include "kudu/util/path_util.h"
@@ -56,17 +56,17 @@ DEFINE_validator(serialization, [](const char* /*n*/, const std::string& v) {
          boost::iequals(v, "json");
 });
 
-namespace kudu {
-
-namespace tools {
-
-using cluster::ExternalDaemon;
-using cluster::ExternalMiniCluster;
-using cluster::ExternalMiniClusterOptions;
+using kudu::cluster::ExternalDaemon;
+using kudu::cluster::ExternalMiniCluster;
+using kudu::cluster::ExternalMiniClusterOptions;
+using kudu::subprocess::SubprocessProtocol;
 using std::string;
 using std::unique_ptr;
 using strings::Substitute;
 
+namespace kudu {
+namespace tools {
+
 namespace {
 
 Status MakeClusterRoot(string* cluster_root) {
@@ -322,17 +322,17 @@ Status RunControlShell(const RunnerContext& /*context*/) {
   int ret;
   RETRY_ON_EINTR(ret, dup2(STDERR_FILENO, STDOUT_FILENO));
   PCHECK(ret == STDOUT_FILENO);
-  ControlShellProtocol::SerializationMode serde_mode;
+  SubprocessProtocol::SerializationMode serde_mode;
   if (boost::iequals(FLAGS_serialization, "json")) {
-    serde_mode = ControlShellProtocol::SerializationMode::JSON;
+    serde_mode = SubprocessProtocol::SerializationMode::JSON;
   } else {
     DCHECK(boost::iequals(FLAGS_serialization, "pb"));
-    serde_mode = ControlShellProtocol::SerializationMode::PB;
+    serde_mode = SubprocessProtocol::SerializationMode::PB;
   }
-  ControlShellProtocol protocol(serde_mode,
-                                ControlShellProtocol::CloseMode::NO_CLOSE_ON_DESTROY,
-                                STDIN_FILENO,
-                                new_stdout);
+  SubprocessProtocol protocol(serde_mode,
+                              SubprocessProtocol::CloseMode::NO_CLOSE_ON_DESTROY,
+                              STDIN_FILENO,
+                              new_stdout);
 
   // Run the shell loop, processing each message as it is received.
   unique_ptr<ExternalMiniCluster> cluster;


[kudu] 01/02: [docs] gradle-related update on RELEASING.adoc

Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit 8369380c880846a590d6a779e056293005de3a4e
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Tue Oct 15 01:13:07 2019 -0700

    [docs] gradle-related update on RELEASING.adoc
    
    This patch updates gradle-related instructions for building the release
    candidate.  Also, added a section about forwarding gpg-agent to a remote
    system over SSH and other security-related tips.  Fixed formatting.
    Updated the paragraph about staging maven repository with RC artifacts.
    
    Change-Id: I861d30615e53d5d6c76bbd01da13e3f2a540e882
    Reviewed-on: http://gerrit.cloudera.org:8080/14443
    Tested-by: Kudu Jenkins
    Reviewed-by: Grant Henke <gr...@apache.org>
---
 RELEASING.adoc | 176 +++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 114 insertions(+), 62 deletions(-)

diff --git a/RELEASING.adoc b/RELEASING.adoc
index fba9f23..f27d2ea 100644
--- a/RELEASING.adoc
+++ b/RELEASING.adoc
@@ -26,6 +26,7 @@ releases] prior to conducting the release management activity. Pay attention to
 the link:https://www.apache.org/legal/resolved.html[3rd-party license policy],
 making sure the release doesn't include anything from the
 link:https://www.apache.org/legal/resolved.html#category-x[Category X].
+The command-line snippets in this document are for `bash` (Bourne-again shell).
 To edit or update this document, edit `RELEASING.adoc` in `master`.
 
 == Stating Intent to Release
@@ -71,11 +72,13 @@ To edit or update this document, edit `RELEASING.adoc` in `master`.
   configuration. Cloudera hosts the Gerrit server and a Cloudera employee will
   have to perform this step because SSH access is behind a firewall. The steps
   are as follows:
-  1. Ensure your public SSH key is in `~gerrit/.ssh/authorized_keys` on gerrit.cloudera.org
+  1. Ensure your public SSH key is in `~gerrit/.ssh/authorized_keys` on
+     gerrit.cloudera.org
   2. From behind the firewall, `ssh gerrit@gerrit.cloudera.org` to log in.
   3. Back up the existing replication configuration file by executing
      `cp ~/etc/replication.config ~/etc/replication.config.bak.`date '+%Y%m%d.%H%M%S'``
-  4. Edit `etc/replication.config` to add a line for the new branch, such as `branch-1.x.y`
+  4. Edit `etc/replication.config` to add a line for the new branch, such as
+     `branch-1.x.y`
   5. Send email to the dev lists for Kudu and Impala (dev@kudu.apache.org and
      dev@impala.apache.org) indicating that you are going to restart Gerrit
      (link:https://s.apache.org/2Wj7[example]). It is best to do the restart at
@@ -88,23 +91,53 @@ To edit or update this document, edit `RELEASING.adoc` in `master`.
 
 . As needed, patches can be cherry-picked to the new branch.
 
-== Updating Versions In Master
+== Updating Versions in Master
 
-. Check out the `master` branch and bump the version in `version.txt`.
+. Check out the `master` branch and bump the version in `version.txt`. Don't
+  update `kudu-version` in `examples/java/java-example/pom.xml` yet: it should
+  be updated later on when release artifacts are published (see below).
 
 . Commit and push that change to Gerrit.
 
-. Notify dev@kudu.apache.org that the new branch is available.
+. Notify dev@kudu.apache.org that the new branch is available (see
+  link:https://lists.apache.org/thread.html/de58960366583943391c00bd6b75dbd1fab3bc9067af05dd7b817a90@%3Cdev.kudu.apache.org%3E[here]
+  for an example).
 
-
-== Preparing A Release Candidate
+== Preparing a Release Candidate
 
 . Before building a release candidate, make sure you have followed the
-Apache committer guide for setting up your GPG keys
-(link:https://www.apache.org/dev/new-committers-guide.html#set-up-security-and-pgp-keys[here]).
+  Apache committer guide for
+  link:https://www.apache.org/dev/new-committers-guide.html#set-up-security-and-pgp-keys[
+  setting up your GPG keys]. In addition to the link:http://pgp.mit.edu/[MIT
+  PGP public key server] referenced in the committer guide, consider adding
+  your keys to one of link:http://pool.sks-keyservers.net:11371[SKS OpenPGP
+  keyservers] or link:http://keyserver.ubuntu.com:11371[Ubuntu OpenPGP
+  keyserver]. The MIT keyserver sometimes is not available, and it takes some
+  time to propagate key updates anyways: the latter two servers are used by the
+  link:https://repository.apache.org[Apache Maven repo server] to verify the
+  signature of the uploaded maven artifacts as of October 2019.
+
+. If building and signing on a remote/shared machine, consider
+  link:https://wiki.gnupg.org/AgentForwarding[forwarding GPG agent via SSH].
+  That means you can keep your secret keys on a local machine even when signing
+  the artifacts to be released
+  (works even for a hardware token like a smartcard, etc.).
+
+. Out of the Kudu git workspace, checkout the
+  link:https://dist.apache.org/repos/dist/release/kudu/[release SVN repository].
+  Later on, the officially released and signed artifacts will be put into
+  this repository. At this point, just add your PGP key to the `KEYS` file
+  (if it's not there yet), making it available for the signature verification:
++
+----
+svn co https://dist.apache.org/repos/dist/release/kudu/ kudu-dist-release
+cd kudu-dist-release
+(gpg --list-sigs <your-email-address> && gpg --armor --export <your-email-address>) >> KEYS
+svn commit --username=<your_apache_username> -m "Adding my key to the KEYS file"
+----
 
 . When close to building a release candidate, try building a source tarball
-(on a supported platform):
+  (on a supported platform):
 +
 ----
   ./build-support/build_source_release.py
@@ -112,16 +145,25 @@ Apache committer guide for setting up your GPG keys
 
 . Fix any issues it finds, such as RAT.
 
+. Make sure `kudu-binary` JAR artifact can be successfully built both on Linux
+  and macOS:
++
+----
+  ./build-support/mini-cluster/build_mini_cluster_binaries.sh
+----
+
 . Test the full Java build. This will sign and build everything without
   deploying any artifacts:
 +
 ----
   # Run a gpg-agent if you don't normally.
   gpg-agent --daemon
+  # List keys with identifiers in the traditional 8-character key ID format.
+  # Take a note of the identifier of the key you want to use for signing.
+  gpg --list-secret-keys --keyid-format=short
   cd java
-  gradle clean install -PforceSigning
+  ./gradlew clean install -PforceSigning -Psigning.gnupg.keyName=<8-character-pgp-key-id>
 ----
-+
 
 . Create a new version update commit which removes the -SNAPSHOT suffix (same
   process as above).
@@ -142,17 +184,18 @@ Apache committer guide for setting up your GPG keys
 
 . Build a source tarball against the RC branch.
 
-. Create a new folder containing the
-  link:https://dist.apache.org/repos/dist/dev/kudu/[dev Subversion (SVN)
-  repository]. Copy the artifacts to this folder and commit.
+. Out of the Kudu git workspace, checkout the
+  link:https://dist.apache.org/repos/dist/dev/kudu/[dev Subversion (SVN) repository].
+  Create a new sub-directory named correspondingly. Copy the artifacts to this
+  sub-directory and commit.
 +
 ----
   svn co --depth=immediates https://dist.apache.org/repos/dist/dev/kudu/ kudu-dev-release
   cd kudu-dev-release
   mkdir 1.x.y-RC1
-  cp <path_to_kudu>/build/apache-kudu-1.x.y.tar.* 1.x.y-RC1
-  svn add 1.x.y-RC1/*
-  svn commit -m "Adding Kudu 1.x.y RC1"
+  cp <path_to_kudu_git_workspace>/build/apache-kudu-1.x.y.tar.* 1.x.y-RC1
+  svn add 1.x.y-RC1
+  svn commit --username=<your_apache_username> -m "Adding Kudu 1.x.y RC1"
 ----
 
 . Create a Maven staging repository for the release candidate Java artifacts.
@@ -161,7 +204,15 @@ Apache committer guide for setting up your GPG keys
   # Run a gpg-agent if you don't normally
   gpg-agent --daemon
   cd java
-  gradle clean uploadArchives -PmavenUsername='<APACHE-LDAP-USERNAME>' -PmavenPassword='<APACHE-LDAP-PASSWORD>'
+  # Turn off bash history: this is to avoid exposing the credentials
+  # via .bash_history file.
+  set +o history
+  ./gradlew clean uploadArchives \
+      -Psigning.gnupg.keyName=<8-character-pgp-key-id> \
+      -PmavenUsername='<APACHE-LDAP-USERNAME>' \
+      -PmavenPassword='<APACHE-LDAP-PASSWORD>'
+  # Turn on bash history.
+  set -o history
 ----
 
 . Build and deploy new binary test JARs for the RC on macOS and Linux. Build
@@ -171,13 +222,22 @@ Apache committer guide for setting up your GPG keys
   need to build on an old version of macOS).
 +
 ----
-  # Build a binary JAR for the local operating system. The resulting JAR
-  # is output into the build/mini-cluster directory.
+  # Build a binary JAR for the local operating system. Make sure the thirdparty
+  # components were built to match the source code that the RC is being built
+  # with. It's a good idea to clone the Kudu git repo into a dedicated
+  # workspace, rebuilding the thirdparty compoments from scratch for particular
+  # release. The resulting JAR is output into the build/mini-cluster directory.
   ./build-support/mini-cluster/build_mini_cluster_binaries.sh
   # Sign and publish all matching kudu-binary artifacts from the
   # build/mini-cluster directory to the Maven staging repository that hosts
   # the Java artifacts of the Apache Kudu project (see above).
-  ./build-support/mini-cluster/publish_mini_cluster_binaries.sh -a=deploy -u='<APACHE-LDAP-USERNAME>' -p='<APACHE-LDAP-PASSWORD>'
+  # Turn off bash history: this is to avoid exposing the credentials persisted
+  # in .bash_history file.
+  set +o history
+  ./build-support/mini-cluster/publish_mini_cluster_binaries.sh -a=deploy \
+      -u='<APACHE-LDAP-USERNAME>' -p='<APACHE-LDAP-PASSWORD>'
+  # Turn bash history back on.
+  set -o history
 ----
 +
 NOTE: If the binary test JAR artifacts are deployed by the same person and from the
@@ -192,34 +252,22 @@ of the JAR file to sign and publish.
 
 . Close the Maven staging repository (or repositories).
 +
-The Maven staging repositories used for the Java and the binary test JAR
-artifacts must be closed in order to be accessible to people testing the RC.
-Go to the link:https://repository.apache.org/\#stagingRepositories[staging
-repository] and look for ‘orgapachekudu-####’ in the staging repositories list.
-You can check the ‘content’ tab at the bottom to make sure you have all of the
-expected stuff (client, various integrations, etc). Hit the checkbox next to
-your new staging repo(s) and hit “close”. Enter something similar to “Apache
-Kudu 1.x.y-RC1” into the description box and confirm. Wait a minute or two and
-hit refresh, and each closed staging repo should now have a URL shown in its
-summary tab, for example
-`https://repository.apache.org/content/repositories/orgapachekudu-1005`
-
-. Create a new folder containing the
-  link:https://dist.apache.org/repos/dist/release/kudu/[release SVN
-  repository]. For a release to be made official, it must eventually be put in
-  this repository. Add your PGP key to the KEYS file:
-+
-----
-svn co https://dist.apache.org/repos/dist/release/kudu/ kudu-dist-release
-cd kudu-dist-release
-(gpg --list-sigs <your-email-address> && gpg --armor --export <your-email-address>) >> KEYS
-svn commit -m "Adding my key to the KEYS file"
-----
+Go to the link:https://repository.apache.org/[repository manager] and log
+into the repository server using your Apache credentials. Make sure to enable
+Adobe Flash in your browser for this Web site. Now, go the
+link:https://repository.apache.org/\#stagingRepositories[staging repository]
+and look for ‘orgapachekudu-####’ in the staging repositories list. You can
+check the `Content` tab at the bottom to make sure you have all of the
+expected stuff (client, various integrations, etc.). Hit the checkbox next to
+your new staging repo and hit `Close`. Enter something similar to
+"Apache Kudu 1.x.y-RC1" into the description box and confirm. Wait a minute
+or two and hit `Refresh`, and your staging repo should now have a URL shown
+in its summary tab
+(e.g. `https://repository.apache.org/content/repositories/orgapachekudu-1005`)
 
 == Initiating a Vote for an RC
 
-. Send an email to dev@kudu.apache.org to start the RC process, using
-  this
+. Send an email to dev@kudu.apache.org to start the RC process, using this
   link:http://mail-archives.apache.org/mod_mbox/kudu-dev/201606.mbox/%3CCAGpTDNduoQM0ktuZc1eW1XeXCcXhvPGftJ%3DLRB8Er5c2dZptvw%40mail.gmail.com%3E[example]
   as a template.
 
@@ -237,42 +285,46 @@ svn commit -m "Adding my key to the KEYS file"
 
 == Release
 
-. Create a new folder in the release repository for the new release and copy
-  the files from the dev repository.
+. For a release to be made official, the result release candidate must be put
+  in the release SVN repository. Create a new sub-directory in the release SVN
+  repository for the new release and copy the files from the dev repository:
 +
 ----
   cd kudu-dist-release
   mkdir 1.x.y
   cp <path_to_kudu-dev-release>/1.x.y-RC1/* 1.x.y
   svn add 1.x.y
-  svn commit -m "Adding files for Kudu 1.x.y"
+  svn commit --username=<your_apache_username> -m "Adding files for Kudu 1.x.y"
 ----
 
-. In the Kudu git repo, create a signed tag from the RC’s tag, and push it to the
+. In the Kudu git repo, create a signed tag from the RC's tag, verify the
+  signature has been applied and verifiable, and push it to the
   Apache Git repository:
 +
 ----
-  git tag -s 1.x.y -m 'Release Apache Kudu 1.x.y' 1.x.y-RC1
+  gpg --list-secret-keys --keyid-format=short
+  git tag -u <gpg_key_id> -m 'Release Apache Kudu 1.x.y' 1.x.y 1.x.y-RC1
+  git tag -v 1.x.y
   git push apache 1.x.y
 ----
 
 . Release the staged Java artifacts. Select the release candidate staging
   repository in link:https://repository.apache.org/#stagingRepositories[Nexus],
-  and click 'Release'. You should shortly be able to see the artifacts in
+  and click `Release`. You should shortly be able to see the artifacts in
   link:https://search.maven.org/search?q=g:org.apache.kudu[Maven Central].
 
 . Release the Python artifacts. You will need to setup an account on link:https://PyPi.org[PyPi.org]
   and ask to be added to the kudu-python PyPi project if you have not done this before.
 +
 ----
-# Prepare and sign the python source distribution.
-cd python
-rm -rf dist/*
-python setup.py sdist
-gpg --detach-sign -a dist/kudu-python-1.x.y.tar.gz
-# Upload the distribution to PyPi using twine.
-pip install twine
-twine upload dist/*
+  # Prepare and sign the python source distribution.
+  cd python
+  rm -rf dist/*
+  python setup.py sdist
+  gpg --detach-sign -a dist/kudu-python-1.x.y.tar.gz
+  # Upload the distribution to PyPi using twine.
+  pip install twine
+  twine upload dist/*
 ----
 Note: You can upload to the test PyPi by adding
 `--repository-url https://test.pypi.org/legacy/` to the twine command.
@@ -285,7 +337,7 @@ Note: You can upload to the test PyPi by adding
   folders there, copy an `index.md` file from the previous release and modify it
   accordingly. Make sure the download page meets the current
   link:https://www.apache.org/dev/release-download-pages.html[criteria]. Base
-  it off the latest release which has the highest chance to comform the
+  it off the latest release which has the highest chance to conform to the
   requirements, but double-check the release pages document as the criteria
   keep changing and the announcement will be rejected if our release page
   doesn't meet the criteria.