Alexey Serbin has posted comments on this change.

Change subject: KUDU-1927 [master] always send IPKI CA certificate
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/6356/1/src/kudu/master/master_service.cc
File src/kudu/master/master_service.cc:

PS1, Line 401: (
> Nit: don't need extra parens here.
Done


PS1, Line 410:     const Status s(Status::ServiceUnavailable(
             :         "not yet ready to serve the request as the leader 
master"));
> Just respond with l.leader_status() directly.
I think we want to have the response to be explicit ServiceUnavailable, right?  
If sending back whatever we get from l.leader_status(), it might not so easy to 
handle that at the client side, I think.

However, I think it's worth adding information from the status of the leader 
into the error string.


Line 415:     LOG(INFO) << "Detected transient state; responding with: " << 
s.ToString();
> Probably overkill.
Yep, that's fair.  I replaced it with VLOG(3).


-- 
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: 1
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: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to