impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Hecht (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5537: Retry RPC on somes exceptions with SSL connection
Date Wed, 21 Jun 2017 00:13:43 GMT
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 <kwho@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message