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

Nevo Hed commented on THRIFT-1302:
----------------------------------

Questions to those involved in this patch - 
I may not have a full understanding of all the layers involved, but

Wouldn't we want to "close()" the socket before we throw the exception?

Just concerned that partial data may have been written already to the socket.

I see that TSocket::write_partial does close on some errors, but not all.  If 
we already wrote something - I am not sure how the client/server can keep in 
sync with a partial message and no tracking of the remaning bytes needed to 
complete it.
                
>  thrift: raise an exception if send() times out in

> ---------------------------------------------------
>
>                 Key: THRIFT-1302
>                 URL: https://issues.apache.org/jira/browse/THRIFT-1302
>             Project: Thrift
>          Issue Type: Improvement
>          Components: C++ - Library
>            Reporter: Dave Watson
>            Assignee: Dave Watson
>            Priority: Minor
>             Fix For: 0.8
>
>         Attachments: 
> 0022-thrift-raise-an-exception-if-send-times-out-in-TSock.patch
>
>
> From 1f76284b5972ab01d6f6ac68f96024a8066a3b59 Mon Sep 17 00:00:00 2001
> From: Adam Simpkins <[email protected]>
> Date: Fri, 16 Apr 2010 17:43:21 +0000
> Subject: [PATCH 22/33] thrift: raise an exception if send() times out in
>  TSocket::write()
> Summary:
> Previously, if send() timed out in TSocket::write(), it would sleep for
> 50 microseconds and retry.  This essentially made the timeout set with
> setSendTimeout() useless.  Now it raises a TTransportException on
> timeout.
> TNonblockingServer does use TSocket with the fd manually put in
> non-blocking mode, and that could cause EWOULDBLOCK and EAGAIN to occur
> during normal operation.  However, it only uses write_partial() and
> never write(), so it should be safe to have write() throw the exception.
> Test Plan:
> Used the client and server in thrift/tutorial/cpp/async/sort/ to test
> sending a large message to the server.  I set a 100ms timeout in the
> client, and verified that it correctly times out now if the server
> process is stopped.
> I also ran [fb unittests] to try and verify that this doesn't negatively
> affect any other code.
> Revert Plan:
> OK
> ---
>  lib/cpp/src/transport/TSocket.cpp |    7 ++++---
>  1 files changed, 4 insertions(+), 3 deletions(-)

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to