Envoy: BUG : MAX 100 HTTP2 streams limit between two Envoy Proxy

[prune998]

description

When doing HTTP2 Streams (ex using gRPC), there is a limit of 100 Streams per TCP connexions between two Envoy proxy.

This situation shows up, for example, when you have 2 pods with Istio Sidecar (which is using Envoy) and talking to each other using gRPC.

Envoy creates ONE TCP connexion to the upstream server per CPU. In my tests with 2 CPUs --> 2 TCP connexions
Then Envoy will multiplex HTTP/2 gRPC calls into the two TCP cnx.
Here are the scenarios for N clients on servers with 1 CPU (so only one TCP cnx between Envoys) :

one Envoy

N client --> 1TCP/1Stream * N --> Envoy --> 1TCP/N Stream --> server

Everything working as expected, can go up to 1024 Streams as per Envoy's default. If I try more, I do get some 503's, which is the expected behaviour

two Envoy

N client --> 1TCP/1Stream * N --> Envoy --> 1TCP/N Stream --> Envoy --> 1TCP/N Stream --> server

When N is > 100, Envoy start queuing the other Streams. As these Streams are long live, they never end, blocking the communication.
What happen is :
N client --> 1TCP/1Stream * N --> Envoy --> 1TCP/100 Stream --> Envoy Sidecar --> 1TCP/100 Stream --> server

Which means if N > 100, I lose connexions.

I can't find why the limit is 100, but I can reproduce this situation every time.
If I have 2 CPUs, I see 2 TCP cnx and 100 streams in each...

I opened an issue in Istio's repo : https://github.com/istio/istio/issues/4940

I also first commented to issue https://github.com/envoyproxy/envoy/issues/2941 which in fact is not related.

[ldemailly]

Subscribe

[mattklein123]

The issue here has to do with how nghttp2 works:
1) nghttp2 defaults to 100 concurrent streams for client connections.
2) When it receives the server side settings frame for max concurrent streams, it will overwrite this value.

Theoretically, this prevents the client from spinning up way too many streams before the server side settings frame comes in.

However, on the Envoy side, we don't send a concurrent streams setting frame in all cases if we think it's the default: https://github.com/envoyproxy/envoy/blob/master/source/common/http/http2/codec_impl.cc#L607

This keeps the client side at 100.

There are a few ways of fixing this:
1) As a quick workaround, just have Envoy sidecar set some value for max concurrent streams on the listener that is not the default value: https://github.com/envoyproxy/data-plane-api/blob/master/envoy/api/v2/core/protocol.proto#L58. As long as it's some value that is below "infinite" Envoy will send the setting frame.

2) I think we should consider just removing the if check and always sending the max concurrent streams setting frame even if it is the default.

3) We could consider using https://nghttp2.org/documentation/nghttp2_option_set_peer_max_concurrent_streams.html on the client side to just default to the h2 default of basically infinite.

Or some combination where we set the default higher to like 1024 vs. 100 and also do (2).

cc @alyssawilk and @PiotrSikora (presumably Piotr will fix this if you want a code change).

[prune998]

Thanks @mattklein123

for the 1), I see no way to change the value for max concurrent streams from the Listener API.

From my point of view, the max concurrent streams of the Listener, which is a "server" from the client's point of view, should either or a mix of :

• be configured dynamically (maybe a big change in the v1/v2 API ?)
• have a default to infinite (max int size, kind of 2^19 or something like that) instead of 100 or 1024
• always send the limit as the first SETTINGS frame when a client connect, whatever the value is
• maybe forward the upstream limit value to the downstream client, while this may not be possible in this situation as I think this SETTINGS must be the first in the communication, not sure it can be sent later on.

On the later, the "right" behaviour in a case where the upstream support a lesser number of streams, should be to open more TCP connexions in parallel instead of cueing.
Thinking of that, when Envoy starts queuing, I see no informations in the logs or stats about that... that could be a great addition to it.

I'm sorry I don't know C++ to be able to help much...

[alyssawilk]

This is due to the current one-connection-per-upstream-per-worker, yes? From our side I'm fairly sure we're going to need to fix this by allowing multiple connections to a given upstream. We have cases where we have many thousands of requests between a given server-upstream pair, and at some point trying to shove that down one TCP connection becomes a problem as head of line blocking is terrible and one RTO can hose your connection.

[prune998]

thanks for the info @alyssawilk
What I understand here that the solution for now is to increase the --concurrency parameter when starting Envoy to increase the connexion pool, have more TCP connexions in parallel with fewer streams in each.

Can you share your experience on the right number of streams per cnx ? gRPC implementation (in Go) is limited to 100, HTTP2 (in GO) is 250, now ngHttp2 is limited to 100. I think some other people using gRPC scaled it quite high (thinking of Cockroachdb)... I experienced using 10k streams per cnx with success on local network during my perf test, the one that showed up the limitation when using Envoy (but I did not performed longevity testing yet).

[mattklein123]

This is due to the current one-connection-per-upstream-per-worker, yes?

@alyssawilk no that's a different issue. This issue is due to how nghttp2 sets up concurrent stream settings per my description. It's only an issue in the envoy <-> envoy case because of how the settings are exposed.

for the 1), I see no way to change the value for max concurrent streams from the Listener API.

HTTP/2 settings an be customized here: https://github.com/envoyproxy/data-plane-api/blob/master/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto#L125. As I already explained, if you set this value to something other than the "infinite" default, server Envoy will send a settings frame and override the 100 default on the client Envoy.

[alyssawilk]

Ah, sorry reread and yeah, I'd thought the default in H2 was 100 but after quick pass on the spec I guess it's unlimited. I'd lean towards keeping (and sending) the limit of 100 streams as I think it's a reasonable default, at least until Envoy has downstream circuit breakers, but no strong feelings either way.

[mattklein123]

@alyssawilk currently the configured default in Envoy is unlimited, but we don't actually send the settings frame in this case. I think we should probably send the settings frame in all cases, no matter the listener setting is.

[prune998]

you're right @mattklein123. As of now, in the situation where Envoy talkes to another Envoy, the 100 streams/one TCP cnx per CPU is a very low limitation.

If Envoy set the default limit on the server as infinite, it HAVE to send it back to the client.
In fact, re-reading the RFC, the client should NOT have a limit :

Initially, there is no limit to this value.  It is recommended that this value be no smaller than 100, so as to not unnecessarily limit parallelism.

In the case the client have a limit and is called by the server, it should send the value back to the server ( this case should never happen with Envoy)

[mattklein123]

@prune998 I understand the standard. We can make a code change to always send the settings frame. In the meantime, I have given you a config workaround which will work with no code changes.

[prune998]

I still have to understand how to port your suggestion on Istio @mattklein123 :)

[costinm]

@prune998 - we're tracking this on Istio side, trying to have it fixed for 0.8.

[gumpt]

My reading of this is that in the original diagnosis of the issue, numbers (2) and (3) have yet to be done.

If I'm correct, I'd like to do both. (2) seems straightforward, (3) seems fixable by setting up separate Http2Options for clients and servers.

[mattklein123]

@gumpt correct. Have at it. Thanks!

[PiotrSikora]

FWIW, I'm not convinced that the diagnosis is correct, but I didn't dig into it yet.

Please CC me on the PRs.

[gumpt]

@mattklein123 Reassign to @PiotrSikora?

[mattklein123]

Reassigned. @PiotrSikora, when you merge in the upstream change, it would be nice to also include the regression test that @gumpt wrote in his closed PR. Thank you!

[PiotrSikora]

Since there is no new nghttp2 release yet, and 1.7.0 is around the corner, should we consider pulling master with the fix? cc @alyssawilk

[mattklein123]

@PiotrSikora yeah let's do it. Can you just put a comment next the SHA that mentions data and why we aren't running a release?

[prune998]

@alyssawilk this issue was created before 1.7.0 release, and seems to still be present in 1.7.0.
When do you expect a fix/a new release ?
Who could help here ?

Thanks

[alyssawilk]

Yep, we moved it to the 1.8.0 milestone when it was clear it wasn't going to be fixed in 1.7.0. Piotr is currently assigned to it so I'll give him a chance to reply to your ping - if he doesn't respond by Monday I'll get ahold of him via chat.

[prune998]

In fact, it sounds nothing have changed in 1.7.0.
I would highly recommend to follow the RFC and :

• forward the negociated limit when applicable
• set the limit to unlimited instead of 100

I can't understand why it's an issue for Envoy/nghttp...

[prune998]

Thanks @alyssawilk

[alyssawilk]

Argh, he's at a conference this week so may be slow to respond. I'd say we don't have an ETA on a fix at this point, but if it's urgent perhaps you can pick up #3658 and see if you can work out tests?

[PiotrSikora]

@prune998 yeah, it got dropped from 1.7.0. See my comment in #3658 on the current state.