Here is my rough thinking on how to go about this (these are rough thoughts, feel free to propose alternatives once you start investigating). This is going to be a fairly complicated change. I think it would be good to break this up into small pieces just so that we can test things incrementally (that should not be that hard to do).

Changes to codec.h

https://github.com/lyft/envoy/blob/master/include/envoy/http/codec.h

IMO we should try to hide as much of the windowing stuff as possible from higher layers.

On Stream interface, we will need a disableRead() and enableRead() (or variant). On HTTP/1 this will directly interface with the connection by disabling/enabling socket reads. On HTTP/2, this will work with the nghttp2 nghttp2_session_consume API. (As part of this change, we will need to switch to using nghttp2_option_set_no_auto_window_update). Roughly, for http/2, when enabled, we should just consume() data as we get it. When disabled, we keep track of how much data needs to be consumed, and then only consume it when we become enabled. When calling consume(), we need to flush pending frames to release window updates also. From an API perspective at this payer, disable/enable of read is eventually consistent. It might not happen right away and some number of reads might still happen.
On the Stream interface, I would add: setWriteBufferHighWatermark() (or variant). In StreamCallbacks, we will need a callback for when we go above the high watermark as well as back below it.
The default HTTP/2 window sizes need to be configurable. Right now we just generically open up to 256MiB. I don't have a strong opinion in v1 of this change as to whether we make this configurable on a global basis or per listener basis. We should probably keep the connection window large and do flow control at stream level. The default write high buffer watermark could also be set in the same default way.

Changes to connection.h

https://github.com/lyft/envoy/blob/master/include/envoy/network/connection.h

We are going to need a setWriteBufferHighWatermark() here as well as a set of callbacks. This will end up being used in the HTTP/1 case so that we can get notified of writes when we are above the high watermark.

Changes to connection_impl

We need to support real socket disable/enable for the HTTP/1 case. Right now we do a soft disable, mainly because it's much faster to get notified of remote close. If we upgrade to libevent 2.1, we can switch this behavior out for #391 so that we can use EPOLLRDHUP, which would be a lot better (there are other solutions also, can talk about this separately).

Changes to filter.h

https://github.com/lyft/envoy/blob/master/include/envoy/http/filter.h

Filters need to be notified of when the downstream connection goes above/below the high watermark. Additionally, filters will need to be able to enable/disable reading from the downstream stream.

In StreamFilterCallbacks, much like reset, I would add the ability to both get notifications of above/below watermark events, as well as enable/disable read. NOTE: It would be technically more correct if enable/disable read actually propagated through the filter stack with the ability for a filter to pause (consume) this event, and then restart, but I do not think this extra complexity is worthwhile. I would just hit the underlying stream directly.

Other random changes

There will be various other changes required to router and a few other places to plumb through the callbacks and then wire it up back and forth. For this change we will also want end-to-end integration tests.

http/1 and http/2: end to end flow control #150

Description

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions