Dan Hecht has posted comments on this change. Change subject: IMPALA-5537: Retry RPC on somes exceptions with SSL connection ......................................................................
Patch Set 5: Code-Review+2 (3 comments) http://gerrit.cloudera.org:8080/#/c/7229/5/be/src/rpc/thrift-util.cc File be/src/rpc/thrift-util.cc: PS5, Line 198: This is not very robust as it's : // possible that the receiver of the RPC call may have received all the RPC payload : // but the ACK to the sender may have been dropped somehow. In which case, it's : // not safe to retry the RPC if it's not idempotent. maybe reword this to say the caller should first check if the send was completed, or something? now that we do that. PS5, Line 202: end-to-end tracking of RPC calls to detect duplicated calls in the receiver side what does that mean? is it still applicable? http://gerrit.cloudera.org:8080/#/c/7229/5/be/src/runtime/client-cache.h File be/src/runtime/client-cache.h: Line 330: client_is_unrecoverable_ = false; i guess this restores the behavior before the patch that introduced this RetryRpc() subroutine? okay. -- To view, visit http://gerrit.cloudera.org:8080/7229 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8243d4cac93c453e9396b0e24f41e147c8637b8c Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-HasComments: Yes
