Implement retry on connection reset to upstream

[Stono]

Describe the feature request
Almost all 503's I see are connection resets.

{"level":"debug","time":"2024-06-25T09:10:35.552211Z","scope":"envoy router","msg":"[Tags: \"ConnectionId\":\"621\",\"StreamId\":\"15076901503968233471\"] upstream reset: reset reason: connection termination, transport failure reason: ","caller":"external/envoy/source/common/router/router.cc:1332","thread":55}

These are connection resets between the Destination sidecar and the local app. I've wrote about them before here connection resets.

Many istio users run up against this, it's a pain to debug, and a pain to resolve, and continually comes back. I believe if the destination sidecar implemented a retry policy on reset to the local app, it'd be a huge improvement for users of Istio out of the box.

The challenge at the moment is the current DestinationRule has a retryOn, but that's describing the connection between the source sidecar and the destination. The resets described in the above scenario do not present at the source sidecar as a reset, they just present as a 503. Arguably you could retryOn 503's but that's a much bigger blast radius than simply retrying on connection reset.

As an added thought, retryOn: reset carries some risk, in that you could in theory retry requests that did make it to the destination service, which is a behaviour you may not want.

envoyproxy/envoy#10007 (unfortunately old and closed) detailed a reset-before-request proposal, eg a safe reset. Implementing this feature, and having it as the default on both source sidecar -> destination sidecar (eg retryOn in DestinationRule), and destination sidecar -> destination app (currently not configurable) I believe would resolve most users 503's.

Describe alternatives you've considered
There are no alternatives, currently users have to spend extremely large amounts of time trying to debug the reason for the 503 at the destination.

Affected product area (please put an X in all that apply)

[ ] Ambient
[ ] Docs
[ ] Dual Stack
[ ] Installation
[x] Networking
[ ] Performance and Scalability
[ ] Extensions and Telemetry
[ ] Security
[ ] Test and Release
[x] User Experience
[ ] Developer Infrastructure

Affected features (please put an X in all that apply)

[ ] Multi Cluster
[ ] Virtual Machine
[ ] Multi Control Plane

Additional context

[ramaraochavali]

These are connection resets between the Destination sidecar and the local app.

Are they happening when pod is shutting down?

[Stono]

No, they are not. This is not related to startup or shutdown scenarios, this is when the workloads are running.

Almost all of the time this is miss-matched keepAlive configuration between envoy and the local app (see https://karlstoney.com/istio-503s-ucs-and-tcp-fun-times/ and envoyproxy/envoy#14981 (comment)). There are other reasons for connection resets too, obviously, but this is the most common one I have observed by far.

It is completely fixable if we expect end users to ensure these align - but fixing it is complex, and completely different depending on language, framework etc of the app. I consider myself an advanced Istio user, and I have ploughed many many hours into debugging 503's. Most users aren't going to do that, they're going to get annoyed any move on.

The proposal here would "fix" it somewhat transparently and many 503 user issues would just go away. I believe it will result in a better out of the box experience. Just search the amount of "istio 503" threads for motivation.

[hzxuzhonghu]

As an added thought, retryOn: reset carries some risk, in that you could in theory retry requests that did make it to the destination service, which is a behaviour you may not want.

Not sure i understand about this risk

[Stono]

so retryOn: reset can retry requests that have actually reached the destination service, but the connection has terminated before the response was received. Retrying with retryOn: reset by default, means that you could potentially retry requests that should not be retired. Think for example; a payment request.

The default behaviour of istio in my opinion should never be to automatically retry requests that have possibly reached the destination. It should only (by default) retry when the request has not been sent. #50506 is an example of it going wrong. IMO https://github.com/istio/istio/blob/master/pilot/pkg/networking/core/route/retry/retry.go#L47 should not have retryable-status-codes in it. That's why I linked envoyproxy/envoy#10007, which is a proposal for reset-before-request.

But that's a separate issue to the original issue I raised - to be completely clear the issue there is that there does not seem to be a retry policy between the destination sidecar, and the local application. Making that retry safer, is better covered under #50506.

[sschepens]

@Stono just curious about what exactly is causing the resets you see.

I agree that we probably need a retry policy for ingress routes, but misaligned idle-timeouts usually produce un-retryable resets because envoy only realizes the connection is closed when writing the headers and at that point request is no longer retryable.

Did you try adding a retry policy to see if those 503 errors went away?

[Stono]

Did you try adding a retry policy to see if those 503 errors went away?

You can't add a retry policy, because the issue is between the local sidecar and the local application on the inbound request (destination, on my diagram). There is no API exposed in Istio for this.

And as detailed on my original post, I wouldn't add a retry policy on 503 to the source sidecar, because then i'll retry unsafe requests, which is why I proposed reset-before-request.

because envoy only realizes the connection is closed when writing the headers and at that point request is no longer retryable.

If that's the case, then what I'm proposing in this issue cannot be achieved safely.

[sschepens]

You can't add a retry policy, because the issue is between the local sidecar and the local application on the inbound request (destination, on my diagram). There is no API exposed in Istio for this.

You can use an EnvoyFilter to patch Routes on ingress listeners

If that's the case, then what I'm proposing in this issue cannot be achieved safely.

You can give it a try with using an EnvoyFilter, but I'm pretty sure that misaligned idle-timeouts issues cannot be retried safely.

We've experienced this issue of misaligned idle-timeouts as well in my company, and still encounter it from time to time. The most troublesome webserver is usually nodejs which defaults to 5s idle timeouts which is way lower than the defaults Istio/Envoy have. I believe that the best way to solve this issues for good is to lower default the default idle timeout in ingress clusters to 5s. This is pretty safe since we're talking of unused connections and since they are local connections, they're pretty cheap to establish.

[Stono]

You can use an EnvoyFilter to patch Routes on ingress listeners

I haven't tried with an EnvoyFilter, no. I see no reason why it wouldn't work.

I believe that the best way to solve this issues for good is to lower default the default idle timeout in ingress clusters to 5s

Seems reasonable too - have you tried that with an EnvoyFilter? I wouldn't say it's mutually exclusive with a sensible/safe retry policy however as we have by default between on outbound clusters.

@ramaraochavali just thinking, the recent change you made in #51929 / #51935 to disable cleanup for inbound clusters, i'm wondering if that might subsequently exacerbate this problem?