On Wed, Jun 08, 2022 at 08:28:18AM +0200, Willy Tarreau wrote:
> On Tue, Jun 07, 2022 at 05:24:09PM -0400, Glenn Strauss wrote:
> > Yes, having the server merge the sizes of small DATA frames into a
> > larger WINDOW_UPDATE might be useful, too, but is independent and
> > complimentary to my patch proposed in
> > https://github.com/haproxy/haproxy/pull/1732

*complementary  [correcting myself]

> I agree that it's independent but it's the one that is not expected to
> cause any regression with any possible client. That's why I'd like to
> have the two. First that one because it should be durable. Second, your
> patch as an optimization, and if anyone complains about trouble with it
> we can safely revert it or decide what to do without worrying about all
> deployed curl clients that will not be updated.

Were this to cause a theoretical regression, this default initial window
size can be configured in haproxy, so the workaround is to configure the
value back to 65535.

> > One approach might be to collect small DATA frames and send a
> > WINDOW_UPDATE once a threshold is reached.
> 
> I thought about that one, focusing on 1/16 of a max frame size at once,
> but I don't like it much because it means we'd delay some WU, and the
> H2 spec was very clear about the fact that we ought to do our best to
> timely ack data in order to avoid deadlocks. You can have clients that
> stop sending until they're able to read a new block of a preconfigured
> window size for example (and that's where 65535 was an absurd choice
> in the first place). For example, a client that is hard-coded to use
> a 65535 buffer and needs to wait for the WU to replace that buffer
> would not receive an update for the block that spans from 61440 to
> 65535 and would not update the buffer. Thus I really want to be certain
> that we do not let processed DATA frames without the equivalent WU sent
> back.
> 
> > The upcoming lighttpd 1.4.65
> > does just that, collecting updates until 16384 is reached, and then
> > sending a WINDOW_UPDATE for 16384.  Cost: a single 1-RTT delay in
> > sending a WINDOW_UPDATE to continue uploads larger than 112k-1.
> 
> It can definitely cause some inefficient clients to hang if they need
> their buffer to be fully acked and their size is not a multiple of the
> frame size. Be careful about this.

I ended up adjusting this before releasing lighttpd 1.4.65.

For the same few instructions, lighttpd 1.4.65 now sends WINDOW_UPDATE
for 16384 when lighttpd receives the first DATA frame containing data
(1-16384 bytes), and then does not send WINDOW_UPDATE until the next
16384 block is started.  This avoids any delay in sending WINDOW_UPDATE,
though it does (temporarily) increase the window size allowed in the
client if the client has not already sent data consuming the window.

> > Another approach might be to pre-emptively effectively double the
> > window size by sending WINDOW_UPDATE with the entire window size (65535)
> > back to the client upon receive of first DATA frame, and then keeping
> > track on server-side until that 65535 is exhausted before sending
> > another WINDOW_UPDATE.
> 
> Sending too large stream window sizes is problematic when some streams
> can be processed slowly, as it easily causes head-of-line blocking. If
> the advertised window is larger than the available storage, a paused
> stream will block other ones.

<sigh> we know that poorly written code knows no bounds.  Since the
server is permitted to increase the window size, inability by the client
to increase the window size is a bug in the client.  The client need
only track the window size -- the client need not actually use the extra
window size if the client does not wish to do so.

Cheers, Glenn

Reply via email to