You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by da...@apache.org on 2017/06/06 00:24:15 UTC

kudu git commit: [tools] Use PrintTable to format ksck's consensus matrix

Repository: kudu
Updated Branches:
  refs/heads/branch-1.4.x afcd412ba -> 9adbdd771


[tools] Use PrintTable to format ksck's consensus matrix

The first version of ksck's consensus matrix used its own table
code, but actually there was already nice table code available
in tool_action_common.h. This switches ksck to use it. It also
generalizes the table code to output to a generic ostream
instead of only cout; this was necessary for ksck-test but might
be useful in other ways later.

Change-Id: I8d77005f20091e6778702580e3a269d9689c5b0a
Reviewed-on: http://gerrit.cloudera.org:8080/7043
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert <da...@apache.org>
Reviewed-by: Mike Percy <mp...@apache.org>
(cherry picked from commit 30682fd13ffc68ebaeb8200e7450f8317b89bf85)
Reviewed-on: http://gerrit.cloudera.org:8080/7086


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

Branch: refs/heads/branch-1.4.x
Commit: 9adbdd7713ddeaa2361085ec49ea3a5506ef81b8
Parents: afcd412
Author: Will Berkeley <wd...@apache.org>
Authored: Thu Jun 1 10:18:15 2017 -0700
Committer: Will Berkeley <wd...@gmail.com>
Committed: Tue Jun 6 00:06:27 2017 +0000

----------------------------------------------------------------------
 src/kudu/tools/ksck-test.cc           |  36 ++++----
 src/kudu/tools/ksck.cc                | 144 ++++++++++-------------------
 src/kudu/tools/tool_action_common.cc  |  55 ++++++-----
 src/kudu/tools/tool_action_common.h   |   4 +-
 src/kudu/tools/tool_action_master.cc  |   4 +-
 src/kudu/tools/tool_action_tserver.cc |   4 +-
 6 files changed, 107 insertions(+), 140 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/9adbdd77/src/kudu/tools/ksck-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/ksck-test.cc b/src/kudu/tools/ksck-test.cc
index bdf1789..90a8f1d 100644
--- a/src/kudu/tools/ksck-test.cc
+++ b/src/kudu/tools/ksck-test.cc
@@ -406,12 +406,12 @@ TEST_F(KsckTest, TestConsensusConflictExtraPeer) {
   ASSERT_EQ("Corruption: 1 table(s) are bad", s.ToString());
   ASSERT_STR_CONTAINS(err_stream_.str(),
       "The consensus matrix is:\n"
-      "  Host                 Voters            Current term  Config index  Committed?\n"
-      "  -------------------  ----------------  ------------  ------------  ----------\n"
-      "  config from master:  A*  B   C                                     Yes       \n"
-      "                   A:  A*  B   C   D     0                           Yes       \n"
-      "                   B:  A*  B   C         0                           Yes       \n"
-      "                   C:  A*  B   C         0                           Yes");
+      " Config source |      Voters      | Current term | Config index | Committed?\n"
+      "---------------+------------------+--------------+--------------+------------\n"
+      " master        | A*  B   C        |              |              | Yes\n"
+      " A             | A*  B   C   D    | 0            |              | Yes\n"
+      " B             | A*  B   C        | 0            |              | Yes\n"
+      " C             | A*  B   C        | 0            |              | Yes");
 }
 
 TEST_F(KsckTest, TestConsensusConflictMissingPeer) {
@@ -427,12 +427,12 @@ TEST_F(KsckTest, TestConsensusConflictMissingPeer) {
   ASSERT_EQ("Corruption: 1 table(s) are bad", s.ToString());
   ASSERT_STR_CONTAINS(err_stream_.str(),
       "The consensus matrix is:\n"
-      "  Host                 Voters        Current term  Config index  Committed?\n"
-      "  -------------------  ------------  ------------  ------------  ----------\n"
-      "  config from master:  A*  B   C                                 Yes       \n"
-      "                   A:  A*  B         0                           Yes       \n"
-      "                   B:  A*  B   C     0                           Yes       \n"
-      "                   C:  A*  B   C     0                           Yes");
+      " Config source |    Voters    | Current term | Config index | Committed?\n"
+      "---------------+--------------+--------------+--------------+------------\n"
+      " master        | A*  B   C    |              |              | Yes\n"
+      " A             | A*  B        | 0            |              | Yes\n"
+      " B             | A*  B   C    | 0            |              | Yes\n"
+      " C             | A*  B   C    | 0            |              | Yes");
 }
 
 TEST_F(KsckTest, TestConsensusConflictDifferentLeader) {
@@ -448,12 +448,12 @@ TEST_F(KsckTest, TestConsensusConflictDifferentLeader) {
   ASSERT_EQ("Corruption: 1 table(s) are bad", s.ToString());
   ASSERT_STR_CONTAINS(err_stream_.str(),
       "The consensus matrix is:\n"
-      "  Host                 Voters        Current term  Config index  Committed?\n"
-      "  -------------------  ------------  ------------  ------------  ----------\n"
-      "  config from master:  A*  B   C                                 Yes       \n"
-      "                   A:  A   B*  C     0                           Yes       \n"
-      "                   B:  A*  B   C     0                           Yes       \n"
-      "                   C:  A*  B   C     0                           Yes");
+      " Config source |    Voters    | Current term | Config index | Committed?\n"
+      "---------------+--------------+--------------+--------------+------------\n"
+      " master        | A*  B   C    |              |              | Yes\n"
+      " A             | A   B*  C    | 0            |              | Yes\n"
+      " B             | A*  B   C    | 0            |              | Yes\n"
+      " C             | A*  B   C    | 0            |              | Yes");
 }
 
 TEST_F(KsckTest, TestOneOneTabletBrokenTable) {

http://git-wip-us.apache.org/repos/asf/kudu/blob/9adbdd77/src/kudu/tools/ksck.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/ksck.cc b/src/kudu/tools/ksck.cc
index 7361841..a99d4c1 100644
--- a/src/kudu/tools/ksck.cc
+++ b/src/kudu/tools/ksck.cc
@@ -23,6 +23,7 @@
 #include <iostream>
 #include <map>
 #include <mutex>
+#include <sstream>
 
 #include "kudu/consensus/quorum_util.h"
 #include "kudu/gutil/map-util.h"
@@ -32,6 +33,7 @@
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/gutil/strings/util.h"
 #include "kudu/tools/color.h"
+#include "kudu/tools/tool_action_common.h"
 #include "kudu/util/atomic.h"
 #include "kudu/util/blocking_queue.h"
 #include "kudu/util/locks.h"
@@ -66,6 +68,7 @@ using std::right;
 using std::setw;
 using std::shared_ptr;
 using std::string;
+using std::stringstream;
 using std::unordered_map;
 using strings::Substitute;
 
@@ -600,7 +603,7 @@ bool Ksck::VerifyTable(const shared_ptr<KsckTable>& table) {
 
 namespace {
 
-// A struct in which to consolidate the state of each replica for easier analysis.
+// A struct consolidating the state of each replica, for easier analysis.
 struct ReplicaInfo {
   KsckTabletReplica* replica;
   KsckTabletServer* ts = nullptr;
@@ -609,103 +612,22 @@ struct ReplicaInfo {
   boost::optional<KsckConsensusState> consensus_state;
 };
 
-// Formatting constants.
-const char col_pad[] = "  ";
-const int peer_width = 4;
-
-// Print a cell of the consensus matrix.
-void PrintCell(const string& val, size_t width) {
-  Out() << setw(width) << left << val;
-}
-
-// Print a row (one config) of the consensus matrix.
-void PrintConfig(const map<string, char>& peer_uuid_mapping,
-                 const string& name,
-                 const boost::optional<KsckConsensusState>& config,
-                 const vector<size_t>& column_widths) {
-  Out() << col_pad << setw(column_widths[0]) << right << Substitute("$0:", name);
-  Out() << col_pad;
-  if (!config) {
-    Out() << "[no config retrieved]" << endl;
-    return;
-  }
-  for (const auto& entry : peer_uuid_mapping) {
-    if (!ContainsKey(config->peer_uuids, entry.first)) {
-      PrintCell("", peer_width);
+// Formats the peers known and unknown to 'config' using labels from 'peer_uuid_mapping'.
+string format_peers(const map<string, char>& peer_uuid_mapping, const KsckConsensusState& config) {
+  stringstream voters;
+  int peer_width = 4;
+  for (const auto &entry : peer_uuid_mapping) {
+    if (!ContainsKey(config.peer_uuids, entry.first)) {
+      voters << setw(peer_width) << left << "";
       continue;
     }
-    if (config->leader_uuid && config->leader_uuid == entry.first) {
-      PrintCell(Substitute("$0*", entry.second), peer_width);
+    if (config.leader_uuid && config.leader_uuid == entry.first) {
+      voters << setw(peer_width) << left << Substitute("$0*", entry.second);
     } else {
-      PrintCell(Substitute("$0", entry.second), peer_width);
-    }
-  }
-
-  string term_str = "";
-  if (config->term) {
-    term_str = Substitute("$0", config->term.get());
-  }
-  Out() << col_pad << setw(column_widths[2]) << left << term_str;
-
-  string opid_str = "";
-  if (config->opid_index) {
-    opid_str = Substitute("$0", config->opid_index.get());
-  }
-  Out() << col_pad << setw(column_widths[3]) << left << opid_str;
-
-  string committed_str = "";
-  switch (config->type) {
-    case KsckConsensusConfigType::PENDING:
-      committed_str = "No"; break;
-    default:
-      committed_str = "Yes"; break;
-  }
-  Out() << col_pad << setw(column_widths[4]) << left << committed_str;
-  Out() << endl;
-}
-
-void PrintConsensusMatrix(const map<string, char>& peer_uuid_mapping,
-                          const KsckConsensusState& master_config,
-                          const vector<ReplicaInfo>& replica_infos) {
-  const vector<string> columns{"Host", "Voters", "Current term", "Config index", "Committed?"};
-  const string master_label = "config from master";
-
-  // Compute the column widths.
-  vector<size_t> column_widths{master_label.size() + 1, // + 1 for the :
-                               peer_width * peer_uuid_mapping.size(),
-                               columns[2].size(),
-                               columns[3].size(),
-                               columns[4].size(),
-  };
-
-  // Print the header rows.
-  const string col_pad = "  ";
-  Out() << col_pad;
-  for (int i = 0; i < columns.size(); i++) {
-    Out() << left << setw(column_widths[i]) << columns[i];
-    if (i < columns.size() - 1) {
-      Out() << col_pad;
-    }
-  }
-  Out() << endl;
-  Out() << col_pad;
-  for (int i = 0; i < column_widths.size(); i++) {
-    Out() << string(column_widths[i], '-');
-    if (i < column_widths.size() - 1) {
-      Out() << col_pad;
+      voters << setw(peer_width) << left << Substitute("$0", entry.second);
     }
   }
-  Out() << endl;
-
-  // Print the master row, then the tserver rows.
-  PrintConfig(peer_uuid_mapping, master_label, master_config, column_widths);
-  for (const auto& replica : replica_infos) {
-    char label = FindOrDie(peer_uuid_mapping, replica.ts->uuid());
-    PrintConfig(peer_uuid_mapping,
-                string(1, label),
-                replica.consensus_state,
-                column_widths);
-  }
+  return voters.str();
 }
 
 } // anonymous namespace
@@ -907,8 +829,40 @@ Ksck::CheckResult Ksck::VerifyTablet(const shared_ptr<KsckTablet>& tablet, int t
       Out() << "  " << entry.second << " = " << entry.first << endl;
     }
     Out() << endl;
-    Out() << "  The consensus matrix is:" << endl;
-    PrintConsensusMatrix(peer_uuid_mapping, master_config, replica_infos);
+    Out() << "The consensus matrix is:" << endl;
+
+    // Prepare the header and columns for PrintTable.
+    const vector<string> headers{ "Config source", "Voters", "Current term",
+                                  "Config index", "Committed?" };
+
+    // Seed the columns with the master info.
+    vector<string> sources{"master"};
+    vector<string> voters{format_peers(peer_uuid_mapping, master_config)};
+    vector<string> terms{""};
+    vector<string> indexes{""};
+    vector<string> committed{"Yes"};
+
+    // Fill out the columns with info from the replicas.
+    for (const auto& replica : replica_infos) {
+      char label = FindOrDie(peer_uuid_mapping, replica.ts->uuid());
+      sources.push_back(string(1, label));
+      if (!replica.consensus_state) {
+        voters.push_back("[config not available]");
+        terms.push_back("");
+        indexes.push_back("");
+        committed.push_back("");
+        continue;
+      }
+      voters.push_back(format_peers(peer_uuid_mapping, replica.consensus_state.get()));
+      terms.push_back(replica.consensus_state->term ?
+                      std::to_string(replica.consensus_state->term.get()) : "");
+      indexes.push_back(replica.consensus_state->opid_index ?
+                        std::to_string(replica.consensus_state->opid_index.get()) : "");
+      committed.push_back(replica.consensus_state->type == KsckConsensusConfigType::PENDING ?
+                          "No" : "Yes");
+    }
+    vector<vector<string>> columns{ sources, voters, terms, indexes, committed };
+    PrintTable(headers, columns, Out());
   }
 
   return result;

http://git-wip-us.apache.org/repos/asf/kudu/blob/9adbdd77/src/kudu/tools/tool_action_common.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/tool_action_common.cc b/src/kudu/tools/tool_action_common.cc
index 79ae3b6..89531a8 100644
--- a/src/kudu/tools/tool_action_common.cc
+++ b/src/kudu/tools/tool_action_common.cc
@@ -110,6 +110,7 @@ using server::SetFlagRequestPB;
 using server::SetFlagResponsePB;
 using std::cout;
 using std::endl;
+using std::ostream;
 using std::setfill;
 using std::setw;
 using std::shared_ptr;
@@ -381,7 +382,9 @@ namespace {
 //  dd23284d3a334f1a8306c19d89c1161f | 130.rack1.dc1.example.com:7050 | 1492596704536543
 //  d8009e07d82b4e66a7ab50f85e60bc30 | 136.rack1.dc1.example.com:7050 | 1492596696557549
 //  c108a85a68504c2bb9f49e4ee683d981 | 128.rack1.dc1.example.com:7050 | 1492596646623301
-void PrettyPrintTable(const vector<string>& headers, const vector<vector<string>>& columns) {
+void PrettyPrintTable(const vector<string>& headers,
+                      const vector<vector<string>>& columns,
+                      ostream& out) {
   CHECK_EQ(headers.size(), columns.size());
   if (headers.empty()) return;
   size_t num_columns = headers.size();
@@ -398,32 +401,32 @@ void PrettyPrintTable(const vector<string>& headers, const vector<vector<string>
   // Print the header row.
   for (int col = 0; col < num_columns; col++) {
     int padding = widths[col] - headers[col].size();
-    cout << setw(padding / 2) << "" << " " << headers[col];
-    if (col != num_columns - 1) cout << setw((padding + 1) / 2) << "" << " |";
+    out << setw(padding / 2) << "" << " " << headers[col];
+    if (col != num_columns - 1) out << setw((padding + 1) / 2) << "" << " |";
   }
-  cout << endl;
+  out << endl;
 
   // Print the separator row.
-  cout << setfill('-');
+  out << setfill('-');
   for (int col = 0; col < num_columns; col++) {
-    cout << setw(widths[col] + 2) << "";
-    if (col != num_columns - 1) cout << "+";
+    out << setw(widths[col] + 2) << "";
+    if (col != num_columns - 1) out << "+";
   }
-  cout << endl;
+  out << endl;
 
   // Print the data rows.
-  cout << setfill(' ');
+  out << setfill(' ');
   int num_rows = columns.empty() ? 0 : columns[0].size();
   for (int row = 0; row < num_rows; row++) {
     for (int col = 0; col < num_columns; col++) {
       const auto& value = columns[col][row];
-      cout << " " << value;
+      out << " " << value;
       if (col != num_columns - 1) {
         size_t padding = widths[col] - value.size();
-        cout << setw(padding) << "" << " |";
+        out << setw(padding) << "" << " |";
       }
     }
-    cout << endl;
+    out << endl;
   }
 }
 
@@ -431,7 +434,9 @@ void PrettyPrintTable(const vector<string>& headers, const vector<vector<string>
 //
 // The table is formatted as an array of objects. Each object corresponds
 // to a row whose fields are the column values.
-void JsonPrintTable(const vector<string>& headers, const vector<vector<string>>& columns) {
+void JsonPrintTable(const vector<string>& headers,
+                    const vector<vector<string>>& columns,
+                    ostream& out) {
   std::ostringstream stream;
   JsonWriter writer(&stream, JsonWriter::COMPACT);
 
@@ -449,7 +454,7 @@ void JsonPrintTable(const vector<string>& headers, const vector<vector<string>>&
   }
   writer.EndArray();
 
-  cout << stream.str() << endl;
+  out << stream.str() << endl;
 }
 
 // Print the table using the provided separator. For example, with a comma
@@ -460,32 +465,34 @@ void JsonPrintTable(const vector<string>& headers, const vector<vector<string>>&
 // dd23284d3a334f1a8306c19d89c1161f,130.rack1.dc1.example.com:7050,1492596704536543
 // d8009e07d82b4e66a7ab50f85e60bc30,136.rack1.dc1.example.com:7050,1492596696557549
 // c108a85a68504c2bb9f49e4ee683d981,128.rack1.dc1.example.com:7050,1492596646623301
-void PrintTable(const vector<vector<string>>& columns, const string& separator) {
+void PrintTable(const vector<vector<string>>& columns, const string& separator, ostream& out) {
   // TODO(dan): proper escaping of string values.
   int num_columns = columns.size();
   int num_rows = columns.empty() ? 0 : columns[0].size();
   for (int row = 0; row < num_rows; row++) {
       for (int col = 0; col < num_columns; col++) {
-        cout << columns[col][row];
-        if (col != num_columns - 1) cout << separator;
+        out << columns[col][row];
+        if (col != num_columns - 1) out << separator;
       }
-      cout << endl;
+      out << endl;
   }
 }
 
 } // anonymous namespace
 
-Status PrintTable(const vector<string>& headers, const vector<vector<string>>& columns) {
+Status PrintTable(const vector<string>& headers,
+                  const vector<vector<string>>& columns,
+                  ostream& out) {
   if (boost::iequals(FLAGS_format, "pretty")) {
-    PrettyPrintTable(headers, columns);
+    PrettyPrintTable(headers, columns, out);
   } else if (boost::iequals(FLAGS_format, "space")) {
-    PrintTable(columns, " ");
+    PrintTable(columns, " ", out);
   } else if (boost::iequals(FLAGS_format, "tsv")) {
-    PrintTable(columns, "	");
+    PrintTable(columns, "	", out);
   } else if (boost::iequals(FLAGS_format, "csv")) {
-    PrintTable(columns, ",");
+    PrintTable(columns, ",", out);
   } else if (boost::iequals(FLAGS_format, "json")) {
-    JsonPrintTable(headers, columns);
+    JsonPrintTable(headers, columns, out);
   } else {
     return Status::InvalidArgument("unknown format (--format)", FLAGS_format);
   }

http://git-wip-us.apache.org/repos/asf/kudu/blob/9adbdd77/src/kudu/tools/tool_action_common.h
----------------------------------------------------------------------
diff --git a/src/kudu/tools/tool_action_common.h b/src/kudu/tools/tool_action_common.h
index a0e39ef..439ba4f 100644
--- a/src/kudu/tools/tool_action_common.h
+++ b/src/kudu/tools/tool_action_common.h
@@ -18,6 +18,7 @@
 #pragma once
 
 #include <memory>
+#include <ostream>
 #include <string>
 #include <vector>
 
@@ -107,7 +108,8 @@ Status SetServerFlag(const std::string& address, uint16_t default_port,
 
 // Prints a table.
 Status PrintTable(const std::vector<std::string>& headers,
-                  const std::vector<std::vector<std::string>>& columns);
+                  const std::vector<std::vector<std::string>>& columns,
+                  std::ostream& out);
 
 // Wrapper around a Kudu client which allows calling proxy methods on the leader
 // master.

http://git-wip-us.apache.org/repos/asf/kudu/blob/9adbdd77/src/kudu/tools/tool_action_master.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/tool_action_master.cc b/src/kudu/tools/tool_action_master.cc
index 1dcce1d..b6d7704 100644
--- a/src/kudu/tools/tool_action_master.cc
+++ b/src/kudu/tools/tool_action_master.cc
@@ -17,6 +17,7 @@
 
 #include "kudu/tools/tool_action.h"
 
+#include <iostream>
 #include <memory>
 #include <string>
 #include <utility>
@@ -42,6 +43,7 @@ namespace kudu {
 using master::ListMastersRequestPB;
 using master::ListMastersResponsePB;
 using master::MasterServiceProxy;
+using std::cout;
 using std::string;
 using std::unique_ptr;
 
@@ -138,7 +140,7 @@ Status ListMasters(const RunnerContext& context) {
     columns.emplace_back(std::move(values));
   }
 
-  RETURN_NOT_OK(PrintTable(headers, columns));
+  RETURN_NOT_OK(PrintTable(headers, columns, cout));
   return Status::OK();
 }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/9adbdd77/src/kudu/tools/tool_action_tserver.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/tool_action_tserver.cc b/src/kudu/tools/tool_action_tserver.cc
index 0352ae8..4a5efe0 100644
--- a/src/kudu/tools/tool_action_tserver.cc
+++ b/src/kudu/tools/tool_action_tserver.cc
@@ -18,6 +18,7 @@
 #include "kudu/tools/tool_action.h"
 
 #include <functional>
+#include <iostream>
 #include <memory>
 #include <string>
 #include <utility>
@@ -38,6 +39,7 @@
 
 DECLARE_string(columns);
 
+using std::cout;
 using std::string;
 using std::unique_ptr;
 
@@ -136,7 +138,7 @@ Status ListTServers(const RunnerContext& context) {
     columns.emplace_back(std::move(values));
   }
 
-  RETURN_NOT_OK(PrintTable(headers, columns));
+  RETURN_NOT_OK(PrintTable(headers, columns, cout));
   return Status::OK();
 }