akka-http
akka-http copied to clipboard
+htp #1205 Time out requests while they are waiting on the queue
Currently, in a situation of a high idle-timeout
, a large
max-open-requests
queue, and a troubled slowly-responding server, the
queue is going to fill up. Once the server recovers, potentially very
old requests are going to be sent (from much more than idle-timeout
ago).
This commit makes no request wait longer than idle-timeout
on the queue,
since the request WOULD have timed out by then anyways, if a connection
had been available.
Fixes #1205.
Test PASSed.
@akka/akka-http-team tests green, ready for review.
Seems clean enough. I wonder how hard would it be to write a test that tests this. We could increase all other timeouts to make sure that request is timeout out in the queue and not somewhere else.
I decided to re-use the existing timeout mostly for simplicity's sake (understanding client timeouts already is hard enough). We could introduce something like queue-timeout
, and update the timeout docs page. But really, this needs a rewrite as well, describing how max-open-requests
is actually configuring a queue.
Having a different timeout would of course make it easier to write @2m 's suggested test...
I'm working on a test and introduced the separate queueTimeout
.
I think I have a problem though: PoolInterfaceActor
always gets an initial demand of 16 items from the downstream PoolConductor
, irrespective of how many connections are actually available to dispatch requests on. Is there a way to configure that initial demand between the two graph stages?
Test FAILed.
PLS BUILD
Test PASSed.
Thanks for the extra review! I'm happy to address the remaining comments. However, @jrudolph, weren't you doing some re-write of the client pool stages that would make this irrelevant? Or am I totally making that up...
@jrudolph I don't think we expect conflicts here, do we?
I should note that this PR does not (fully) work: Because of the 16-item internal akka streams buffer, there will still be 16 items queued up that we can't purge. And though that buffer theoretically can be lowered to 1, I wasn't able to do that accurately without changing the global buffer size.
Plus, going from "we can't purge requests while on the queue" to "we can purge some requests from the queue" may not help with the predictability of the http client... :)
Test FAILed.
Test FAILed.
Test FAILed.
!!! Couldn't read commit file !!!