http2: Potential improvements to behavior of buffer high watermark when proxing over H2

[antoniovicente]

I'm wondering if we should invest some effort to tighten the bounds on max size of H2 connection output buffers. I think that the current H2 codec implementation eagerly moves data frames from the per-stream buffers to the output buffer as long as connection window is available, even in cases where the output buffer is already over the configured high watermark.

In the nghttp2 API documentation, I found mentions that nghttp2_send_data_callback can return NGHTTP2_ERR_WOULDBLOCK to stop data frame generation in cases where the generated frames wouldn't fit in the output buffer. It would be good to explore how this API works and how use of NGHTTP2_ERR_WOULDBLOCK affects the relative ordering of generated control and data frames when asked to resume.

Possible behavior strawman:

• Prioritize control frames over data frames when serializing
• Serializing control frames when output buffer is above high-watermark is acceptable.
• Avoid generating data frames when above high watermark
• Continue requiring last data frame serialized before trailers
• Trigger sendPendingFrames when transitioning below low-watermark

Also, let's reach out to nghttp2 if there are some missing APIs that prevent us from getting the behavior we want.

[mattklein123]

cc @tatsuhiro-t who I'm sure would be happy to chat APIs if there is something missing that we would like.

[tatsuhiro-t]

If I understand it correctly, the H2 connection output buffer is envoy specific output buffer which contains the bytes produced by nghttp2_session_send or nghttp2_session_mem_send.

The use of NGHTTP2_ERR_WOULDBLOCK (and its variant NGHTTP2_ERR_PAUSE which has slightly different semantics) is good use case here. If buffer is full, return the error code. And don't call nghttp2_session_mem_send as long as output buffer is full. Because whole output of nghttp2 is stopped, there is no ordering issue here (unless you don't like the current behaviour of nghttp2 frame ordering).

[antoniovicente]

/assign @antoniovicente

I started looking into this one. I expect testing to be one of the big challenges during implementation.