You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by to...@apache.org on 2016/08/12 01:59:39 UTC
[1/2] kudu git commit: maintenance_manager: fix gflags docs
Repository: kudu
Updated Branches:
refs/heads/master b811cbc7e -> 564eb4ed1
maintenance_manager: fix gflags docs
The gflag doc referenced an "emergency flush thread" which
was removed more than a year ago.
Change-Id: I0a68873db85e6ce5143d1a417a7b38965af088ae
Reviewed-on: http://gerrit.cloudera.org:8080/3950
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/1a589960
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/1a589960
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/1a589960
Branch: refs/heads/master
Commit: 1a589960328bcddf067e727538452e2518bc1f2e
Parents: b811cbc
Author: Todd Lipcon <to...@apache.org>
Authored: Thu Aug 11 17:33:38 2016 -0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Fri Aug 12 01:01:33 2016 +0000
----------------------------------------------------------------------
src/kudu/util/maintenance_manager.cc | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/1a589960/src/kudu/util/maintenance_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/maintenance_manager.cc b/src/kudu/util/maintenance_manager.cc
index 1060bf8..54c4dc6 100644
--- a/src/kudu/util/maintenance_manager.cc
+++ b/src/kudu/util/maintenance_manager.cc
@@ -38,9 +38,9 @@ using std::shared_ptr;
using strings::Substitute;
DEFINE_int32(maintenance_manager_num_threads, 1,
- "Size of the maintenance manager thread pool. Beyond a value of '1', one thread is "
- "reserved for emergency flushes. For spinning disks, the number of threads should "
- "not be above the number of devices.");
+ "Size of the maintenance manager thread pool. "
+ "For spinning disks, the number of threads should "
+ "not be above the number of devices.");
TAG_FLAG(maintenance_manager_num_threads, stable);
DEFINE_int32(maintenance_manager_polling_interval_ms, 250,
[2/2] kudu git commit: KUDU-564 (part 1): log a 'diff' when tablet
config changes
Posted by to...@apache.org.
KUDU-564 (part 1): log a 'diff' when tablet config changes
This adds some code to 'diff' two consensus configurations, so that log
messages in the master and during config change are much easier to read.
For example, after the update, raft_consensus-itest has logs like:
I0810 16:05:15.715248 21161 raft_consensus_state.cc:609] T 382e9565bbc64aefb973d1f0b21fcc7c P d4ce96ae644c45e1a867f4e14e6f6746 [term 1 NON_PARTICIPANT]: Committing config change with OpId 1.2: config changed from index -1 to 2, VOTER P d4ce96ae644c45e1a867f4e14e6f6746 (127.37.24.4) evicted. New config: { opid_index: 2 peers { permanent_uuid: "544750e465814b6087b2c44a23d240f4" member_type: VOTER last_known_addr { host: "127.37.24.0" port: 39133 } } peers { permanent_uuid: "535909db4a48478288433b7f6999565e" member_type: VOTER last_known_addr { host: "127.37.24.3" port: 35244 } } peers { permanent_uuid: "9b9c692a90854ba7909a47912fc3d0bb" member_type: VOTER last_known_addr { host: "127.37.24.1" port: 39890 } } peers { permanent_uuid: "13754a25859d4fd1a97fcaa0fcb6f849" member_type: VOTER last_known_addr { host: "127.37.24.2" port: 40505 } } }
I left in the dumping of the full 'new config' since having all the
information can still be relevant, but the human readable summary makes
this much easier to interpret.
Change-Id: Icb785093176eea067de039b14f6600084998861a
Reviewed-on: http://gerrit.cloudera.org:8080/3939
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mp...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/564eb4ed
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/564eb4ed
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/564eb4ed
Branch: refs/heads/master
Commit: 564eb4ed1440aeadc0c3948168be8fe12888a46d
Parents: 1a58996
Author: Todd Lipcon <to...@apache.org>
Authored: Wed Aug 10 16:07:56 2016 -0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Fri Aug 12 01:59:06 2016 +0000
----------------------------------------------------------------------
src/kudu/consensus/quorum_util-test.cc | 72 +++++++++++++++
src/kudu/consensus/quorum_util.cc | 117 +++++++++++++++++++++++-
src/kudu/consensus/quorum_util.h | 11 ++-
src/kudu/consensus/raft_consensus_state.cc | 6 +-
src/kudu/master/catalog_manager.cc | 5 +-
5 files changed, 204 insertions(+), 7 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/564eb4ed/src/kudu/consensus/quorum_util-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/quorum_util-test.cc b/src/kudu/consensus/quorum_util-test.cc
index 8d99cf0..4b9f50d 100644
--- a/src/kudu/consensus/quorum_util-test.cc
+++ b/src/kudu/consensus/quorum_util-test.cc
@@ -31,6 +31,7 @@ static void SetPeerInfo(const string& uuid,
RaftPeerPB* peer) {
peer->set_permanent_uuid(uuid);
peer->set_member_type(type);
+ peer->mutable_last_known_addr()->set_host(uuid + ".example.com");
}
TEST(QuorumUtilTest, TestMemberExtraction) {
@@ -56,5 +57,76 @@ TEST(QuorumUtilTest, TestMemberExtraction) {
ASSERT_EQ("B", peer_pb.permanent_uuid());
}
+TEST(QuorumUtilTest, TestDiffConsensusStates) {
+ ConsensusStatePB old_cs;
+ SetPeerInfo("A", RaftPeerPB::VOTER, old_cs.mutable_config()->add_peers());
+ SetPeerInfo("B", RaftPeerPB::VOTER, old_cs.mutable_config()->add_peers());
+ SetPeerInfo("C", RaftPeerPB::VOTER, old_cs.mutable_config()->add_peers());
+ old_cs.set_current_term(1);
+ old_cs.set_leader_uuid("A");
+ old_cs.mutable_config()->set_opid_index(1);
+
+ // Simple case of no change.
+ EXPECT_EQ("no change",
+ DiffConsensusStates(old_cs, old_cs));
+
+ // Simulate a leader change.
+ {
+ auto new_cs = old_cs;
+ new_cs.set_leader_uuid("B");
+ new_cs.set_current_term(2);
+
+ EXPECT_EQ("term changed from 1 to 2, "
+ "leader changed from A (A.example.com) to B (B.example.com)",
+ DiffConsensusStates(old_cs, new_cs));
+ }
+
+ // Simulate eviction of a peer.
+ {
+ auto new_cs = old_cs;
+ new_cs.mutable_config()->set_opid_index(2);
+ new_cs.mutable_config()->mutable_peers()->RemoveLast();
+
+ EXPECT_EQ("config changed from index 1 to 2, "
+ "VOTER C (C.example.com) evicted",
+ DiffConsensusStates(old_cs, new_cs));
+ }
+
+ // Simulate addition of a peer.
+ {
+ auto new_cs = old_cs;
+ new_cs.mutable_config()->set_opid_index(2);
+ SetPeerInfo("D", RaftPeerPB::NON_VOTER, new_cs.mutable_config()->add_peers());
+
+ EXPECT_EQ("config changed from index 1 to 2, "
+ "NON_VOTER D (D.example.com) added",
+ DiffConsensusStates(old_cs, new_cs));
+ }
+
+ // Simulate change of a peer's member type.
+ {
+ auto new_cs = old_cs;
+ new_cs.mutable_config()->set_opid_index(2);
+ new_cs.mutable_config()->mutable_peers()->Mutable(2)->set_member_type(RaftPeerPB::NON_VOTER);
+
+ EXPECT_EQ("config changed from index 1 to 2, "
+ "C (C.example.com) changed from VOTER to NON_VOTER",
+ DiffConsensusStates(old_cs, new_cs));
+ }
+
+ // Simulate change from no leader to a leader
+ {
+ auto no_leader_cs = old_cs;
+ no_leader_cs.clear_leader_uuid();
+ auto new_cs = old_cs;
+ new_cs.set_current_term(2);
+
+ EXPECT_EQ("term changed from 1 to 2, "
+ "leader changed from <none> to A (A.example.com)",
+ DiffConsensusStates(no_leader_cs, new_cs));
+ }
+
+}
+
} // namespace consensus
} // namespace kudu
http://git-wip-us.apache.org/repos/asf/kudu/blob/564eb4ed/src/kudu/consensus/quorum_util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/quorum_util.cc b/src/kudu/consensus/quorum_util.cc
index 26cc4a6..44dc306 100644
--- a/src/kudu/consensus/quorum_util.cc
+++ b/src/kudu/consensus/quorum_util.cc
@@ -16,10 +16,14 @@
// under the License.
#include "kudu/consensus/quorum_util.h"
+#include <map>
#include <set>
#include <string>
+#include <utility>
+#include <vector>
#include "kudu/gutil/map-util.h"
+#include "kudu/gutil/strings/join.h"
#include "kudu/gutil/strings/substitute.h"
#include "kudu/util/status.h"
@@ -27,6 +31,8 @@ namespace kudu {
namespace consensus {
using google::protobuf::RepeatedPtrField;
+using std::map;
+using std::pair;
using std::string;
using strings::Substitute;
@@ -196,5 +202,114 @@ Status VerifyConsensusState(const ConsensusStatePB& cstate, RaftConfigState type
return Status::OK();
}
-} // namespace consensus
+std::string DiffRaftConfigs(const RaftConfigPB& old_config,
+ const RaftConfigPB& new_config) {
+ // Create dummy ConsensusState objects so we can reuse the code
+ // from the below function.
+ ConsensusStatePB old_state;
+ old_state.mutable_config()->CopyFrom(old_config);
+ ConsensusStatePB new_state;
+ new_state.mutable_config()->CopyFrom(new_config);
+
+ return DiffConsensusStates(old_state, new_state);
+}
+
+string DiffConsensusStates(const ConsensusStatePB& old_state,
+ const ConsensusStatePB& new_state) {
+ bool leader_changed = old_state.leader_uuid() != new_state.leader_uuid();
+ bool term_changed = old_state.current_term() != new_state.current_term();
+ bool config_changed = old_state.config().opid_index() != new_state.config().opid_index();
+
+ // Construct a map from Peer UUID to '<old peer, new peer>' pairs.
+ // Due to the default construction nature of std::map and std::pair, if a peer
+ // is present in one configuration but not the other, we'll end up with an empty
+ // protobuf in that element of the pair.
+ map<string, pair<RaftPeerPB, RaftPeerPB>> peer_infos;
+ for (const auto& p : old_state.config().peers()) {
+ peer_infos[p.permanent_uuid()].first = p;
+ }
+ for (const auto& p : new_state.config().peers()) {
+ peer_infos[p.permanent_uuid()].second = p;
+ }
+
+ // Now collect strings representing the changes.
+ vector<string> change_strs;
+ if (config_changed) {
+ change_strs.push_back(
+ Substitute("config changed from index $0 to $1",
+ old_state.config().opid_index(),
+ new_state.config().opid_index()));
+ }
+
+ if (term_changed) {
+ change_strs.push_back(
+ Substitute("term changed from $0 to $1",
+ old_state.current_term(),
+ new_state.current_term()));
+ }
+
+ if (leader_changed) {
+ string old_leader = "<none>";
+ string new_leader = "<none>";
+ if (old_state.has_leader_uuid()) {
+ old_leader = Substitute("$0 ($1)",
+ old_state.leader_uuid(),
+ peer_infos[old_state.leader_uuid()].first.last_known_addr().host());
+ }
+ if (new_state.has_leader_uuid()) {
+ new_leader = Substitute("$0 ($1)",
+ new_state.leader_uuid(),
+ peer_infos[new_state.leader_uuid()].second.last_known_addr().host());
+ }
+
+ change_strs.push_back(Substitute("leader changed from $0 to $1",
+ old_leader, new_leader));
+ }
+
+ for (const auto& e : peer_infos) {
+ const auto& old_peer = e.second.first;
+ const auto& new_peer = e.second.second;
+ if (old_peer.has_permanent_uuid() && !new_peer.has_permanent_uuid()) {
+ change_strs.push_back(
+ Substitute("$0 $1 ($2) evicted",
+ RaftPeerPB_MemberType_Name(old_peer.member_type()),
+ old_peer.permanent_uuid(),
+ old_peer.last_known_addr().host()));
+ } else if (!old_peer.has_permanent_uuid() && new_peer.has_permanent_uuid()) {
+ change_strs.push_back(
+ Substitute("$0 $1 ($2) added",
+ RaftPeerPB_MemberType_Name(new_peer.member_type()),
+ new_peer.permanent_uuid(),
+ new_peer.last_known_addr().host()));
+ } else if (old_peer.has_permanent_uuid() && new_peer.has_permanent_uuid()) {
+ if (old_peer.member_type() != new_peer.member_type()) {
+ change_strs.push_back(
+ Substitute("$0 ($1) changed from $2 to $3",
+ old_peer.permanent_uuid(),
+ old_peer.last_known_addr().host(),
+ RaftPeerPB_MemberType_Name(old_peer.member_type()),
+ RaftPeerPB_MemberType_Name(new_peer.member_type())));
+ }
+ }
+ }
+
+ // We expect to have detected some differences above, but in case
+ // someone forgets to update this function when adding a new field,
+ // it's still useful to report some change unless the protobufs are identical.
+ // So, we fall back to just dumping the before/after debug strings.
+ if (change_strs.empty()) {
+ if (old_state.ShortDebugString() == new_state.ShortDebugString()) {
+ return "no change";
+ }
+ return Substitute("change from {$0} to {$1}",
+ old_state.ShortDebugString(),
+ new_state.ShortDebugString());
+ }
+
+
+ return JoinStrings(change_strs, ", ");
+
+}
+
+} // namespace consensus
} // namespace kudu
http://git-wip-us.apache.org/repos/asf/kudu/blob/564eb4ed/src/kudu/consensus/quorum_util.h
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/quorum_util.h b/src/kudu/consensus/quorum_util.h
index 3d1ccb0..ed21e9d 100644
--- a/src/kudu/consensus/quorum_util.h
+++ b/src/kudu/consensus/quorum_util.h
@@ -62,7 +62,7 @@ int MajoritySize(int num_voters);
// If the peer uuid is not a voter in the configuration, this function will return
// NON_PARTICIPANT, regardless of whether it is listed as leader in cstate.
RaftPeerPB::Role GetConsensusRole(const std::string& uuid,
- const ConsensusStatePB& cstate);
+ const ConsensusStatePB& cstate);
// Verifies that the provided configuration is well formed.
// If type == COMMITTED_QUORUM, we enforce that opid_index is set.
@@ -73,6 +73,15 @@ Status VerifyRaftConfig(const RaftConfigPB& config, RaftConfigState type);
// leader is a configuration voter, if it is set, and that a valid term is set.
Status VerifyConsensusState(const ConsensusStatePB& cstate, RaftConfigState type);
+// Provide a textual description of the difference between two consensus states,
+// suitable for logging.
+std::string DiffConsensusStates(const ConsensusStatePB& old_state,
+ const ConsensusStatePB& new_state);
+
+// Same as the above, but just the RaftConfigPB portion of the configuration.
+std::string DiffRaftConfigs(const RaftConfigPB& old_config,
+ const RaftConfigPB& new_config);
+
} // namespace consensus
} // namespace kudu
http://git-wip-us.apache.org/repos/asf/kudu/blob/564eb4ed/src/kudu/consensus/raft_consensus_state.cc
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/raft_consensus_state.cc b/src/kudu/consensus/raft_consensus_state.cc
index 3b3e703..7d937df 100644
--- a/src/kudu/consensus/raft_consensus_state.cc
+++ b/src/kudu/consensus/raft_consensus_state.cc
@@ -607,9 +607,9 @@ Status ReplicaState::AdvanceCommittedIndexUnlocked(const OpId& committed_index,
const RaftConfigPB& committed_config = GetCommittedConfigUnlocked();
if (new_config.opid_index() > committed_config.opid_index()) {
LOG_WITH_PREFIX_UNLOCKED(INFO) << "Committing config change with OpId "
- << current_id << ". "
- << "Old config: { " << old_config.ShortDebugString() << " }. "
- << "New config: { " << new_config.ShortDebugString() << " }";
+ << current_id << ": "
+ << DiffRaftConfigs(old_config, new_config)
+ << ". New config: { " << new_config.ShortDebugString() << " }";
CHECK_OK(SetCommittedConfigUnlocked(new_config));
} else {
LOG_WITH_PREFIX_UNLOCKED(INFO) << "Ignoring commit of config change with OpId "
http://git-wip-us.apache.org/repos/asf/kudu/blob/564eb4ed/src/kudu/master/catalog_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc
index e2061d9..49001f8 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -1979,8 +1979,9 @@ Status CatalogManager::HandleReportedTablet(TSDescriptor* ts_desc,
// If a replica is reporting a new consensus configuration, update the
// master's copy of that configuration.
- LOG(INFO) << "Tablet: " << tablet->tablet_id() << " reported consensus state change."
- << " New consensus state: " << cstate.ShortDebugString();
+ LOG(INFO) << "T " << tablet->tablet_id() << " reported consensus state change: "
+ << DiffConsensusStates(prev_cstate, cstate)
+ << ". New consensus state: " << cstate.ShortDebugString();
// If we need to change the report, copy the whole thing on the stack
// rather than const-casting.