retry filter

[szuecs]

This looks great to use, maybe as filter https://github.com/sethvargo/go-retry
Needs to check what if we have non idempotent requests like POST requests, but in general as opt-in feature could be done.

[AlexanderYastrebov]

Seems to be related to #674

[szuecs]

There is also hashicorp, but they will do a blocking read the body:
https://github.com/hashicorp/go-retryablehttp/blob/a53bec1353a135f9548327f10bb8459bfe96e2d8/client.go#L255-L256
for the RoundTrip https://github.com/hashicorp/go-retryablehttp/blob/master/roundtripper.go#L38

[szuecs]

Maybe http.Transport can already do this for us if we add idempotent header:

Transport only retries a request upon encountering a network error if the request is idempotent and either has no body or has its Request.GetBody defined. HTTP requests are considered idempotent if they have HTTP methods GET, HEAD, OPTIONS, or TRACE; or if their Header map contains an "Idempotency-Key" or "X-Idempotency-Key" entry. If the idempotency key value is a zero-length slice, the request is treated as idempotent but the header is not sent on the wire.

Cite is the last section of https://pkg.go.dev/net/http#Transport

[oporkka]

We are running a Node.js application with very uneven complexity of requests, and I have been wondering if a retry-pattern from Skipper as ingress could help us to solve cases where one or few pods are overloaded and would fail fast, but still allow delivering a successful response to the clients.

For example

1. Skipper requests pod "A"
2. Pod "A" fails fast with 503
3. Skipper tries one retry to another pod (next one in rotation)

[szuecs]

@oporkka I expect that you need http body, right?

[oporkka]

@oporkka I expect that you need http body, right?

Yes, I would need this for POST requests, which I know that would be idempotent failures (short-circuit). I wonder if this should be steered by a response header, in order to distinguish from other 503 cases which might not be idempotent.

[szuecs]

@oporkka I think if we implement the decision as filter then I don't mind that it always retries also POST requests. I think in general if POST is to be retried or not should be done by creating the routes (ORing by Method("POST") or not).
If we check the response header then I think the affect will be that it's less interesting, because most incidents that I reviewed were a mix of 503 (for example read/write timeouts while streaming body in both directions and all kind of connect errors), 504 (proxy to backend timeout) first and short time later a flood of 499 (client time out).