[ 
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

        

Reply via email to