Mike Percy has posted comments on this change. Change subject: Make ConsensusMetadata thread-safe ......................................................................
Patch Set 9: (3 comments) http://gerrit.cloudera.org:8080/#/c/6958/9/src/kudu/consensus/consensus_meta.cc File src/kudu/consensus/consensus_meta.cc: PS9, Line 135: lock_guard<Mutex> l(lock_); : switch (type) { : case ACTIVE_CONFIG: : return IsRaftConfigVoter(uuid, active_config_unlocked()); : case COMMITTED_CONFIG: : return IsRaftConfigVoter(uuid, committed_config_unlocked()); : case PENDING_CONFIG: : return IsRaftConfigVoter(uuid, pending_config_unlocked()); : default: : LOG(FATAL) << "Unsupported RaftConfigState: " << type; : } > could this and the three functions below be simpler if you added something Good idea. Done. http://gerrit.cloudera.org:8080/#/c/6958/9/src/kudu/consensus/raft_consensus.cc File src/kudu/consensus/raft_consensus.cc: Line 841: Status RaftConsensus::IsSingleVoterConfig(bool* single_voter) const { > mind adding a TODO here that there's no reason for this to return a Status? I just fixed the signature Line 2345: int64_t committed_config_opid_index = cmeta_->GetConfigOpIdIndex(COMMITTED_CONFIG); > isn't this equivalent to committed_config.opid_index() now? seems unnecessa ah, yeah, I meant to remove line 2344 -- To view, visit http://gerrit.cloudera.org:8080/6958 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id2c70605c0593e55486184705ea448dcb4bef2d7 Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
