AsyncHttpClient / async-http-client

Asynchronous Http and WebSocket Client library for Java
Other
6.28k stars 1.59k forks source link

Requests might not time out after configured duration #276

Closed slandelle closed 10 years ago

slandelle commented 11 years ago

AHC throw an Exception saying "No response received after 60000". Yet, the requests randomly time out after 120 sec.

This looks like a bug with the FutureReaper that wasn't able to indeed rip the first time it woke up.

Observed with the Netty Provider.

kamstrup commented 11 years ago

I see this as well (also Netty). Timeouts work fine when set < 60s. Anything above 60s is timed out at 60s.

Does anyone know if any of the other backends work properly in this regard?

slandelle commented 11 years ago

Beware that there's 2 different timeouts: connection and request.

kamstrup commented 11 years ago

Right. I just made sure I set both (to 2 mins). But after 1 min I get a timeout:

java.util.concurrent.ExecutionException: java.util.concurrent.TimeoutException: No response received after 120000 at com.ning.http.client.providers.netty.NettyResponseFuture.abort(NettyResponseFuture.java:327) ~[bet-frontend.jar:na] at com.ning.http.client.providers.netty.NettyAsyncHttpProvider.abort(NettyAsyncHttpProvider.java:1399) ~[bet-frontend.jar:na] at com.ning.http.client.providers.netty.NettyAsyncHttpProvider.access$800(NettyAsyncHttpProvider.java:142) ~[bet-frontend.jar:na] at com.ning.http.client.providers.netty.NettyAsyncHttpProvider$ReaperFuture.run(NettyAsyncHttpProvider.java:1882) ~[bet-frontend.jar:na] at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471) ~[na:1.7.0_21] at java.util.concurrent.FutureTask$Sync.innerRunAndReset(FutureTask.java:351) ~[na:1.7.0_21] at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:178) ~[na:1.7.0_21] at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:178) ~[na:1.7.0_21] at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:293) ~[na:1.7.0_21] at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145) [na:1.7.0_21] at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615) [na:1.7.0_21] at java.lang.Thread.run(Thread.java:722) [na:1.7.0_21]

Note that it says 120000, but the elapsed wallclock time is actually 60000ms.

slandelle commented 11 years ago

Regarding the wrong message, you run into #281 There's several issues on this ReaperFuture...

slandelle commented 11 years ago

Sorry, there's also a third timeout: idle. I think you hit this one.

slandelle commented 11 years ago

I've pushed a fix for #281 on 1.7.X branch. Could you try it out, please?

slandelle commented 11 years ago

The ReaperFuture does 2 things:

It's scheduled periodically with period equals to the request time out. With such a design:

IMHO, we could use Netty's HashedWheelTimer to spawn tons of scheduled once tasks without having to worry about canceling them. @jfarcand WDYT?

jfarcand commented 11 years ago

@slandelle we can try that, but at the time I implemented it I wasn't getting good result, hence this is why I came with the reaper idea. But worth a new try.

slandelle commented 11 years ago

I will give it a try, then. How bad were the results back then?

slandelle commented 10 years ago

implemented in master by 4d029594c47405b94729f246168e30eb17b1feaf

slandelle commented 10 years ago

backported fix in dece38920dc44316946e5073e827973c66e45f7a