Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9642 )
Change subject: Factor out consensus queue methods ...................................................................... Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/9642/1/src/kudu/consensus/consensus_queue.cc File src/kudu/consensus/consensus_queue.cc: http://gerrit.cloudera.org:8080/#/c/9642/1/src/kudu/consensus/consensus_queue.cc@945 PS1, Line 945: more_pending In the context of this function, I don't think 'more_pending' is a great name. Really this is just indicating that we hit an LMP mismatch and therefore need to retrigger a new request immediately, right? Let's rename it to something that makes sense given the purpose of this function It's also scary that it is set true here, set false below for INVALID_TERM, but left untouched for the non-error case up above. I think the out-param should be always set one way or another http://gerrit.cloudera.org:8080/#/c/9642/1/src/kudu/consensus/consensus_queue.cc@965 PS1, Line 965: void PeerMessageQueue::PromoteIfNeeded(TrackedPeer* peer, const TrackedPeer& prev_peer_state, > warning: parameter 'prev_peer_state' is unused [misc-unused-parameters] I'm OK with you leaving this since you're planning on addressing in the next commit -- To view, visit http://gerrit.cloudera.org:8080/9642 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I278de150e3dc42181ccfbbd3a4c0e5cc4de90c1a Gerrit-Change-Number: 9642 Gerrit-PatchSet: 1 Gerrit-Owner: Mike Percy <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Thu, 15 Mar 2018 00:14:44 +0000 Gerrit-HasComments: Yes
