-
Notifications
You must be signed in to change notification settings - Fork 70
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
Configurable Call Timeout #1040
Comments
The way it was supposed to be used is to split your client up into multiple clients if there is a need to separate configuration within a client: riptide.clients:
example-login:
connections:
connect-timeout: 25 milliseconds
socket-timeout: 25 milliseconds
retry:
enabled: true
max-retries: 2
timeouts:
enabled: true
global: 150 milliseconds
example-download:
connections:
connect-timeout: 25 milliseconds
socket-timeout: 500 milliseconds
retry:
enabled: true
max-retries: 2
timeouts:
enabled: true
global: 30 seconds
That's what Global timeouts allow you to manage your own SLAs (what you promise your own clients) while connect- and socket-timeouts (and to some degree backup-requests) allow you to deal with SLAs of your downstream dependencies. |
That's understood and also what we do.
Please correct me if I am wrong, but setting What I want my application to do is to not wait longer than e.g. 50ms per request for a specific client and if this 50ms "call timeout" budget was eaten up, trigger one of my retries (which then has another 50ms guaranteed "call timeout" budget but not more)... Similar to what you would achieve with changing policy composition from the current default |
Yes, correct.
You can with a backup request. The following will issue a second, concurrent request if the first one took longer than 50 milliseconds. Backup requets are essentially latency-based retries. Sounds like what you want in this case (because your strategy to counter them is retrying). riptide.clients:
example:
connections:
connect-timeout: 25 milliseconds
socket-timeout: 25 milliseconds
backup-request:
enabled: true
delay: 50 milliseconds
timeouts:
enabled: true
global: 150 milliseconds
Correct, that would give you what you want. There is actually no need to remove the outer most Timeout. Both serve a purpose. So you'd like to have this, then? riptide.clients:
example:
connections:
connect-timeout: 25 milliseconds
socket-timeout: 25 milliseconds
retry:
enabled: true
max-retries: 2
timeouts:
enabled: true
call: 50 milliseconds
global: 160 milliseconds # 3 calls, each 50ms + some time to issue a new request This would only really be useful for scenarios where there is no retry desired, i.e. really fail fast. Personally, I don't see a big value in failing because something took to long only to immediately retry it. Your client will have to wait either way. |
Yes, we also just discussed the usage of backup-request, but that is limited to one additional request, right?
Correct, I simplified to make my point clear.
I think the assumption here is: If a call already took longer than expected latency (e.g. p999 + 10% = 25ms) the possibility that it will finish any time soon is rather low. So better stop waiting and issue a new request, reusing the open connection. But you are right, sounds like nice usecase for a backup call. Edit: |
Correct.
Backup requests would run concurrently and give the original request a chance to still finish. The first request to finish will be used. Might as well be the original request.
Are you interested in contributing this? The way I see it, it's essentially just an addition to the auto configuration, because all the building blocks are already there ( |
@whiskeysierra Yes, I believe we have a couple of guys who would like to contribute. |
@jbspeakr I saw you made a commit to your fork. Dare to open a PR? |
I would like to be able to configure timeouts on a per request basis similar to what OkHttp calls
Call Timeout
(link).Detailed Description
My current understanding is, that following request-related timeouts can be configured:
connections.connect-timeout
for starting the TCP connectionconnections.socket-timeout
for data flow/ packetstimeouts.global
wrapping everything (retries, queuing, backup requests, fallbacks, etc)From my perspective, what is missing is a way to define a time limit for a complete HTTP call (only), including resolving DNS, connecting, writing the request body, server processing, as well as reading the response body. OkHttp calls this
Call Timeout
.Context
Currently it appears to me that there are only super low-level or super high-level timeout configuration possibilities, which makes it hard to configure SLO-based timeouts. As upstream services often define latency objectives on a per-request basis, I would also like to configure my request timeouts on a per request basis.
To be more specific: I am neither interested in a particular
connections.connect-timeout
nor a specificconnections.socket-timeout
as long as the entire HTTP request finishes within a given time.Unfortunately the
timeouts.global
is too global on the other hand because it spans multiple request, which means that configuring something like this...... could still result in the global timeout kicking in before any retry happens.
Possible Implementation
Your Environment
The text was updated successfully, but these errors were encountered: