Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/18661 )

Change subject: IMPALA-11384: Upgrade CPP thrift components to thrift-0.16.0
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18661/2/be/src/rpc/thrift-util.cc
File be/src/rpc/thrift-util.cc:

http://gerrit.cloudera.org:8080/#/c/18661/2/be/src/rpc/thrift-util.cc@164
PS2, Line 164: bool IsReadTimeoutTException(const TTransportException& e) {
             :   // String taken from TSocket::read() Thrift's TSocket.cpp and 
TSSLSocket.cpp.
             :   return (e.getType() == TTransportException::TIMED_OUT &&
             :              strstr(e.what(), "EAGAIN (timed out)") != nullptr) 
||
             :          (e.getType() == TTransportException::INTERNAL_ERROR &&
             :              strstr(e.what(), "SSL_read: Resource temporarily 
unavailable") != nullptr);
             : }
             :
             : bool IsPeekTimeoutTException(const TTransportException& e) {
             :   // String taken from TSocket::peek() Thrift's TSocket.cpp and 
TSSLSocket.cpp.
             :   return (e.getType() == TTransportException::UNKNOWN &&
             :              strstr(e.what(), "recv(): Resource temporarily 
unavailable") != nullptr) ||
             :          (e.getType() == TTransportException::INTERNAL_ERROR &&
             :              strstr(e.what(), "SSL_peek: Resource temporarily 
unavailable") != nullptr);
             : }
             :
             : bool IsConnResetTException(const TTransportException& e) {
             :   // Strings taken from TTransport::readAll(). This happens iff 
TSocket::read() returns 0.
             :   // As readAll() is reading non-zero length payload, this can 
only mean recv() called
             :   // by read() returns 0. According to man page of recv(), this 
implies a stream socket
             :   // peer has performed an orderly shutdown.
             :   return (e.getType() == TTransportException::END_OF_FILE &&
             :              strstr(e.what(), "No more data to read.") != 
nullptr) ||
             :          (e.getType() == TTransportException::INTERNAL_ERROR &&
             :              strstr(e.what(), "SSL_read: Connection reset by 
peer") != nullptr);
             : }
> These exceptions are one thing that can change with newer Thrift versions.
I look around and it does not seem that we need to change this.
The only failing test after upgrade is TestGracefulShutdown.test_shutdown_idle, 
and it is addressed by change in client-request-state.cc.



--
To view, visit http://gerrit.cloudera.org:8080/18661
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic278ac5c973ff5c3e829a6139b9c16e9d2c62a59
Gerrit-Change-Number: 18661
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Reviewer: Yida Wu <[email protected]>
Gerrit-Comment-Date: Thu, 23 Jun 2022 20:29:53 +0000
Gerrit-HasComments: Yes

Reply via email to