| Summary: | Support for a total timeout | ||
|---|---|---|---|
| Product: | [RT] Jetty | Reporter: | Simone Bordet <simone.bordet> |
| Component: | client | Assignee: | Simone Bordet <simone.bordet> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | gregw, jetty-inbox, kowalewski.bartosz, sjavurek |
| Version: | 7.3.1 | ||
| Target Milestone: | 7.2.x | ||
| Hardware: | PC | ||
| OS: | Linux | ||
| Whiteboard: | |||
|
Description
Simone Bordet
Hi, This issue seems to be treated as an improvement and not a bug. I fully agree that the connection per address property needs to configured properly. If the request rate is predictable, Jetty client will work just fine. It should be the one that uses this library that is responsible for configuring the client properly. However, really bad things tend to happen from time to time. If request rate suddenly increases, timeouts stop working the way they should and even out of memory error might be observed. While the current impl of Jetty client supports timeouts, the fact that this library can accept unlimited number of requests without notifying anybody what its state is (in my opinion) a bug. That is why (again, in my opinion), adding this new timeout mechanism is not an enhancement, but a bug fix. However, if the queuing mechanism used in this client library is changed so that when there's no idle connection and no new connection could be started it either: a) throws an exception, so that code that uses Jetty client can detect that Jetty is overloaded or b) Jetty blocks the thread that was used to pass a request to Jetty client code - blocks until a Jetty's HTTP connection is assigned to this given exchange ... then the new mechanism might definitely be treated as an enhancement. Thanks, Bartek Sorry, but calling it a bug rather than a feature request will not get it handled any faster. We are peddling as hard as we can eitherway. But this issue is coming close to the top of the todo pile, so expect something soon. So the solution that we are thinking of implementing is two fold: Firstly we will add a HttpClient.setMaxDestinationQueueSize So that we can control infinite queues. If a send is done that fills the queue then a RejectedExecutionException will be thrown. We will also add a total timeout. Currently we are still debating if this should be a change in the semantics of setTimeout, or a new timeout. I'm leaning towards the former. We hope to have a patch for this in a day or two. Tracking the addition of maxQueueSize in https://bugs.eclipse.org/bugs/show_bug.cgi?id=343567 Hi Greg, It looks like you are making great progress with this issue! I see on 2011-04-18 19:38:32 EDT that you were hoping to have a patch for this issue? I know there was a holiday in between but I was wondering if I could check in on the status? Kindest regards, Susan Fixed. Now the semantic of HttpClient.timeout and HttpExchange.timeout is that of a timeout that encompasses the time between HttpClient.send(HttpExchange) and a final state (either complete, excepted, canceled, etc.), and includes queuing, connection (if needed) and request/response times. That's great Simone! Thank you so much. May I asked which version of Jetty you fixed this in? Was it back-ported to any previous releases? Will it be available in an upcoming release? If so do you have a target date for that? Sorry for so many questions, just trying to plan how we can best get this fix to my customer. All the best, Susan The fix is in current trunk, and will be part of Jetty 7.4.1. I do not know the dates for a 7.4.1 release, but from past release cycles expect it within a couple of weeks or at most end of month. |