Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/14246 )
Change subject: IMPALA-8634: Catalog client should retry RPCs ...................................................................... Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/14246/1/be/src/runtime/exec-env.cc File be/src/runtime/exec-env.cc: http://gerrit.cloudera.org:8080/#/c/14246/1/be/src/runtime/exec-env.cc@142 PS1, Line 142: DEFINE_int32(catalog_client_connection_num_retries, 3, "The number of times connections " > Yeah, 10 seconds does seem a little long. How about 10 retries, with an int Seems reasonable to me. http://gerrit.cloudera.org:8080/#/c/14246/1/be/src/runtime/exec-env.cc@144 PS1, Line 144: 0 We may want to consider setting this timeout to non-zero at some point. When we convert the Catalog clients to use KRPC, we may be able to rely on using TCP_USER_TIMEOUT which seems more appropriate than socket timeout. http://gerrit.cloudera.org:8080/#/c/14246/1/be/src/runtime/exec-env.cc@180 PS1, Line 180: 1, 0 May help to name these constants as variables. IIUC, this change relies on the sleep and retry in the retry loop of DoRpcWithRetry() so simply don't try recreating new connections on failure now. -- To view, visit http://gerrit.cloudera.org:8080/14246 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7f33ad2b36d301fb64e70a939e71decab0ca993c Gerrit-Change-Number: 14246 Gerrit-PatchSet: 1 Gerrit-Owner: Sahil Takiar <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Sahil Takiar <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Wed, 18 Sep 2019 06:52:15 +0000 Gerrit-HasComments: Yes
