-
Notifications
You must be signed in to change notification settings - Fork 620
GEP-1742: HTTPRoute Timeouts API #1997
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
Changes from 2 commits
accce11
0525880
7696daa
09ca210
18b6894
2799811
90606a8
f07a259
ad0e9c7
db6688b
224d342
7a01f3e
29cc9f0
26e451a
7b6315b
bef97ed
7e20c3d
650874d
659d478
3c0e8e6
6c01fa8
4e7c538
2a1c178
f9181ec
12c731a
ab8283d
bad35be
f0ac5ac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -301,8 +301,99 @@ Could not find any HTTP specific timeouts. PRs welcomed. 😊 | |
|
|
||
| ## API | ||
|
|
||
| TBD. | ||
| The above diagrams show that there are many different kinds of configurable timeouts | ||
| supported by Gateway implementations: connect, idle, request, upstream, downstream. | ||
| Although there may be opportunity for the specification of a common API for more of | ||
| them in the future, this GEP will focus on the L7 timeouts in HTTPRoutes that are | ||
| most valuable to clients. | ||
|
Comment on lines
+306
to
+307
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think one of the most important questions here is where the timeout config belongs. This proposes HTTPRouteRule, but I think that both HTTPBackendRef and/or a new policy to extend Service would be compelling alternatives. It would be helpful if this GEP described why HTTPRouteRule is proposed and why the others alternatives were not chosen.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added some text in the Alternatives section. |
||
|
|
||
| From the above analysis, it appears that most implementations are capable of supporting | ||
| the configuration of simple downstream request timeouts on HTTPRoute rules. This is a | ||
| relatively small addition (one new field) that would benefit many users. | ||
|
|
||
| Because request timeouts are very much related to retry attempts, we also propose | ||
| addressing issue: [#1731](https://github.com/kubernetes-sigs/gateway-api/issues/1731) | ||
| by adding another field for retry conifguration in an HTTPRoute rule. This would also | ||
| benefit many users and seems to be implementable by many (most?) Gateway implementations. | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you clarify if the client request covers the entire connection from client to backend, or just the downstream request from client to gateway? There is also at least one tunnel timeout (HAProxy) and perhaps some other timeouts that cover the entire connection, not just the downstream and upstream halves of the connection.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a diagram in the "Timeout values" section that shows the relationships, but you need to look at the rendered markdown to see it. https://github.com/kubernetes-sigs/gateway-api/blob/7e20c3d25368b2c91d0952dfbd842f46e3324b36/geps/gep-1742.md#timeout-values
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did see the diagram, yet still wanted to get the explanation in words and in the godoc of the implementation section for I don't see any timeout that would cover the entire transaction, unless |
||
| ### GO | ||
|
|
||
| ```go | ||
| type HTTPRouteRule struct { | ||
| // Timeout for HTTP requests, disabled by default. | ||
| Timeout *time.Duration `json:"timeout,omitempty"` | ||
|
||
| // Retry policy for HTTP requests. | ||
| Retries *HTTPRetry `json:"retries,omitempty"` | ||
| // ... | ||
| } | ||
|
|
||
| type HTTPRetry struct { | ||
| // Number of retries to be allowed for a given request. | ||
| Attempts int32 `json:"attempts,omitempty"` | ||
|
||
| // Timeout per attempt for a given request, including the initial call and any retries. | ||
| // Default is the value of the HTTPRouteRule request Timeout. | ||
| PerTryTimeout *time.Duration `json:"perTryTimeout,omitempty"` | ||
| // Specifies one or more conditions under which retry takes place. | ||
| RetryOn []string `json:"retryOn,omitempty"` | ||
| // Flag to specify whether the retries should retry to other localities. | ||
|
||
| RetryRemoteLocalities *bool `json:"retryRemoteLocalities,omitempty"` | ||
| } | ||
| ``` | ||
|
|
||
| ### YAML | ||
|
|
||
| ```yaml | ||
| apiVersion: gateway.networking.k8s.io/v1beta1 | ||
| kind: HTTPRoute | ||
| metadata: | ||
| name: timeout-example | ||
| spec: | ||
| ... | ||
| rules: | ||
| - backendRefs: | ||
| - name: some-service | ||
| port: 8080 | ||
| timeout: 10s | ||
| retries: | ||
| attempts: 3 | ||
| perTryTimeout: 2s | ||
| retryOn: [ "503" ] | ||
| ``` | ||
|
|
||
| ### Timeout values | ||
|
||
|
|
||
| Timeout and PerTryTimeout values are formatted as 1h/1m/1s/1ms and MUST BE >= 1ms. | ||
|
||
|
|
||
| ### Retry attempts | ||
|
|
||
| By default, the number of retry attempts is implementation dependent. An implementation MAY | ||
| provide external mechanisims for controlling the default retry behavior. | ||
|
|
||
| When a request Timeout in the HTTPRouteRule or PerTryTimeout value is configured, the actual | ||
| number of retries attempted may be reduced depending on the specified request Timeout and | ||
| PerTryTimeout values. | ||
|
|
||
| An implementation MUST ensure that the number of retries never exceeds the Attempts value, | ||
| if specified. | ||
|
|
||
| The interval between retries is implementation dependent but SHOULD be a minimum of 25ms. | ||
|
|
||
| ### RetryOn Conditions | ||
|
|
||
| The value of an individual RetryOn condition can be either an HTTP status codes (e.g., 503) | ||
| or some other identifier known to the implementation. | ||
|
|
||
| An implementation MUST support any HTTP status code value for a RetryOn condition. | ||
|
|
||
| The default conditions that will trigger retries, if no RetryOn conditions are specified, | ||
| is implementation dependent. | ||
|
|
||
| ### RetryRemoteLocalities | ||
|
|
||
| When an implementation supports prioritizing endpoints based on their locations, this field | ||
| can be used to control whether or not retry attempts should only select endpoints with the | ||
| same priority (RetryRemoteLocalities=false) or any healthy endpoint, regardless of location | ||
| (RetryRemoteLocalities=true). | ||
|
|
||
| ## Alternatives | ||
|
|
||
|
|
||
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.
When I first read this I wondered why it only focused on HTTPRoute, but later I realized the original GEP specified that focus. It would be a good idea to change the title to "HTTPRoute Timeouts API", or else mention in Non-Goals that other route types are not covered.
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.
Title changed