Adar Dembo has posted comments on this change. Change subject: consensus: KUDU-2147. Unknown leader should not be treated as valid UUID ......................................................................
Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/8109/1/src/kudu/integration-tests/raft_config_change-itest.cc File src/kudu/integration-tests/raft_config_change-itest.cc: PS1, Line 56: The : // master should update its record of which replica is the leader after a new : // leader is elected. Which part of the test asserts this? Would the last WaitForServersToAgree fail? Line 96: TServerDetails* leader = nullptr; Nit: don't need to initialize this; it'll always be written to by FindTabletLeader on success. Line 113: // Leader delays heartbeats for 2 sec; followers delay by even longer. You already wrote this on L102. PS1, Line 114: if (cluster_->tablet_server(i)->uuid() != leader->uuid()) { : followers.push_back(ts_map_[cluster_->tablet_server(i)->uuid()]); : } How about doing this work in the loop on L101? http://gerrit.cloudera.org:8080/#/c/8109/1/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: If the conclusion is that cstate.has_leader_uuid() is untrustworthy, there are other calls to it outside catalog_manager.cc. For example, quorum_util.cc, master-path-handlers.cc, ksck.cc, etc. Shouldn't all of them be updated too? -- To view, visit http://gerrit.cloudera.org:8080/8109 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie882d05fc58e55836edc0235d14974e65125df6c Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
