You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by mp...@apache.org on 2018/02/09 06:04:59 UTC
kudu git commit: Add additonal form and tests for GetConsensusRole()
Repository: kudu
Updated Branches:
refs/heads/master f5d7292fa -> c8a1c0f97
Add additonal form and tests for GetConsensusRole()
There are cases where calling GetConsensusRole() without creating a
ConsensusStatePB object would be more efficient. This patch adds a
version of GetConsensusRole() with the following signature:
RaftPeerPB::Role GetConsensusRole(const std::string& peer_uuid,
const std::string& leader_uuid,
const RaftConfigPB& config);
This patch also fixes a bug where the role of a non-voter that was
thought to be the leader would be returned as NON_PARTICIPANT instead of
LEARNER. However, that is an illegal state so it should not actually
affect production code.
Also added a unit test for both forms of GetConsensusRole().
Change-Id: Ic28f15c40ce99f89481cb6869d4843cbc024ed48
Reviewed-on: http://gerrit.cloudera.org:8080/9260
Reviewed-by: Alexey Serbin <as...@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/c8a1c0f9
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/c8a1c0f9
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/c8a1c0f9
Branch: refs/heads/master
Commit: c8a1c0f977c122e4da0e12deec9e23f7ce6a9e74
Parents: f5d7292
Author: Mike Percy <mp...@apache.org>
Authored: Thu Feb 8 14:53:27 2018 -0800
Committer: Mike Percy <mp...@apache.org>
Committed: Fri Feb 9 06:04:20 2018 +0000
----------------------------------------------------------------------
src/kudu/consensus/quorum_util-test.cc | 44 +++++++++++++++++++++++++++++
src/kudu/consensus/quorum_util.cc | 27 +++++++++++-------
src/kudu/consensus/quorum_util.h | 18 +++++++++---
3 files changed, 75 insertions(+), 14 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/c8a1c0f9/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 2a80177..9dc4d7a 100644
--- a/src/kudu/consensus/quorum_util-test.cc
+++ b/src/kudu/consensus/quorum_util-test.cc
@@ -30,6 +30,7 @@
#include "kudu/util/status.h"
#include "kudu/util/test_macros.h"
+using std::pair;
using std::string;
using std::unique_ptr;
using std::vector;
@@ -99,6 +100,16 @@ static void AddPeer(RaftConfigPB* config,
}
}
+using RaftMemberSpec = pair<string, RaftPeerPB::MemberType>;
+
+static RaftConfigPB CreateConfig(const vector<RaftMemberSpec>& specs) {
+ RaftConfigPB config;
+ for (const auto& spec : specs) {
+ AddPeer(&config, spec.first, spec.second);
+ }
+ return config;
+}
+
static void PromotePeer(RaftConfigPB* config, const string& peer_uuid) {
RaftPeerPB* peer_pb;
const Status s = GetRaftConfigMember(config, peer_uuid, &peer_pb);
@@ -245,6 +256,39 @@ TEST(QuorumUtilTest, TestDiffConsensusStates) {
}
}
+// Unit test for the variants of GetConsensusRole().
+TEST(QuorumUtilTest, TestGetConsensusRole) {
+ const auto LEADER = RaftPeerPB::LEADER;
+ const auto FOLLOWER = RaftPeerPB::FOLLOWER;
+ const auto LEARNER = RaftPeerPB::LEARNER;
+ const auto NON_PARTICIPANT = RaftPeerPB::NON_PARTICIPANT;
+
+ // 3-argument variant of GetConsensusRole().
+ const auto config1 = CreateConfig({ {"A", V}, {"B", V}, {"C", N} });
+ ASSERT_EQ(LEADER, GetConsensusRole("A", "A", config1));
+ ASSERT_EQ(FOLLOWER, GetConsensusRole("B", "A", config1));
+ ASSERT_EQ(FOLLOWER, GetConsensusRole("A", "", config1));
+ ASSERT_EQ(LEARNER, GetConsensusRole("C", "A", config1));
+ ASSERT_EQ(LEARNER, GetConsensusRole("C", "C", config1)); // Illegal.
+ ASSERT_EQ(NON_PARTICIPANT, GetConsensusRole("D", "A", config1));
+ ASSERT_EQ(NON_PARTICIPANT, GetConsensusRole("D", "D", config1)); // Illegal.
+ ASSERT_EQ(NON_PARTICIPANT, GetConsensusRole("", "A", config1)); // Illegal.
+ ASSERT_EQ(NON_PARTICIPANT, GetConsensusRole("", "", config1)); // Illegal.
+
+ // 2-argument variant of GetConsensusRole().
+ const auto config2 = CreateConfig({ {"A", V}, {"B", V}, {"C", V} });
+ ConsensusStatePB cstate;
+ *cstate.mutable_committed_config() = config1;
+ *cstate.mutable_pending_config() = config2;
+ cstate.set_leader_uuid("A");
+ ASSERT_EQ(LEADER, GetConsensusRole("A", cstate));
+ ASSERT_EQ(FOLLOWER, GetConsensusRole("B", cstate));
+ ASSERT_EQ(FOLLOWER, GetConsensusRole("C", cstate));
+ ASSERT_EQ(NON_PARTICIPANT, GetConsensusRole("D", cstate));
+ cstate.set_leader_uuid("D");
+ ASSERT_EQ(NON_PARTICIPANT, GetConsensusRole("D", cstate)); // Illegal.
+}
+
TEST(QuorumUtilTest, TestIsRaftConfigVoter) {
RaftConfigPB config;
AddPeer(&config, "A", V);
http://git-wip-us.apache.org/repos/asf/kudu/blob/c8a1c0f9/src/kudu/consensus/quorum_util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/quorum_util.cc b/src/kudu/consensus/quorum_util.cc
index f664a59..2697911 100644
--- a/src/kudu/consensus/quorum_util.cc
+++ b/src/kudu/consensus/quorum_util.cc
@@ -122,22 +122,20 @@ int MajoritySize(int num_voters) {
return (num_voters / 2) + 1;
}
-RaftPeerPB::Role GetConsensusRole(const std::string& uuid, const ConsensusStatePB& cstate) {
- // The active config is the pending config if there is one, else it's the committed config.
- const RaftConfigPB& config = cstate.has_pending_config() ?
- cstate.pending_config() :
- cstate.committed_config();
- if (cstate.leader_uuid() == uuid) {
- if (IsRaftConfigVoter(uuid, config)) {
- return RaftPeerPB::LEADER;
- }
+RaftPeerPB::Role GetConsensusRole(const std::string& peer_uuid,
+ const std::string& leader_uuid,
+ const RaftConfigPB& config) {
+ if (peer_uuid.empty()) {
return RaftPeerPB::NON_PARTICIPANT;
}
for (const RaftPeerPB& peer : config.peers()) {
- if (peer.permanent_uuid() == uuid) {
+ if (peer.permanent_uuid() == peer_uuid) {
switch (peer.member_type()) {
case RaftPeerPB::VOTER:
+ if (peer_uuid == leader_uuid) {
+ return RaftPeerPB::LEADER;
+ }
return RaftPeerPB::FOLLOWER;
default:
return RaftPeerPB::LEARNER;
@@ -147,6 +145,15 @@ RaftPeerPB::Role GetConsensusRole(const std::string& uuid, const ConsensusStateP
return RaftPeerPB::NON_PARTICIPANT;
}
+RaftPeerPB::Role GetConsensusRole(const std::string& peer_uuid,
+ const ConsensusStatePB& cstate) {
+ // The active config is the pending config if there is one, else it's the committed config.
+ const RaftConfigPB& config = cstate.has_pending_config() ?
+ cstate.pending_config() :
+ cstate.committed_config();
+ return GetConsensusRole(peer_uuid, cstate.leader_uuid(), config);
+}
+
Status VerifyRaftConfig(const RaftConfigPB& config) {
std::set<string> uuids;
if (config.peers().empty()) {
http://git-wip-us.apache.org/repos/asf/kudu/blob/c8a1c0f9/src/kudu/consensus/quorum_util.h
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/quorum_util.h b/src/kudu/consensus/quorum_util.h
index d774a31..a4cc095 100644
--- a/src/kudu/consensus/quorum_util.h
+++ b/src/kudu/consensus/quorum_util.h
@@ -79,10 +79,20 @@ int CountVoters(const RaftConfigPB& config);
// Calculates size of a configuration majority based on # of voters.
int MajoritySize(int num_voters);
-// Determines the role that the peer with uuid 'uuid' plays in the cluster.
-// 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,
+// Determines the role that the peer with uuid 'peer_uuid' plays in the
+// cluster. If 'peer_uuid' is empty or is not a member of the configuration,
+// this function will return NON_PARTICIPANT, regardless of whether it is
+// specified as the leader in 'leader_uuid'. Likewise, if 'peer_uuid' is a
+// NON_VOTER in the config, this function will return LEARNER, regardless of
+// whether it is specified as the leader in 'leader_uuid' (although that
+// situation is illegal in practice).
+RaftPeerPB::Role GetConsensusRole(const std::string& peer_uuid,
+ const std::string& leader_uuid,
+ const RaftConfigPB& config);
+
+// Same as above, but uses the leader and active role from the given
+// ConsensusStatePB.
+RaftPeerPB::Role GetConsensusRole(const std::string& peer_uuid,
const ConsensusStatePB& cstate);
// Verifies that the provided configuration is well formed.