[jira] Commented: (THRIFT-638) BufferedTransport + C extensions block until recv timeout is reached on last fread call

["Nicholas Telford (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/THRIFT-638?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13001003#comment-13001003 ] 

Nicholas Telford commented on THRIFT-638:
-----------------------------------------

This problem is linked to the timeout issues discussed in THRIFT-347, and in fact has been exacerbated by the "fixes" applied for those tickets.

Specifically, THRIFT-347 was addressed by detecting timeouts with the following expression:
{code}
if (true === $md['timed_out'] && false === $md['blocked'])
{code}

The issue here is that $md['blocked'] denotes whether a stream is a blocking (true) or non-blocking (false) stream; since all TSocket streams are blocking (they never change the default, nor should they) this *always* evaluates to false: The result is, _TException is never thrown even for legitimate timeouts_.

This doesn't cause the problem itself, but it handily hides it away by making read() block excessively when there's not enough data to fill the fread() buffer instead of erroneously throwing a TException as it did before. It's also the reason the stream_socket family of functions are reportedly ignoring timeouts. They don't, TSocket is.

Chris correctly points out that this issue is resolved by replacing fread() calls with stream_socket_recv() calls, however, it's not a bug in PHP (the bug report linked is invalid). fread() is buffered by design - from the php.net docs: 

bq. Reading stops as soon as one of the following conditions is met: length bytes have been read, EOF (end of file) is reached or a packet becomes available or the socket timeout occurs (for network streams)

My understanding of TSocket is that it's meant as a raw socket abstraction and that any desired buffering should be done in a wrapping TTransport (e.g. TBufferedTransport or TFramedTransport), if that's the case then we should use stream_socket_recv() for all socket reading *and* stream_socket_sendto() for all socket writing.

> BufferedTransport + C extensions block until recv timeout is reached on last fread call
> ---------------------------------------------------------------------------------------
>
>                 Key: THRIFT-638
>                 URL: https://issues.apache.org/jira/browse/THRIFT-638
>             Project: Thrift
>          Issue Type: Bug
>          Components: PHP - Library
>    Affects Versions: 0.2
>            Reporter: Chris Goffinet
>             Fix For: 0.7
>
>         Attachments: 0001-Replace-freads-with-stream_socket_recvfrom.patch, 0002-tocket-read-meta-data-check.diff
>
>
> I wanted to throw this out if any other folks experience this later on. At Digg we've been using the BufferedTransport + C extension of Thrift in PHP. Every so often, we will see spikes in latency increases on RPC calls that we know have acceptable response times (<200ms). This seems to happen based on how much data is being sent back over the wire. This is more of a PHP problem, but can be corrected in Thrift's PHP library for folks who don't want to upgrade PHP. I am still waiting to see if it's corrected in later versions (we use 5.2.9).
> http://bonsai.php.net/bug.php?id=42720
> Replacing the fread statements in TSocket.php with stream_socket_recvfrom correct this behavior so that calls do not wait until they hit the recv timeout.

-- 
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira