Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9052 )
Change subject: KUDU-1927: no half-baked responses on ConnectoToMaster ...................................................................... Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/9052/5/src/kudu/master/master_service.cc File src/kudu/master/master_service.cc: http://gerrit.cloudera.org:8080/#/c/9052/5/src/kudu/master/master_service.cc@477 PS5, Line 477: if (cert_authority->is_initialized()) { isn't this guaranteed? leader_status() only is OK in the case that this node is the leader and that PrepareForLeadershipTask() has completed. That function unconditionally inits the CA. I tried running your tests with a CHECK(cert_authority->is_initialized()) here and it still passed. So perhaps that portion of this patch can be removed and the original call can be left in place, which assumes ca_cert_der() is always available. http://gerrit.cloudera.org:8080/#/c/9052/5/src/kudu/master/master_service.cc@507 PS5, Line 507: (!has_cert || (!has_authn_token && needs_authn_token))) { can we just simplify this to the approach outlined in the JIRA? ie: // Rather than consulting the current consensus role, instead // base it on the catalog manager's view. This prevents us // from advertising LEADER until we have taken over all the // associated responsibilities. resp->set_role(l.leader_status().ok() ? LEADER : FOLLOWER); -- To view, visit http://gerrit.cloudera.org:8080/9052 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9c906863f5f0e1995041281b122135e1b2cd3a4 Gerrit-Change-Number: 9052 Gerrit-PatchSet: 5 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Thu, 25 Jan 2018 00:02:30 +0000 Gerrit-HasComments: Yes
