Client: go
In compiler generated TProcessorFunction implementations, add a
goroutine after read the request to do connectivity check on the input
transport. If the transport is no longer open, cancel the context object
passed into the handler implementation.
Also define ErrAbandonRequest error, to help TSimpleServer closing
client connections that's already closed on the other end.
Client: go
We separated timeout in go's TSocket into connect timeout and socket
timeout in 81334cd, this change does the same for TSSLSocket to keep
them consistent.
Also rename the arg in NewTSocketFromConnTimeout from connTimeout to
socketTimeout, because in that function we already have a connection,
so connect timeout is never used again. The timeout passed into that
function is really for socket timeout, not connect timeout. The name of
that function actually means "New TSocket From Conn (with) Timeout", not
"New TSocket From ConnTimeout" (I guess that's where the original
confusion came from).
Also add the missing change note for the breaking change.
Client: go
As discussed in the JIRA ticket, this commit changes how we handle I/O
timeouts in the go library.
This is a breaking change that adds context to all Read*, Write*, and
Skip functions to TProtocol, along with the compiler change to support
that, and also adds context to TStandardClient.Recv, TDeserializer,
TStruct, and a few others.
Along with the function signature changes, this commit also implements
context cancellation check in the following TProtocol's ReadMessageBegin
implementations:
- TBinaryProtocol
- TCompactProtocol
- THeaderProtocol
In those ReadMessageBegin implementations, if the passed in context
object has a deadline attached, it will keep retrying the I/O timeout
errors, until the deadline on the context object passed. They won't
retry I/O timeout errors if the passed in context does not have a
deadline attached (still return on the first error).
Client: go
As discussed in the JIRA ticket, this commit changes how we handle I/O
timeouts in the go library.
This is a breaking change that adds context to all Read*, Write*, and
Skip functions to TProtocol, along with the compiler change to support
that, and also adds context to TStandardClient.Recv, TDeserializer,
TStruct, and a few others.
Along with the function signature changes, this commit also implements
context cancellation check in the following TProtocol's ReadMessageBegin
implementations:
- TBinaryProtocol
- TCompactProtocol
- THeaderProtocol
In those ReadMessageBegin implementations, if the passed in context
object has a deadline attached, it will keep retrying the I/O timeout
errors, until the deadline on the context object passed. They won't
retry I/O timeout errors if the passed in context does not have a
deadline attached (still return on the first error).
Client: go
This is a slightly different, and less error-prone approach from the
fix in e382275b.
The previous approach relies on passing the set socket timeout into the
underlying socketConn from TSocket and TSSLSocket. But since we have so
many different constructors for TSocket and TSSLSocket, some makes the
initial connection in the constructor and some does not, there are so
many different places we would need to remember to pass socketTimeout
into socketConn. In the future, when we add another constructor to them,
we could either forget to pass the socket timeout into socketConn, or
try to pass it while we haven't constructed socketConn yet (which will
cause panic), both are bad.
In this approach we just clear the read deadline in the connectivity
check read. Because that's a non-blocking read, it would work just fine
without a read deadline.
Client: go
We added socketConn to go library for connectivity check in
https://github.com/apache/thrift/pull/2153, but forgot to push read
deadline on the socket when doing the connectivity checks. This caused
the issue of large number of connectivity checks to fail with I/O
timeout errors.
Client: go
While debugging the issue in THRIFT-5214, I original thought that was a
bug in TFramedTransport implementation, so I took some time scrutinize
the TFramedTransport code. Although in the end there's no bug, the
current implementation of TFramedTransport, especially in the Read
function, has some weird handling while at frame boundary, which is both
error-prone and hard to follow (I did found and fixed one bug there in
https://github.com/apache/thrift/pull/1810 before).
The new implementation reads the whole frame into a buffer, which would
use slightly more memory, but it follows the pattern of TFramedTransport
implementation of other languages (e.g. Java), and also the pattern we
handle frame in THeaderTransport. It also reduced the complexity and
weirdness of the frame boundary handling in Read implementation.
This rewrite also removes the print call from library code.
Client: go
In THeaderTransport implementation, use io.CopyN instead of
io.Copy+io.LimitReader.
Underlying io.CopyN is actually implemented with io.Copy+io.LimitReader
[0], but it also does some extra checks we can take advantage of. It
also simplifies the code in thrift repo.
[0]: 83b181c68b/src/io/io.go (L340)
Client: java
Patch: David Mollitor
This closes#2137
Use SLF4J API to log full Exception details. Use SLF4J parameterized logging instead of String format.
