search

python-hyper / h2

HTTP/2 State-Machine based protocol implementation
https://h2.readthedocs.io/en/stable
MIT License
963 stars 151 forks source link

The connection should emit a WindowUpdated event on INITIAL_WINDOW_SIZE change #1193

Open pgjones opened 5 years ago

pgjones commented 5 years ago

I think the connection should emit a WindowUpdated event on receipt of a settings frame that changes the INITIAL_WINDOW_SIZE. This is because this frame is valid after the receipt of headers and hence it implicitly updates the window (see _flow_control_change_from_settings). Without emitting this frame implementations have to specifically look for this settings change and respond appropriately themselves.

(I'll implement this, just wanted to get views on whether this is the correct thing to do).

Lukasa commented 5 years ago

I think that’s reasonable enough, yeah.

vmagamedov commented 5 years ago

I had the same issue in grpclib. This is not obvious (all the ways window can change) but current implementation sticks more closely to the spec than proposed change. And you can always point to the RFC 7540 Section 6.9.2 in the documentation to explain how flow-control works.

Proposed change requires to generate fake WindowUpdated events for all current streams. This introduces additional (small?) overhead and this is just feels weird for me: WindowUpdate event doesn't mean WINDOW_UPDATE frame anymore.