Todd Lipcon has posted comments on this change.

Change subject: consensus: Get rid of ReplicaState class
......................................................................


Patch Set 5:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/7007/5//COMMIT_MSG
Commit Message:

Line 12: There are no functional changes in this patch.
can you loop raft_consensus-itest for this and report the results?


http://gerrit.cloudera.org:8080/#/c/7007/5/src/kudu/consensus/raft_consensus.cc
File src/kudu/consensus/raft_consensus.cc:

Line 1605:     local_peer_uuid = peer_uuid_;
isn't this not protected by the lock?


PS5, Line 2463: ConsensusStatePB RaftConsensus::ConsensusStateUnlocked() const {
              :   return cmeta_->ToConsensusStatePB();
              : }
              : 
              : RaftPeerPB::Role RaftConsensus::GetActiveRoleUnlocked() const {
              :   DCHECK(lock_.is_locked());
              :   return cmeta_->active_role();
              : }
              : 
              : bool RaftConsensus::IsConfigChangePendingUnlocked() const {
              :   DCHECK(lock_.is_locked());
              :   return cmeta_->has_pending_config();
              : }
some of these could probably just be inlined into callsites now that they're so 
trivial (separate patch is fine)


Line 2509:   cmeta_->clear_pending_config();
same


PS5, Line 2578: const int64_t RaftConsensus::GetCurrentTermUnlocked() const {
              :   DCHECK(lock_.is_locked());
              :   return cmeta_->current_term();
              : }
              : 
              : const string& RaftConsensus::GetLeaderUuidUnlocked() const {
              :   DCHECK(lock_.is_locked());
              :   return cmeta_->leader_uuid();
              : }
              : 
              : const bool RaftConsensus::HasVotedCurrentTermUnlocked() const {
              :   DCHECK(lock_.is_locked());
              :   return cmeta_->has_voted_for();
              : }
same with these


http://gerrit.cloudera.org:8080/#/c/7007/5/src/kudu/consensus/raft_consensus.h
File src/kudu/consensus/raft_consensus.h:

Line 65:   enum State {
does this need to be public?


Line 82:   typedef std::unique_lock<simple_spinlock> UniqueLock;
same


-- 
To view, visit http://gerrit.cloudera.org:8080/7007
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie1e62eff37d3f8655100b364939375608063aa80
Gerrit-PatchSet: 5
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: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <[email protected]>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to