gep: align GEP-1731 retry semantics with implementations - #5167
Conversation
|
/cc @mikemorris @rikatz |
|
/retest |
Signed-off-by: Norwin Schnyder <norwin.schnyder+github@gmail.com>
Signed-off-by: Norwin Schnyder <norwin.schnyder+github@gmail.com>
|
/lgtm @youngnick @robscott for approval, as I am biased on this one |
|
/cc @youngnick @robscott |
| // | ||
| // +optional | ||
| // <gateway:experimental> | ||
| // +listType=set |
There was a problem hiding this comment.
This isn't the actual types, but we need to ensure that <gateway:experimental> stays until the feature graduates to Standard...
There was a problem hiding this comment.
Yes, this only applies to the GEP. The other fields that are also considered experimental don't have the marker too.
Once we have agreed on the changes, I'm going to add them to the API as well.
| // `100ms` will cause a backend request to first be retried approximately | ||
| // 100 milliseconds after timing out or receiving a response code configured | ||
| // to be retriable. | ||
| // The maximum duration a Gateway should wait before a retry attempt is |
There was a problem hiding this comment.
This mandates that implementations must use exponential backoff, right? We didn't want to mandate that before, although it seems like it's almost always a better idea.
There was a problem hiding this comment.
The most common backoff strategies are:
- none
- fixed
- linear
- exponential
(Each of these can be combined with jitter and with a max cap)
For HTTP traffic, IMO the most common choice is capped exponential backoff with full jitter.
HAProxy and NGINX support only none. Traefik and Envoy support capped exponential backoff with jitter.
Before this change, backoff was specified as a lower bound on the retry interval a semantic that none of these data plane technologies can satisfy.
Signed-off-by: Norwin Schnyder <norwin.schnyder+github@gmail.com>
|
New changes are detected. LGTM label has been removed. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: snorwin The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/retest |
What type of PR is this?
/kind gep
What this PR does / why we need it:
GEP-1731 currently specifies
backoffas "the minimum duration a Gateway should wait between retry attempts". None of the current implementations provide this guarantee. Airlock Microgateway, Istio, Envoy Gateway and kgateway all mapbackoffto Envoy'sretry_back_off.base_interval.In addition, this PR clarifies the retry behavior on connection errors. The previous wording ("disconnect, reset, timeout, TCP failure") included conditions where the request may already have been sent and processed.
Which issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?:
AI Disclosure
AI (Claude;Fable 5) was used only for spell-checking and minor rephrasing suggestions.