Repository: kudu Updated Branches: refs/heads/master ec81bd9d2 -> fbaa7dce9
[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 <[email protected]> Reviewed-by: Mike Percy <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/30682fd1 Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/30682fd1 Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/30682fd1 Branch: refs/heads/master Commit: 30682fd13ffc68ebaeb8200e7450f8317b89bf85 Parents: ec81bd9 Author: Will Berkeley <[email protected]> Authored: Thu Jun 1 10:18:15 2017 -0700 Committer: Will Berkeley <[email protected]> Committed: Thu Jun 1 19:50:28 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/30682fd1/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/30682fd1/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/30682fd1/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/30682fd1/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/30682fd1/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/30682fd1/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(); }
