Settings for http2 initial stream/connection window sizes

[okhaliavka]

Describe the feature request
Istio should have global setting(s) for initial_stream_window_size and initial_connection_window_size that go into listeners and http2 clusters that pilot generates. Envoy's default of 256MB is often too much. For us, it's causing trouble with sidecars' memory usage, and decreasing these window sizes helps. We would like to decrease it once and for all connections, but have found that it's impossible, since setting http2_protocol_options on a cluster makes envoy think that upstream supports http2. While this is often useful for outbound clusters and works fairly well, this is very problematic for inbound clusters, leaving us no choice but to configure it individually for every http2 workload.

We're not the first ones having problems with http2 flow control defaults:

• https://discuss.istio.io/t/correct-way-to-change-initial-stream-window-size-for-envoy/10061
• Make Envoy's per connection buffer limit configurable #15260
• Envoy connection buffer limits, badly need documentation please! #33416

Describe alternatives you've considered
EnvoyFilter. It works for listeners, but doesn't for clusters for the reasons described above. It could work if EnvoyFilter's ClusterMatch allowed to filter clusters with present http2_protocol_options though.

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

[ramaraochavali]

@akhalyavka are you suggesting to have to global default for all or do you need this configurability at individual service?

[okhaliavka]

@ramaraochavali Global defaults. Configurability at individual service would be nice too, but it's at least possible with EnvoyFilter.

[lambdai]

Still an ongoing issue

[zirain]

seems we still need this? cc @hzxuzhonghu

[ramaraochavali]

Configurability at individual service would be nice too, but it's at least possible with EnvoyFilter.

Same is possible for global also with Envoy filter in root namespace without any workload selector.

[manderson23]

It would be nice if there were simple Istio level options to override these without resorting to EnvoyFilter.

If that isn't possible then having an HTTP2 example with these parameters in the Envoy Filter documentation could help.

[okhaliavka]

Same is possible for global also with Envoy filter in root namespace without any workload selector.

@ramaraochavali Setting http2_protocol_options on inbound clusters tells envoy to use http2 to talk to the app container.
Doing it globally breaks all applications that do not support http2.

[ramaraochavali]

Setting http2_protocol_options on inbound clusters tells envoy to use http2 to talk to the app container.
Doing it globally breaks all applications that do not support http2.

Sorry I meant having a workload selector like http2_enabled : true in root namespace and all http2 services have that label. But I agree, having service specific config in DR would be better.

[kyessenov]

Re-opening since this is a valid request.

CC @briansonnenberg @howardjohn how best to model the config here. Ideally, I'd imagine we want to move away from the 256MB window size by default. That looks great on throughput demos, but adds too much risk to the production deployments.

[keithmattix]

@jaellio is adding a setting for max_concurrent_streams in istio/api#2952 and I think Zack Butcher is adding some connection pool settings in istio/api#2961. Feels like we're wanting a lot of http2 settings in DestinationRule; do we feel like we need a unified approach to these h2 settings?

[kyessenov]

I don't have a strong opinion where to put them. We do need them per-hop (app-client, client-server, server-app) and in both directions.

[keithmattix]

+1 on server->app - I've worked with a customer really looking for that functionality

[ZackButcher]

IMO this kind of config makes sense in the connection pool settings -- then as Kuat says they'll apply everywhere (via DestinationRule) and with istio/api#2961 they'll be configurable for the server directly via Sidecar as well.

That begs the larger question (probably not for this thread), do we want to support other TrafficPolicy style configuration via the Sidecar as well? As I called out in this comment, using the full TrafficPolicy in the Sidecar has some interesting possibilities for configuring app-client and server-app (client-server is well-handled by DestinationRule today)

[howardjohn]

I think we actually implemented TLS in the inbound sidecar api before. it was removed before it launched; I forget why. We should look into that before we go down that path though. Shriram wrote it initially

…

On Thu, Oct 12, 2023 at 3:13 PM Zack Butcher ***@***.***> wrote: IMO this kind of config makes sense in the connection pool settings -- then as Kuat says they'll apply everywhere (via DestinationRule) and with istio/api#2961 <istio/api#2961> they'll be configurable for the server directly too. That begs the larger question (probably not for this thread), do we want to support other TrafficPolicy style configuration via the Sidecar as well? As I called out in this comment <istio/api#2961 (comment)>, using the full TrafficPolicy in the Sidecar has some interesting possibilities for configuring app-client and server-app (client-server is well-handled by DestinationRule today) — Reply to this email directly, view it on GitHub <#34169 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEYGXPTV7FW6RL7LUTMKO3X7BTQBANCNFSM5AVT6QSQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[kyessenov]

My feeling is that we probably want to pick a better default. But we need a config setting to opt-out/opt-in during the transition. So we may not need a long term API, but we still need a knob right now.

[hzxuzhonghu]

I believe the original design is not to include all the configs from envoy, there are tens(maybe hundreds) of them for only protocol-related

[kyessenov]

@hzxuzhonghu Yes that would be the case if the default was good. But it's not - and putting everyone at risk because we don't want to expose a config is not a good strategy.

[hzxuzhonghu]

Fair point

[istio-policy-bot]

🚧 This issue or pull request has been closed due to not having had activity from an Istio team member since 2023-10-16. If you feel this issue or pull request deserves attention, please reopen the issue. Please see this wiki page for more information. Thank you for your contributions.

Created by the issue and PR lifecycle manager.

[manderson23]

Should re-open as it looks like the earlier PRs referenced didn't include the initial stream and connection window sizes