-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
connect: Add local_request_timeout_ms to allow configuring the Envoy request timeout on local_app #9554
Conversation
ec07d5a
to
68934c3
Compare
The specific changes in this PR make sense and you even managed to do the extra bits of adding tests and documentation updates. 🎉 Could you do one last thing on this branch before I merge it? Please create a
Then our automated changelog generation code will make sure it's added to the right places during release time. |
68934c3
to
9530c69
Compare
@chrisboulton is attempting to deploy a commit to the HashiCorp Team on Vercel. A member of the Team first needs to authorize it. |
This pull request is being automatically deployed with Vercel (learn more). consul-ui-staging – ./ui🔍 Inspect: https://vercel.com/hashicorp/consul-ui-staging/etnjiqkb5 [Deployment for 68aee94 canceled] |
9530c69
to
68aee94
Compare
@rboyer Yup, have just pushed a change log entry up, and also rebased. Super appreciate you taking a look at this! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
🍒 If backport labels were added before merging, cherry-picking will start automatically. To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/316661. |
Would this change be added to 1.9.x? |
Hi @fredwangwang since this is a new feature and not a bug/stability/security fix, it won't be backported to 1.9.x. It will be available in the Consul 1.10. |
Hi @chrisboulton
Tried with specific timeout as well - local_request_timeout_ms = 360000
I am new to this so still trying to understand how the configuration works. Would really appreciate any insight/suggestion on this. |
Hey @arushi315 -- I haven't pulled down the Consul 1.0 beta to mess with this myself yet, but keep in mind there are two places that can be timing out. Based on my understanding of your setup, these would be:
This PR changes the I think you've found that you also need to change the timeout on the egress Envoy - the only way (per my comment here: #6382 (comment) - "Upstream Request Timeouts") to do that today is with a Looking at what you pasted, I also think you pasted the wrong block from the ingress gateway -- that one is specifically a route for handling the
|
Thank you @chrisboulton for the quick response and explaining this. |
Hi @chrisboulton,
OR Configurable via proxy config on a service: (snip from Nomad job)
As of now I have this configuration:
and service-router:
Checking the envoy configuration, I do not see "timeout"
Anything you see that I am missing here? |
@chrisboulton I finally got it working. My teammate corrected my configuration :)
Thank you again @chrisboulton for helping. |
Using:
I'm sorry; itis a bit confusing for me how this should work. What did you change in your example when you say you got it working, @arushi315 ? I have: client -> traefik proxy -> consul ingress gateway -> api-service (soap) When I made a long-running request, it timed out after 15s ("upstream request timeout") Consul
Proxy defaults{
"Kind": "proxy-defaults",
"Name": "global",
"Config": {
"local_request_timeout_ms": 600000
}
} Backend service-router{
"Kind": "service-router",
"Name": "my-service",
"Routes": [
{
"Match": {
"HTTP": {}
},
"Destination": {
"RequestTimeout": "10m0s"
}
}
],
} |
Hi @kds-rune Using Consul 1.10.0 beta version and setting the "RequestTimeout" did the trick for me and I didn't need to add anything in the proxy-defaults. |
Per #6382, there are two HTTP request timeouts that are configurable for meshed traffic: the egress Envoy, and the ingress Envoy on
local_app
. The request timeout in Envoy defaults to 15s. Today, using aservice-router
configuration entry, you can change the Egress envoy timeout, but it's not possible without using an escape hatch override to tweak the timeout forlocal_app
. This change makes that possible in the same way that you can changelocal_connect_timeout_ms
: via the proxy configuration or a proxy default.The exact timeout we're interested in tweaking in Envoy is the route action timeout: https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#envoy-v3-api-field-config-route-v3-routeaction-timeout
By default, today's behaviour remains the same: the Envoy route timeout is not configured, which means Consul Connect/Envoy will use the default in Envoy: 15s. I could go either way on this: always explicitly configuring the timeout (same way as
local_connect_timeout_ms
works, or just leaving out of the configuration unless the user explicitly configures it.If you explicitly set
local_request_timeout_ms = 0
, it will disable the timeout in local_app completely (to allow for streaming connections with an infinite lifetime). Other values will set the appropriate timeout.The request timeout only applies to HTTP based listeners.
Configured via proxy-defaults:
Configurable via proxy config on a service: (snip from Nomad job)
Resulting route configuration:
Most of what I'm doing with Connect code in Consul at the moment is based on exploratory code spelunking - I'd welcome suggestions if there's a better to this.
Alternatives and Other Things
In my comment on #6382 (comment), I discussed the other request timeout, and its configurability only existing with the explicit creation of a
service-router
- I think this is a bit of a usability issue, it'd be great with some kind of default to be able to globally change this timeout in one hit. There's also some commentary I left regarding enabling therespect_expected_rq_timeout
flag, which allows timeout headers to be propagated between Envoys.