kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ale...@apache.org
Subject kudu git commit: [consensus] micro-cleanup on VerifyRaftConfig()
Date Tue, 26 Sep 2017 01:03:10 GMT
Repository: kudu
Updated Branches:
  refs/heads/master 98b66574b -> bab96a2b2


[consensus] micro-cleanup on VerifyRaftConfig()

Removed unused RaftConfigState parameter for VerifyRaftConfig().
In addition, fixed typos in metadata.proto inline documentation.

Change-Id: Ic5b4cb6ceacde7a0c0b2f1c75dca4b7d1681ef33
Reviewed-on: http://gerrit.cloudera.org:8080/8078
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mpercy@apache.org>


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

Branch: refs/heads/master
Commit: bab96a2b299de4f1f911e23c80167938ed69394d
Parents: 98b6657
Author: Alexey Serbin <aserbin@cloudera.com>
Authored: Thu Sep 14 19:57:43 2017 -0700
Committer: Alexey Serbin <aserbin@cloudera.com>
Committed: Tue Sep 26 00:56:27 2017 +0000

----------------------------------------------------------------------
 src/kudu/consensus/consensus_meta.cc |  2 +-
 src/kudu/consensus/metadata.proto    |  8 ++++----
 src/kudu/consensus/quorum_util.cc    | 10 +++++-----
 src/kudu/consensus/quorum_util.h     |  5 +----
 src/kudu/consensus/raft_consensus.cc |  8 ++++----
 src/kudu/master/sys_catalog.cc       |  5 ++---
 6 files changed, 17 insertions(+), 21 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/bab96a2b/src/kudu/consensus/consensus_meta.cc
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/consensus_meta.cc b/src/kudu/consensus/consensus_meta.cc
index 75de554..9954c8a 100644
--- a/src/kudu/consensus/consensus_meta.cc
+++ b/src/kudu/consensus/consensus_meta.cc
@@ -267,7 +267,7 @@ Status ConsensusMetadata::Flush(FlushMode flush_mode) {
 
   flush_count_for_tests_++;
   // Sanity test to ensure we never write out a bad configuration.
-  RETURN_NOT_OK_PREPEND(VerifyRaftConfig(pb_.committed_config(), COMMITTED_CONFIG),
+  RETURN_NOT_OK_PREPEND(VerifyRaftConfig(pb_.committed_config()),
                         "Invalid config in ConsensusMetadata, cannot flush to disk");
 
   // Create directories if needed.

http://git-wip-us.apache.org/repos/asf/kudu/blob/bab96a2b/src/kudu/consensus/metadata.proto
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/metadata.proto b/src/kudu/consensus/metadata.proto
index cc7223d..718d9eb 100644
--- a/src/kudu/consensus/metadata.proto
+++ b/src/kudu/consensus/metadata.proto
@@ -59,7 +59,7 @@ message RaftPeerPB {
   // Permanent uuid is optional: RaftPeerPB/RaftConfigPB instances may
   // be created before the permanent uuid is known (e.g., when
   // manually specifying a configuration for Master/CatalogManager);
-  // permament uuid can be retrieved at a later time through RPC.
+  // permanent uuid can be retrieved at a later time through RPC.
   optional bytes permanent_uuid = 1;
   optional MemberType member_type = 2;
   optional HostPortPB last_known_addr = 3;
@@ -97,8 +97,8 @@ message ConsensusStatePB {
   // to a term check on the UpdateConsensus() RPC request.
   //
   // Whenever the local peer sees a new term, the leader flag is cleared until
-  // a new leader is acknowledged based on the above critera. Simply casting a
-  // vote for a peer is not sufficient to assume that that peer has won the
+  // a new leader is acknowledged based on the above criteria. Simply casting a
+  // vote for a peer is not sufficient to assume that the peer has won the
   // election, so we do not update this field based on our vote.
   //
   // The leader may be a part of the committed or the pending configuration (or both).
@@ -125,7 +125,7 @@ message ConsensusMetadataPB {
   // Whenever a new election is started, the candidate increments this by one
   // and requests votes from peers.
   //
-  // If any RPC or RPC response is received from another node containing a term higher
+  // If any RPC request or response is received from another node containing a term higher
   // than this one, the server should step down to FOLLOWER and set its current_term to
   // match the caller's term.
   //

http://git-wip-us.apache.org/repos/asf/kudu/blob/bab96a2b/src/kudu/consensus/quorum_util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/quorum_util.cc b/src/kudu/consensus/quorum_util.cc
index 6c791b3..ca074b4 100644
--- a/src/kudu/consensus/quorum_util.cc
+++ b/src/kudu/consensus/quorum_util.cc
@@ -134,9 +134,9 @@ RaftPeerPB::Role GetConsensusRole(const std::string& uuid, const ConsensusStateP
   return RaftPeerPB::NON_PARTICIPANT;
 }
 
-Status VerifyRaftConfig(const RaftConfigPB& config, RaftConfigState type) {
+Status VerifyRaftConfig(const RaftConfigPB& config) {
   std::set<string> uuids;
-  if (config.peers_size() == 0) {
+  if (config.peers().empty()) {
     return Status::IllegalState(
         Substitute("RaftConfig must have at least one peer. RaftConfig: $0",
                    SecureShortDebugString(config)));
@@ -150,7 +150,7 @@ Status VerifyRaftConfig(const RaftConfigPB& config, RaftConfigState
type) {
   }
 
   for (const RaftPeerPB& peer : config.peers()) {
-    if (!peer.has_permanent_uuid() || peer.permanent_uuid() == "") {
+    if (!peer.has_permanent_uuid() || peer.permanent_uuid().empty()) {
       return Status::IllegalState(Substitute("One peer didn't have an uuid or had the empty"
           " string. RaftConfig: $0", SecureShortDebugString(config)));
     }
@@ -190,9 +190,9 @@ Status VerifyConsensusState(const ConsensusStatePB& cstate) {
   if (!cstate.has_committed_config()) {
     return Status::IllegalState("ConsensusStatePB missing config", SecureShortDebugString(cstate));
   }
-  RETURN_NOT_OK(VerifyRaftConfig(cstate.committed_config(), COMMITTED_CONFIG));
+  RETURN_NOT_OK(VerifyRaftConfig(cstate.committed_config()));
   if (cstate.has_pending_config()) {
-    RETURN_NOT_OK(VerifyRaftConfig(cstate.pending_config(), PENDING_CONFIG));
+    RETURN_NOT_OK(VerifyRaftConfig(cstate.pending_config()));
   }
 
   if (!cstate.leader_uuid().empty()) {

http://git-wip-us.apache.org/repos/asf/kudu/blob/bab96a2b/src/kudu/consensus/quorum_util.h
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/quorum_util.h b/src/kudu/consensus/quorum_util.h
index d502037..822a789 100644
--- a/src/kudu/consensus/quorum_util.h
+++ b/src/kudu/consensus/quorum_util.h
@@ -24,7 +24,6 @@
 #include "kudu/util/status.h"
 
 namespace kudu {
-
 namespace consensus {
 
 enum RaftConfigState {
@@ -66,9 +65,7 @@ RaftPeerPB::Role GetConsensusRole(const std::string& uuid,
                                   const ConsensusStatePB& cstate);
 
 // Verifies that the provided configuration is well formed.
-// If type == COMMITTED_QUORUM, we enforce that opid_index is set.
-// If type == UNCOMMITTED_QUORUM, we enforce that opid_index is NOT set.
-Status VerifyRaftConfig(const RaftConfigPB& config, RaftConfigState type);
+Status VerifyRaftConfig(const RaftConfigPB& config);
 
 // Superset of checks performed by VerifyRaftConfig. Also ensures that the
 // leader is a configuration voter, if it is set, and that a valid term is set.

http://git-wip-us.apache.org/repos/asf/kudu/blob/bab96a2b/src/kudu/consensus/raft_consensus.cc
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/raft_consensus.cc b/src/kudu/consensus/raft_consensus.cc
index d70d566..53268db 100644
--- a/src/kudu/consensus/raft_consensus.cc
+++ b/src/kudu/consensus/raft_consensus.cc
@@ -796,7 +796,7 @@ Status RaftConsensus::Update(const ConsensusRequestPB* request,
   std::lock_guard<simple_spinlock> lock(update_lock_);
   Status s = UpdateReplica(request, response);
   if (PREDICT_FALSE(VLOG_IS_ON(1))) {
-    if (request->ops_size() == 0) {
+    if (request->ops().empty()) {
       VLOG_WITH_PREFIX(1) << "Replica replied to status only request. Replica: "
                           << ToString() << ". Response: "
                           << SecureShortDebugString(*response);
@@ -1740,7 +1740,7 @@ Status RaftConsensus::UnsafeChangeConfig(const UnsafeChangeConfigRequestPB&
req,
   new_config.set_opid_index(replicate_opid_index);
 
   // Sanity check the new config. 'type' is irrelevant here.
-  Status s = VerifyRaftConfig(new_config, PENDING_CONFIG);
+  Status s = VerifyRaftConfig(new_config);
   if (!s.ok()) {
     *error_code = TabletServerErrorPB::INVALID_CONFIG;
     return Status::InvalidArgument(Substitute("The resulting new config for tablet $0  "
@@ -2558,7 +2558,7 @@ Status RaftConsensus::CheckNoConfigChangePendingUnlocked() const {
 
 Status RaftConsensus::SetPendingConfigUnlocked(const RaftConfigPB& new_config) {
   DCHECK(lock_.is_locked());
-  RETURN_NOT_OK_PREPEND(VerifyRaftConfig(new_config, PENDING_CONFIG),
+  RETURN_NOT_OK_PREPEND(VerifyRaftConfig(new_config),
                         "Invalid config to set as pending");
   if (!new_config.unsafe_config_change()) {
     CHECK(!cmeta_->has_pending_config())
@@ -2579,7 +2579,7 @@ Status RaftConsensus::SetCommittedConfigUnlocked(const RaftConfigPB&
config_to_c
   TRACE_EVENT0("consensus", "RaftConsensus::SetCommittedConfigUnlocked");
   DCHECK(lock_.is_locked());
   DCHECK(config_to_commit.IsInitialized());
-  RETURN_NOT_OK_PREPEND(VerifyRaftConfig(config_to_commit, COMMITTED_CONFIG),
+  RETURN_NOT_OK_PREPEND(VerifyRaftConfig(config_to_commit),
                         "Invalid config to set as committed");
 
   // Compare committed with pending configuration, ensure that they are the same.

http://git-wip-us.apache.org/repos/asf/kudu/blob/bab96a2b/src/kudu/master/sys_catalog.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/sys_catalog.cc b/src/kudu/master/sys_catalog.cc
index 421e82e..9fb3885 100644
--- a/src/kudu/master/sys_catalog.cc
+++ b/src/kudu/master/sys_catalog.cc
@@ -92,7 +92,6 @@ DEFINE_double(sys_catalog_fail_during_write, 0.0,
               "Fraction of the time when system table writes will fail");
 TAG_FLAG(sys_catalog_fail_during_write, hidden);
 
-using kudu::consensus::COMMITTED_CONFIG;
 using kudu::consensus::ConsensusMetadata;
 using kudu::consensus::ConsensusMetadataManager;
 using kudu::consensus::ConsensusStatePB;
@@ -188,7 +187,7 @@ Status SysCatalogTable::Load(FsManager *fs_manager) {
     RETURN_NOT_OK_PREPEND(cmeta_manager_->Load(tablet_id, &cmeta),
                           "Unable to load consensus metadata for tablet " + tablet_id);
     ConsensusStatePB cstate = cmeta->ToConsensusStatePB();
-    RETURN_NOT_OK(consensus::VerifyRaftConfig(cstate.committed_config(), COMMITTED_CONFIG));
+    RETURN_NOT_OK(consensus::VerifyRaftConfig(cstate.committed_config()));
     CHECK(!cstate.has_pending_config());
 
     // Make sure the set of masters passed in at start time matches the set in
@@ -301,7 +300,7 @@ Status SysCatalogTable::CreateDistributedConfig(const MasterOptions&
options,
     }
   }
 
-  RETURN_NOT_OK(consensus::VerifyRaftConfig(resolved_config, consensus::COMMITTED_CONFIG));
+  RETURN_NOT_OK(consensus::VerifyRaftConfig(resolved_config));
   VLOG(1) << "Distributed Raft configuration: " << SecureShortDebugString(resolved_config);
 
   *committed_config = resolved_config;


Mime
View raw message