Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WindowUpdates and Flow Control and Large Frames #89

Closed
Kriechi opened this issue Jan 10, 2016 · 2 comments
Closed

WindowUpdates and Flow Control and Large Frames #89

Kriechi opened this issue Jan 10, 2016 · 2 comments

Comments

@Kriechi
Copy link
Member

Kriechi commented Jan 10, 2016

Hi,

are there any plans to include flow control in hyper-h2?
As of now, the user has to keep track of the inbound_flow_control_window. Would it be possible to just generate a WindowUpdate for each incoming frame with the same increment, to keep the window always at the same size? Or are there any other viable solutions to include such functionality?

I guess dealing with outbound_flow_control_window is a bit trickier because hyper-h2 does not actually control the socket, but we could include window_updates in data_to_send interleaved with regular data frames? Splitting of too large data blogs into smaller frames (chunking) would also be a nice things to be included.

I'm happy to send PRs if we find a good approach!

@Lukasa
Copy link
Member

Lukasa commented Jan 10, 2016

@Kriechi So flow control is one of the tricky issues.

Right now, hyper-h2 simply punts on the problem altogether. Right now in the Twisted HTTP/2 server I am writing, the flow control logic is handled in the Twisted application code. Right now it uses the same naive approach you've just talked about, but longer term I'd like it to consider more data when making flow control decisions.

For example, when thinking about flow control you have to balance a few different concerns: how much data has been received on the connection vs the window, how much data you're expecting to receive, and how much you can handle. Some of this information can be known (e.g. from Content-Length headers), but how much can be safely handled is a question that is best answered by the application developers.

For this reason, I'm disinclined to have hyper-h2 take complete control of this. I'd be much more interested in the idea of factoring the logic out to a library, like I did with priority, and then having a recommended recipe or integration for flow control management. I'm open to suggestions for more intelligent approaches to flow control that such a library could use, and if we decide that building such a thing is sensible I'm happy to have it as part of the Hyper project's suite of projects.

Do note that the user doesn't have to keep track of this themselves: hyper-h2's H2Connection class has the remote_flow_control_window method that provides some insight into the flow control state of a given stream. However, that may not be sufficiently detailed for a user to want to use, I totally agree.

As for chunking up data blocks, I'm also a little disinclined to do that in hyper-h2, though my aversion to this is smaller. My primary concern is that hyper-h2 does not (and should not) know anything about how HTTP/2 prioritisation should work, and that should really function on a frame-by-frame basis. If hyper-h2 transparently allows oversize frames to be split, that makes it a bit easier for a user to mispriorize data. Admittedly that's a pretty minor change, so I'd be open to a change that has send_data split a frame up according to max_outbound_frame_size.

Does all of that make sense?

@Kriechi
Copy link
Member Author

Kriechi commented Jan 11, 2016

Makes perfect sense, thanks for the explanation.
I guess my specific use case just does not require such sophisticated approaches.
As said, a very simple default strategy would be nice to be included in hyper-h2.

@Kriechi Kriechi closed this as completed Jan 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants