Alexey Serbin has posted comments on this change. Change subject: server_negotiation: improve error handling ......................................................................
Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/6154/1/src/kudu/rpc/server_negotiation.cc File src/kudu/rpc/server_negotiation.cc: PS1, Line 562: > Could you expand on this? Holding a reference to an rvalue seems like a bug It's not a bug once it's const ref: this is perfectly fine since the language has specific clause that guarantees that if you bind a reference to an rvalue a temporary is created and the lifetime is extended until the end of the scope where the reference is created. We have been doing that that in our RETURN_NOT_OK() macro for a long time already. However, if you find this misleading, then maybe just const Status s = ...; would be better. PS1, Line 567: case security::VerificationResult::UNKNOWN_SIGNING_KEY: > I was imagining that this would happen if the tablet server had been partit I do not see any other scenarios besides that partitioning scenario and the very first start of the cluster (the latter could be considered just as a sub-case of the former, actually). http://gerrit.cloudera.org:8080/#/c/6154/2/src/kudu/rpc/server_negotiation.cc File src/kudu/rpc/server_negotiation.cc: PS2, Line 567: case security::VerificationResult::UNKNOWN_SIGNING_KEY: Do you think the partitioning scenario could not get such a timing when retry could help in case when the server receives the needed token verification key a little bit later than the corresponding request from a client? -- To view, visit http://gerrit.cloudera.org:8080/6154 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I64f9c0f59aa608bf5078d65883a7d9a6fb186c04 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
