Todd Lipcon has posted comments on this change. Change subject: KUDU-1927 [master] always send IPKI CA certificate ......................................................................
Patch Set 4: (7 comments) http://gerrit.cloudera.org:8080/#/c/6356/4/src/kudu/integration-tests/master_cert_authority-itest.cc File src/kudu/integration-tests/master_cert_authority-itest.cc: PS4, Line 345: builder : .default nit: funny newline (usually we don't do this for builders elsewhere) Line 351: tls_context().trusted_cert_count_for_tests()); nit: unnecessary wrap PS4, Line 356: const MonoTime t_stop = MonoTime::Now() + : MonoDelta::FromSeconds(run_time_seconds_); : same PS4, Line 397: Besides In addition http://gerrit.cloudera.org:8080/#/c/6356/4/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: PS4, Line 748: MAYBE_INJECT_RANDOM_LATENCY( : FLAGS_catalog_manager_inject_latency_load_ca_info_ms); nit: no need to wrap, right? http://gerrit.cloudera.org:8080/#/c/6356/4/src/kudu/master/master_service.cc File src/kudu/master/master_service.cc: PS4, Line 424: resp->set_role(RaftPeerPB::UNKNOWN_ROLE); : resp->mutable_error()->set_code(MasterErrorPB::NOT_THE_LEADER); : StatusToPB(leader_status, resp->mutable_error()->mutable_status()); : VLOG(3) << "Detected transient state; responding with: " : << leader_status.ToString(); sorta curious why we don't just treat this the same as still being a follower? i.e. set_role(FOLLOWER) and just merge with the above case? PS4, Line 456: resp->mutable_error()->set_code(MasterErrorPB::NOT_THE_LEADER); : const Status resp_status(Status::ServiceUnavailable( : "master server could not generate authn token: $0", s.ToString())); : odd to be using NOT_THE_LEADER here... here we are the leader (the leader status said OK) but we still couldn't issue a token, which sounds like something's quite wrong, no? Is it worth the client retrying vs fail fast? -- To view, visit http://gerrit.cloudera.org:8080/6356 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib9a20e6da6531091fcc597af58a7bbf01a8847f5 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
