trafficserver icon indicating copy to clipboard operation
trafficserver copied to clipboard

Make HTTP/2 session and stream windows configurable

Open bneradt opened this issue 2 years ago • 8 comments

Issue #8199 outlines the issue. This PR adds a new setting to allow for setting the session window separately from the stream window.

Closes #8199


This takes over for PR #8203 since that now has conflicts.

bneradt avatar Sep 10 '22 23:09 bneradt

Like I said on #8199, this change doesn't make sense to me. Just setting a large number to connection window size is effectively removing flow control for a connection. And even if you can set a reasonable value to connection window size, setting stream window size separately by hand isn't great because you need to estimate the average number of streams on a connection before hand. Let's say connection window size is 256KB. If the average number of streams is 2, stream window size should be 256 / 2 = 128KB. This stream window size is not the best for a connection that has just one stream (one stream can't fully utilize the connection).

My suggestion is:

  • Keep proxy.config.http2.initial_window_size_in as the only setting for H2 window size
  • Set the initial connection window size to proxy.config.http2.initial_window_size_in * proxy.config.http2.max_concurrent_streams_in at the beginning of a connection
  • Send SETTINGS frame with the value of the-connection-window-size / #-of-open-streams when the number of open streams changes (or alternatively, send WINDOW_UPDATE on each stream)

You can set initial connection window size by adjusting initial_window_size_in (for stream) and max_concurrent_streams_in. This should work for you since you'd have a larger connection window size while you keep your initial stream window size as is. The last item is optional but it's to fully utilize a connection by a low number of streams and I think we should do this.

maskit avatar Sep 12 '22 01:09 maskit

Hmm. I'd be open to a solution that dynamically changes the initial window size based on the number of open streams. I would be concerned that fiddling the setting that frequently would mess up the the peer.

One benefit of having two district settings is if there are in fact numerous concurrent streams, it is unlikely that one will suck up all the resources before the other streams can get started.

Without this PR or some sort of dynamically resizing stream window in setting, we do need to update our documentation to tell folks that they should set the connection window size to be number of expected concurrent active streams * the actual desired window size.

I noticed this issue primarily with session resource with H2 to origin. It probably doesn't matter as much for the client side. But on the origin side, you are more vulnerable to greedy streams.

shinrich avatar Sep 12 '22 23:09 shinrich

One benefit of having two district settings is if there are in fact numerous concurrent streams, it is unlikely that one will suck up all the resources before the other streams can get started.

I don't understand why adjusting proxy.config.http2.initial_window_size_in and proxy.config.http2.max_concurrent_streams_in isn't enough, if we use them to calculate the connection window size. You just need to do a math. If we had a setting for connection window size, how those three would be used?

Let's say we have, initial_connection_window_size=100,000, initial_stream_window_size=1,000 and max_concurrent_streams=10, what's your expectation? You never reach the connection window size. If it never reach the connection window size set, what's the point? It's the same as setting it to 1PB. It limits nothing. This is why I don't want the setting for connection window size.

Similarly, initial_connection_window_size=100,000, initial_stream_window_size=10,000 and max_concurrent_streams=100, without dynamic size change, first 10 streams can dominate the connection window and nothing balances the unfair connection window use. This is why I think we need dynamic adjustment mechanism.

If you want to keep initial stream window size small at the beginning and make it bigger in the middle of transfer for some reasons, having a separate setting for client window size may make sense because stream-window-size * max-concurrent-streams would be too small. But I don't see any reasons to do that.

I would be concerned that fiddling the setting that frequently would mess up the the peer.

It might depends on the peer implementation, but I don't think it requires a lot of tasks nor heavy tasks to the peer. If the frequent update is going to be a problem, we can use the average number of streams of the calculation.

And nobody has suggested any other solutions for connection utilization issue on a connection that has only one stream.

maskit avatar Sep 13 '22 01:09 maskit

[approve ci autest]

bneradt avatar Sep 13 '22 15:09 bneradt

Thank you for the ideas @maskit.

I don't understand why adjusting proxy.config.http2.initial_window_size_in and proxy.config.http2.max_concurrent_streams_in isn't enough, if we use them to calculate the connection window size. You just need to do a math. If we had a setting for connection window size, how those three would be used?

To make sure I understand you correctly: you prefer that this patch set the window (session) size to the product of max_concurrent_streams_in by initial_window_size_in (the stream window size)? That is, you would prefer that over adding a separate configuration to set the window size? I could potentially be persuaded to start by just setting the session window size to the product of these two values for now.

I think @SolidWallOfCode pointed out that we might have issues with origin sessions with many streams eating up more bandwidth than we like. I believe his suggestion was to add a configurable multiplier. What are your thoughts on a configurable multiplier?

Let's say we have, initial_connection_window_size=100,000, initial_stream_window_size=1,000 and max_concurrent_streams=10, what's your expectation? You never reach the connection window size. If it never reach the connection window size set, what's the point? It's the same as setting it to 1PB. It limits nothing. This is why I don't want the setting for connection window size.

Similarly, initial_connection_window_size=100,000, initial_stream_window_size=10,000 and max_concurrent_streams=100, without dynamic size change, first 10 streams can dominate the connection window and nothing balances the unfair connection window use. This is why I think we need dynamic adjustment mechanism.

These are good points, but I guess I'm personally not too concerned about this. The knobs should be pretty clear concerning what they mean. We already have a configuration that sets the stream size, so it doesn't seem out of place to me to have the ability to set the session window size. Given your thoughts, though, it would be good to add documentation to highlight how these three configurations can relate to each other. Having the separate session window size configuration is still my preference.

bneradt avatar Sep 13 '22 18:09 bneradt

To make sure I understand you correctly: you prefer that this patch set the window (session) size to the product of max_concurrent_streams_in by initial_window_size_in (the stream window size)? That is, you would prefer that over adding a separate configuration to set the window size? I could potentially be persuaded to start by just setting the session window size to the product of these two values for now.

Yes, that's correct. If there's something we can't do without the setting for connection window size, I'm totally fine with adding it. But I don't see the necessity so far.

What are your thoughts on a configurable multiplier?

It doesn't make much sense to me because you can achieve the same thing by adjusting the two existing settings.

A compromise would be a setting for session window size with support for auto adjustment. By default it's set to -1 and the size will be automatically calculated based on initial stream window size and max concurrent streams, but you can set a specific size if you need for some reasons.

But it still solves only half of my concern. How do you fully utilize a connection (or its connection window) by unknown number of streams? I'm asking this question again and again, but nobody has told me any solution. It hasn't been an issue because the both window sizes are the same. One stream could occupy the entire connection window. Now, with the new setting, session window size is bigger than stream window size. A connection that has just one stream would be unreasonably limited. You don't care about it?

maskit avatar Sep 14 '22 01:09 maskit

But it still solves only half of my concern. How do you fully utilize a connection (or its connection window) by unknown number of streams? I'm asking this question again and again, but nobody has told me any solution. It hasn't been an issue because the both window sizes are the same. One stream could occupy the entire connection window. Now, with the new setting, session window size is bigger than stream window size. A connection that has just one stream would be unreasonably limited. You don't care about it?

This is an important concern, and I appreciate you raising it again. But I don't think that has been ignored. Maybe it feels like it has been ignored because we haven't explicitly addressed it recently, but it has been discussed at some length in #8199. As I understand the discussion there, the conclusion was that the agreed upon best behavior would be to have ATS tune the stream window sizes as the number of streams either increases or decreases. This way the streams can utilize an appropriate amount of the connection window without starving other streams. That is, one stream by itself in a session can use the whole session window, while twenty streams sharing a session can use about 1/20 of the session window each. But given the complexity of implementing that dynamic behavior, I believe our conclusion was to move forward with this separate explicit and static session and stream window configuration.

To be clear, without some change, HTTP/2 to origin will be useless because Susan observed that as things currently stand, a few greedy streams can starve the other sibling streams.

I'd be content with either of these solutions:

  • For now, simply set the session window size to the product of the stream window size and the max streams size, or
  • for now, add the separate static session window size configuration (i.e., what is implemented in this patch).

It seems to me that long-term we all agree that we want a session window size configuration. Am I correct in understanding that? That is, if the stream window size will be dynamic, then users will generally prefer to set the session window size to some value and leave the stream window size to be dynamic based upon the number of streams. In which case the stream window size will probably support a -1 value (as you suggested above for the session window size) which means it will be dynamic. If that's the future, then we might as well add a session window size configuration now. If I'm wrong about this however, and we're not sure we want a session window, then we might as well choose the first option (have no session window configuration and just have it be the produce of the stream window size and the max streams configuration).

Thank you for continuing this discussion, @maskit. I want to end up committing what we are all content with.

bneradt avatar Sep 16 '22 20:09 bneradt

But given the complexity of implementing that dynamic behavior, I believe our conclusion was to move forward with this separate explicit and static session and stream window configuration.

This is why I feel like the issue is not fully understood. I don't think we should go that way. I was -0 because I was tired.

For now, simply set the session window size to the product of the stream window size and the max streams size, or

H2 to Origin would work since session window size would be larger (although this PR is one for incoming connection), but the session window utilization issue would remain until somebody implements dynamic stream window size based on the number of streams. Increasing stream window size means increasing session window size, so the window utilization issue can't be resolved with any setting values. If we do this, I'd say dynamic stream window size is essential and we need to have it first to avoid the issue.

for now, add the separate static session window size configuration (i.e., what is implemented in this patch).

Like I explained with examples above, this allows users to set window sizes to values that don't make sense. Setting a reasonably big value to session window size make sense in general and people would do that, but without dynamic stream window size, such setting actually causes the window utilization issue and make things worse if a client don't use many streams. I don't think we should add such a trap.

It seems to me that long-term we all agree that we want a session window size configuration. Am I correct in understanding that?

Not quite, there isn't huge difference in this context though. I'd say we all agree that we want to make session window size configurable. I can live with the setting for session window size but I don't want to have a value for session window size on my config. That's why I suggested to support -1 (auto) as a compromise if we really need to add it.

That is, if the stream window size will be dynamic, then users will generally prefer to set the session window size to some value and leave the stream window size to be dynamic based upon the number of streams. In which case the stream window size will probably support a -1 value (as you suggested above for the session window size) which means it will be dynamic.

Hmm, we all need to be more specific if we talk about this. Note that the existing setting is INITIAL stream window size. A server may not want to give much window size to a stream until it validates a request on the stream is valid, although ATS doesn't increase the size at the moment. If we have initial_stream_window_size and also max_stream_window_size, and if you set session_window_size to a value > 0, then setting max_stream_window_size to -1 makes sense. Either max_stream_window_size or session_window_size can be -1, but not the both at the same time.

And in that sense, INITIAL session window size, which you proposed, doesn't make sense because nothing updates it.

I'm wondering if we can wait for dynamic stream window size, if we agree it's best behavior. I don't think it's too complicated. I can't work on it right now, but it seems like there are still a couple of things to do to merge H2 to Origin. Can you tell me your concerns on dynamic stream window size?

maskit avatar Sep 20 '22 07:09 maskit

The http2_flow_control autest will fail until I get a new release of Proxy Verifier created and merged into master/10-Dev. The new Proxy Verifier adds WINDOW_UPDATE and SETTINGS frame logging that the test depends upon.


Update

Our Proxy Verifier version was updated via: https://github.com/apache/trafficserver/pull/9110. The AuTests should now have what they need to work.

bneradt avatar Sep 27 '22 20:09 bneradt