[jira] Issue Comment Edited: (THRIFT-845) async client does not respect timeout

["Ning Liang (JIRA)" <ji...@apache.org>]

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

Ning Liang edited comment on THRIFT-845 at 8/19/10 3:33 AM:
------------------------------------------------------------

How about:

{code}
    // Timeout any existing method calls
    private void timeoutIdleMethods() {
      Iterator<TAsyncMethodCall> iterator = timeoutWatchSet.iterator();
      while (iterator.hasNext()) {
        TAsyncMethodCall methodCall = iterator.next();
        long clientTimeout = methodCall.getClient().getTimeout();
        long timeElapsed = System.currentTimeMillis() - methodCall.getLastTransitionTime();
        if (timeElapsed > clientTimeout) {
          iterator.remove();
          String message = "Operation " + methodCall.getClass() + " timed out after " + timeElapsed + " milliseconds.";
          methodCall.onError(new TimeoutException(message));
        }
      }
    }
{code}

      was (Author: ning):
    How about:

    // Timeout any existing method calls
    private void timeoutIdleMethods() {
      Iterator<TAsyncMethodCall> iterator = timeoutWatchSet.iterator();
      while (iterator.hasNext()) {
        TAsyncMethodCall methodCall = iterator.next();
        long clientTimeout = methodCall.getClient().getTimeout();
        long timeElapsed = System.currentTimeMillis() - methodCall.getLastTransitionTime();
        if (timeElapsed > clientTimeout) {
          iterator.remove();
          String message = "Operation " + methodCall.getClass() + " timed out after " + timeElapsed + " milliseconds.";
          methodCall.onError(new TimeoutException(message));
        }
      }
    }
  
> async client does not respect timeout
> -------------------------------------
>
>                 Key: THRIFT-845
>                 URL: https://issues.apache.org/jira/browse/THRIFT-845
>             Project: Thrift
>          Issue Type: Bug
>          Components: Library (Java)
>    Affects Versions: 0.4, 0.5
>            Reporter: Eric Jensen
>             Fix For: 0.5
>
>         Attachments: async_timeout.diff
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> there effectively is no timeout on calls via the async client, so the consumer may never be called back with either a success or error.  this is not an awesome contract, and it also leaves the potential for the client itself to leak.  ning is working on a patch for this based on the below discussion:
> implementing the timeout inside TAsyncClientManager seems pretty easy, we would just loop through Selector.keys() there and make sure none of them have been waiting for longer than the SO_TIMEOUT we set on its TNonblockingSocket, if one is, we just do methodCall.onError on it to make sure the caller is notified.
> We also need to change Selector.select() to Selector.select(long timeout), with a small value so that the selector thread wakes up with sufficient granularity to timeout calls.
> I think we can just track the timeout of the pending operation within TAsyncMethodCall itself and loop over Selector.keys() to check them.  But since we just made the change to leave the keys for all open connections registered with the selector until they are closed, this could be somewhat costly.
> For reference, here's how jetty does this (http://grepcode.com/file/repo1.maven.org/maven2/org.eclipse.jetty/jetty-io/8.0.0.M0/org/eclipse/jetty/io/nio/SelectorManager.java#SelectorManager) which i do _not_ encourage you to read, but i am confident from reading it that it's doing just what i said:  tracking the list of outstanding read/write operations and their timeouts themselves, looping through them within the selector thread, and closing their sockets if their so-called "idle timeout" has expired.
> Also, there are a couple java selector bugs jetty links to that we should be aware of, the only one applicable to us has been fixed in jdk 1.6 update 18 (the latest is 21).  

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.