Skip to content
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

Setting to override default request timeout #3251

Closed
owenhaynes opened this issue Apr 23, 2024 · 18 comments · Fixed by #4329
Closed

Setting to override default request timeout #3251

owenhaynes opened this issue Apr 23, 2024 · 18 comments · Fixed by #4329
Assignees
Labels
area/api API-related issues kind/enhancement New feature or request
Milestone

Comments

@owenhaynes
Copy link
Contributor

Description:
The default timeout looks to be 10seconds we would like to easily be able to change this globally to 30 seconds. It maybe possible to do this with envoy config patching but this seems overly complicated for a simple change.

@zirain
Copy link
Contributor

zirain commented Apr 23, 2024

there's a lot of timeout in envoy, which one?

@owenhaynes
Copy link
Contributor Author

owenhaynes commented Apr 23, 2024

we currently set the rule in the httproute

timeouts:
            request: "30s" 

But this is for the full request if I understand. Getting a lot of up stream timeout errors with out this being set

So ideally it would be the response timeout we would like to alter globally as we have some services which can take over 10seconds

So I assume it a global route timeout that is required

@guydc guydc added kind/enhancement New feature or request area/api API-related issues and removed triage labels Apr 24, 2024
@guydc
Copy link
Contributor

guydc commented Apr 24, 2024

Hi @owenhaynes. The timeout that you're referring to in the Gateway API HTTP Route is implemented in EG as Envoy's route timeout: https://www.envoyproxy.io/docs/envoy/latest/faq/configuration/timeouts#route-timeouts. I'm not familiar with an equivalent global Envoy setting.

One thing that can be done here to improve UX is to additionally support this timeout value in EG's ClientTrafficPolicy. Then, you can attach a policy to Gateway, which will make this timeout apply to all routes attached to that gateway.

However, since this creates overlap with existing Gateway-API config, perhaps the best approach is to raise this requirement in Gateway-API.

@owenhaynes
Copy link
Contributor Author

Yeah I don' think envoy has a global setting for this as a question got asked in the https://groups.google.com/g/envoy-users/c/Bbp3SNLIw8Y

I like the concept of doing this at a ClientTrafficPolicy but this is not great when merge gateways are used as at a platform level we have no control. So maybe the EnvoyProxy resource is a better place. I mean we could add a Kyverno mutation to fix this but that's asking for trouble 😄.

I do like the idea of it being added to the Gateway-API but that is slow moving

@guydc
Copy link
Contributor

guydc commented Apr 25, 2024

EnvoyPatchPolicy is also an option that you can consider here. It supports GatewayClass attachment for the MergeGateways scenario that you describe: https://gateway.envoyproxy.io/v1.0.1/tasks/extensibility/envoy-patch-policy/.

Copy link

This issue has been automatically marked as stale because it has not had activity in the last 30 days.

@github-actions github-actions bot added the stale label May 25, 2024
@galihputera
Copy link

I got this issue when trying to perform long-running gRPC server stream (>15s). Try to use patch policy but still unlucky

@github-actions github-actions bot removed the stale label Jun 6, 2024
@arkodg
Copy link
Contributor

arkodg commented Jun 6, 2024

looks like this was asked in slack recently as well https://envoyproxy.slack.com/archives/C03E6NHLESV/p1717439123191249
I'd be a +1 to add requestTimeouts to BackendTrafficPolicy so it can be applied globally at the Gateway level

@arkodg arkodg changed the title Setting to override default timeout Setting to override default request timeout Jun 6, 2024
@arkodg
Copy link
Contributor

arkodg commented Jun 6, 2024

@galihputera if you are looking at setting request timeouts at the route level, these examples https://gateway.envoyproxy.io/v1.0.1/tasks/traffic/http-timeouts/ should help

@galihputera
Copy link

Thanks, @arkodg but I'm asking about the timeout by using grpcroute resource https://gateway.envoyproxy.io/v1.0.1/tasks/traffic/grpc-routing/ seems there is no configuration there yet. We want to enable the grpc-web feature

@arkodg
Copy link
Contributor

arkodg commented Jun 7, 2024

I'm proposing adding the requestTimeout field to BackendTrafficPolicy as well
https://gateway.envoyproxy.io/v1.0.1/api/extension_types/#httptimeout, so that

  1. it can be applied to the Gateway and set for multiple routes
  2. works for GRPCRoute, currently there is no way to set this timeout for it, raised a issue upstream Support Request Timeouts for GRPCRoute kubernetes-sigs/gateway-api#3139
    A workaround here is to use HTTPRoute and add a h2 or h2c value to the applicationProtocol field within the Service backendRef

wdyt @envoyproxy/gateway-maintainers

@arkodg arkodg added the kind/decision A record of a decision made by the community. label Jun 7, 2024
@arkodg arkodg added this to the v1.1.0-rc1 milestone Jun 7, 2024
@shawnh2
Copy link
Contributor

shawnh2 commented Jun 11, 2024

+1 for adding requestTimeout in BTP.

Just one small question: if we apply requestTimeout for Gateway and timeouts.requests for the HTTPRoute at the same time, does the former overwrites latter? or the other way around?

@arkodg
Copy link
Contributor

arkodg commented Jun 11, 2024

ah good point, httpRoute.spec.timeouts.request has highest priority and will override the rest

@guydc
Copy link
Contributor

guydc commented Jun 11, 2024

+1 for adding requestTimeout in BTP.

@arkodg arkodg added help wanted Extra attention is needed and removed kind/decision A record of a decision made by the community. labels Jun 12, 2024
@arkodg arkodg modified the milestones: v1.1.0-rc1, Backlog Jul 3, 2024
@sanposhiho
Copy link
Contributor

Picking it up.

/assign

@arkodg arkodg removed the help wanted Extra attention is needed label Aug 16, 2024
@arkodg arkodg modified the milestones: Backlog, v1.2.0-rc1 Aug 16, 2024
Copy link

This issue has been automatically marked as stale because it has not had activity in the last 30 days.

@github-actions github-actions bot added the stale label Sep 15, 2024
@sanposhiho
Copy link
Contributor

sanposhiho commented Sep 15, 2024

(No worries, it's not slipped from my list. It's just that my OSS capacity is overwhelmed on k/k side at the moment...)

@github-actions github-actions bot removed the stale label Sep 15, 2024
@arkodg
Copy link
Contributor

arkodg commented Sep 20, 2024

np @sanposhiho, we have about a month for the v1.2.0 release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api API-related issues kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants