Hey Willy. This is great work, thanks! On Sat, Feb 8, 2014 at 11:49 PM, Willy Tarreau <w...@1wt.eu> wrote: > > The observations and analysis are interesting, and the choice of solutions > is not easy. I think that relying on a pause only is dangerous because > there > are generally a number of losses when delivering to clients, causing pauses > during the transfers, and we wouldn't want to reset the window in this > case, > TCP congestion control already handles this. >
Right, that makes sense. I reran the same test page against SPDY3.1 and HTTP backends: SPDY/3.1 - http://www.webpagetest.org/result/140211_KF_2257217df0d1fe65353b8e8aa415de9b/3/details/ - http://cloudshark.org/captures/c8eab73137e5?filter=tcp.stream%3D%3D1 -- packet #271: new request after 1.2s idle timeout > record size is reset and then ramps to 16K -- packet #605: new request after 2.5s+ idle timeout > record size is reset HTTP/1.1 - http://www.webpagetest.org/result/140211_DF_df85fe6c3e91c96bd38e6c8d1df77ceb/4/details/(uses 2 TCP connections) -- http://cloudshark.org/captures/b17452deed52?filter=tcp.stream%3D%3D5 --- transfers three objects with 1s+ idle timeouts, and record size is reset and ramps in each run -- http://cloudshark.org/captures/b17452deed52?filter=tcp.stream%3D%3D9 --- transfers three objects: record size is reset on new request (no idle timeout - see packet #287), and also after 1s+ timeout. The only difference between SPDY and HTTP/1.1 cases here is that HAProxy "understands" HTTP/1.1 and resets the record size on one of the connections with back-to-back requests. As we discussed earlier in the thread, I think this behavior makes sense in HTTP/1.1 land, so that's great! Even better, with the new logic, the SPDY connection also works as advertised, with record size resets when buffer is empty + timeout is reached. In short, this looks awesome. :) One last set of followup question on configuration and defaults: - we allow the user to tune buffer sizes - that's great. - we allow the user to adjust record sizes: assuming above logic is in place, can we change the default size to start small by default? - should we expose the timeout as an additional config variable? I think this is a reasonable knob to have. 1s could be a default value. ig On Sun, Feb 9, 2014 at 9:00 AM, Willy Tarreau <w...@1wt.eu> wrote: > Hi Ilya, > > I've finished the change. It seems to do the right thing for me with > HTTP, though I have not tested with SPDY. > > If a read happens after a pause of more than one second where the > output buffer was empty, we reset the streamer flags. Thus it will > cover the case where the client sends a new request and will not > reset the flags in case of occasional congestion. > > The delay is hard-coded here in stream_interface.c:si_conn_recv_cb() : > > if ((chn->flags & (CF_STREAMER | CF_STREAMER_FAST)) && > !chn->buf->o && > (unsigned short)(now_ms - chn->last_read) >= 1000) { > > You may want to experiment with other values, though I'm not much > convinced it's worth going really below if we don't want to hit some > long RTTs on 3G during a slowstart happening with initcwnd=2. > > I'm appending the 3 patches to be applied on top of current git (though > they should apply to what you already have). > > Do not hesitate to suggest further improvements ! > > Thanks, > Willy > >
