Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handling read timeout #15

Closed
allenxwang opened this issue Jan 9, 2014 · 14 comments
Closed

Handling read timeout #15

allenxwang opened this issue Jan 9, 2014 · 14 comments

Comments

@allenxwang
Copy link
Collaborator

Netty's handling of read timeout is not very intuitive: you need to add a ReadTimeoutHandler in the pipeline. However, channel read can still happen even if ReadTimeoutHandler has thrown out a ReadTimeoutException down the pipe.

In the blocking socket, setting read timeout ensures that socket resources will be released after the timeout is passed. How does the read timeout play a role in non-blocking socket?

Do we still care about read timeout ? Should it be handled in RxNetty, Ribbon, or through Observable APIs by the caller?

@NiteshKant
Copy link
Member

Netty's read timeout handler closes the channel on read timeout, which
essentially is releasing resources per se. So you should add a read timeout
handler in your pipeline irrespective of whether it's a blocking or
non-blocking I/O.

On Wednesday, January 8, 2014, allenxwang wrote:

Netty's handling of read timeout is not very intuitive: you need to add a
ReadTimeoutHandler in the pipeline. However, channel read can still happen
even if ReadTimeoutHandler has thrown out a ReadTimeoutException down the
pipe.

In the blocking socket, setting read timeout ensures that socket resources
will be released after the timeout is passed. How does the read timeout
play a role in non-blocking socket?

Do we still care about read timeout ? Should it be handled in RxNetty,
Ribbon, or through Observable APIs by the caller?


Reply to this email directly or view it on GitHubhttps://github.com//issues/15
.

@allenxwang
Copy link
Collaborator Author

OK. The actual problem with ReadTimeoutHandler is in this issue:

netty/netty#1840

Basically ReadTimeoutException gets thrown even after channel read happens.

@benjchristensen
Copy link
Member

If we use Observable.timeout and it cancels the connection, everything should work correctly as well.

@NiteshKant
Copy link
Member

With the read timeout handler there should at least be some bytes read
every timeout interval till you close the channel. It does not matter if
there has been some available bytes once.
Is this the behavior you are seeing?

On Wednesday, January 8, 2014, allenxwang wrote:

OK. The actual problem with ReadTimeoutHandler is in this issue:

netty/netty#1840 netty/netty#1840

Basically ReadTimeoutException gets thrown even after channel read happens.


Reply to this email directly or view it on GitHubhttps://github.com//issues/15#issuecomment-31902581
.

@allenxwang
Copy link
Collaborator Author

The behavior I am expecting is that once channel read starts within the specified timeout, no ReadTimeoutException should be fired. But in reality, I always get ReadTimeoutException if you wait long enough (after the specified timeout).

@benjchristensen
Copy link
Member

The behavior I am expecting is that once channel read starts within the specified timeout, no ReadTimeoutException should be fired. But in reality, I always get ReadTimeoutException if you wait long enough (after the specified timeout).

That's definitely not what we want from the Netty layer. At the Rx layer using Observable.timeout that's what I'd expect as that is for the full sequence (IO + computation).

@NiteshKant
Copy link
Member

I had a chat with Allen.
The scenario here is that the connection is a persistent connection & there is no activity on the connection after the response is received, which is the expected behavior of the timeout handler.

The correct way of using this handler would be to add it when you are expecting a response (request write complete) and remove it when response is done (full response received)

@benjchristensen
Copy link
Member

What would the syntax look like for that?

Since we're embracing Observable in here, perhaps we just leverage Observable.timeout so the user decides when they want a timeout and it will correctly cancel the underlying Netty Future?

How does this affect connection pooling and persistent connections?

@allenxwang
Copy link
Collaborator Author

The timeout we can put on Observable is the "response" timeout, not the network timeout. i.e., it includes the time to set up pipeline, establish the connection and write the request. So I would say it is a slightly different concept. It can be used on behalf of the caller to govern the perceived response time.

If the purpose of read timeout is to protect socket resources, I feel the ReadTimeoutHandler is a better choice. For connection pool, this handler must be removed before a persistent connection is returned to pool.

@benjchristensen
Copy link
Member

Ah okay. Makes sense, I mistook this to be the response timeout since it was stated as "read" not "connect" timeout.

In Netty is there something in between, or is this the "connect" timeout we're discussing?

allenxwang referenced this issue in allenxwang/RxNetty Jan 10, 2014
…ndler interface so that caller can get notified on channel operations.

Address Issue #15 by introducing self removing read timeout handler.

Added more junit tests for connect timeout and read timeout.
@allenxwang
Copy link
Collaborator Author

Talked with Ben. Users of RxNetty can set Observable timeout and when it happens, ObservableHttpClient automatically closes the channel.

On the other hand, we have advanced users and library developers (like ribbon) who want to set specific network timeout for connect and read. So we still need to support such use cases by using Netty APIs.

@benjchristensen
Copy link
Member

Yup. Thanks Allen.

@NiteshKant
Copy link
Member

After this refactoring: #19

RxClient supports read timeouts such that if there is more than configured time lapsed between reading data from the socket buffer, it will result in a timeout. The implementation uses Netty's ReadTimeoutHandler as the first handler in the pipeline. This handler is added after any write completes on the channel & removed after the request processing is over or the connection is closed.

Now, one can achieve:

  1. Socket level read timeout via the above.
  2. Complete request processing timeout using Rx's timeout.

Do, we need the functionality of response read timeout i.e. 2) above, excluding time spent in sending request?

@NiteshKant
Copy link
Member

Chatted with Allen offline. We don't need the response read timeout functionality which excludes request send time. One can just use Rx's timeout.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants