Todd Lipcon has posted comments on this change. Change subject: Refactor ConsensusStatePB to hold committed and pending configs ......................................................................
Patch Set 8: (4 comments) http://gerrit.cloudera.org:8080/#/c/6809/8/src/kudu/consensus/quorum_util.cc File src/kudu/consensus/quorum_util.cc: PS8, Line 197: return Status::IllegalState( : Substitute("Leader with UUID $0 is not a VOTER in the committed or pending config! " : "Consensus state: $1", hrm, isn't this possible? eg when catching up a node that is more than one config change behind, you might have a leader which isn't in its committed or pending config yet? Line 219: namespace { nit: don't indent inside namespace Line 221: typedef map<string, pair<RaftPeerPB, RaftPeerPB>> PeerInfoMap; Add a comment about the structure of this map (key and value contents?) PS8, Line 332: gained a gained/lost is a bit strange terminology here, since it sounds sort of like there's a list. Maybe just "changed pending config" or "now has pending config" or somesuch -- To view, visit http://gerrit.cloudera.org:8080/6809 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4bc4bdd9752fc29a7ce2cefcdc95c4366f5353af Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley <wdberke...@gmail.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-HasComments: Yes
