Will Berkeley has posted comments on this change. Change subject: Refactor ConsensusStatePB to hold committed and pending configs ......................................................................
Patch Set 6: (7 comments) Also hit all the comments that somehow got missed from PS6. http://gerrit.cloudera.org:8080/#/c/6809/6/src/kudu/consensus/metadata.proto File src/kudu/consensus/metadata.proto: PS6, Line 103: There is a corner case in Raft where a node may be elected leader of a : // pending (uncommitted) configuration. In such a case, the master will : // detect that the node is not a member of the committed configuration. > Not done yet in rev 8 Sorry. Done. PS6, Line 113: 5 > Not done in rev 8 Sorry. Done. 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", > I actually don't think it's possible to hit this in the current production Done. Omitting redundant checks for required fields and the leader check, the only thing that remains is to check that the committed_config is valid and that we're not storing a pending_config. Line 219: string DiffConsensusStates(const ConsensusStatePB& old_state, > nit: don't indent inside namespace Done Line 221: bool leader_changed = old_state.leader_uuid() != new_state.leader_uuid(); > Add a comment about the structure of this map (key and value contents?) Done Line 304: if (SecureShortDebugString(old_state) == SecureShortDebugString(new_state)) { > nit: 4 line indentation for a continued line per https://google.github.io/s Done PS8, Line 332: > gained/lost is a bit strange terminology here, since it sounds sort of like Done -- 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: 6 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
