Juan Yu has posted comments on this change. Change subject: IMPALA-3575: Add retry to backend connection request and rpc timeout ......................................................................
Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/3343/1/be/src/runtime/client-cache.h File be/src/runtime/client-cache.h: PS1, Line 242: e.getType() == apache::thrift::transport::TTransportException::TIMED_OUT > This IsTimeoutTException checks error string "EAGAIN (timed out)", this onl Sorry I misunderstand the timeout handling here. I'll revert the exception handling here. Correct me if I am wrong We want to return error is timeout happens at recv call which means send is done and error or exception at thrift server side. and we want to retry the whole RPC for all the rest scenarios (mainly resource unavailable cases) http://gerrit.cloudera.org:8080/#/c/3343/1/be/src/runtime/exec-env.cc File be/src/runtime/exec-env.cc: PS1, Line 134: DEFINE_int32(backend_client_send_timeout_ms, 1000, "(Advanced) The time after " : "which a backend client send RPC call will timeout."); > we can just use a single one According to my testing, a single receive timeout might not be sufficient. If sending timeout, we might want to fail the query or retry the whole RPC, especially for control messages, like CancelPlanFragment If recv timeout, we should just return error. But we don't want error out accidentally, e.g. data stream sender might need to wait for long time due to back pressure. I am going to add a loop for recv_TransmitData() to make sure we wait long enough. Does this make sense? -- To view, visit http://gerrit.cloudera.org:8080/3343 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id6723cfe58df6217f4a9cdd12facd320cbc24964 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Juan Yu <[email protected]> Gerrit-Reviewer: Alan Choi <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Juan Yu <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-HasComments: Yes
