Todd Lipcon has posted comments on this change. Change subject: KUDU-1580 retry tserver RPC if negotiation times out ......................................................................
Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/6926/1/src/kudu/rpc/connection.cc File src/kudu/rpc/connection.cc: PS1, Line 170: const Status reported_status = status.IsTimedOut() : ? Status::Incomplete("RPC is not sent due to connection error", : status.ToString()) : : status; : this seems like a very risky change to me, because now anywhere in the whole codebase where we were checking for IsTimedOut may now have an unexpected Incomplete status instead. Perhaps we would be better off giving the RpcController and OutboundCall object some more getters to try to distinguish underlying specific causes for different timeouts? eg call->SetFailed(status, NEGOTIATION_TIMED_OUT, error.release()) or whatever? -- To view, visit http://gerrit.cloudera.org:8080/6926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icee8bf4978365a23d6627e7bc411b63f53540a3b Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
