Alexey Serbin 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 nod It seems to be true, and it seems the original version has never aborted because of that even if similar CHECK() was in MasterCertAuthority::ca_cert_der() method. But it seemed a little bit brittle to rely on that. What's wrong with explicit check of MasterCertAuthority::is_initialized()? With that, it's not required to remember whether initialized CA authority is guaranteed if l.leader_status().ok(). 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: The crucial point here was also to make sure the authn token is issued, if needed. Don't we need that assurance? What JIRA is that, BTW? I could not find that piece in KUDU-1927 (nor in KUDU-1924). -- 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:55:58 +0000 Gerrit-HasComments: Yes
