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
>From bebc3e5740864ed8b7cf51e95ed38a97c0643cdf Mon Sep 17 00:00:00 2001
From: Willy Tarreau <[email protected]>
Date: Sun, 9 Feb 2014 08:31:49 +0100
Subject: BUG/MINOR: channel: initialize xfer_small/xfer_large on new buffers
These ones are only reset during transfers. There is a low but non-null
risk that a first full read causes the previous value to be reused and
immediately to immediately set the CF_STREAMER flag. The impact is only
to increase earlier than expected the SSL record size and to use splice().
This bug was already present in 1.4, so a backport is possible.
---
include/proto/channel.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/proto/channel.h b/include/proto/channel.h
index 49a68b2..0afdc65 100644
--- a/include/proto/channel.h
+++ b/include/proto/channel.h
@@ -55,6 +55,7 @@ static inline void channel_init(struct channel *chn)
chn->buf->i = 0;
chn->buf->p = chn->buf->data;
chn->to_forward = 0;
+ chn->xfer_small = chn->xfer_large = 0;
chn->total = 0;
chn->pipe = NULL;
chn->analysers = 0;
--
1.7.12.2.21.g234cd45.dirty
>From 185f5e581a2b9aad2986758e43ce7f4ed8e42c43 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <[email protected]>
Date: Sun, 9 Feb 2014 17:45:16 +0100
Subject: MINOR: channel: add the date of last read in the channel
We store the time stamp of last read in the channel in order to
be able to measure some bit rate and pause lengths. We only use
16 bits which were unused for this. We don't need more, as it
allows us to measure with a millisecond precision for up to 65s.
---
include/proto/channel.h | 1 +
include/types/channel.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/include/proto/channel.h b/include/proto/channel.h
index 0afdc65..36633c6 100644
--- a/include/proto/channel.h
+++ b/include/proto/channel.h
@@ -55,6 +55,7 @@ static inline void channel_init(struct channel *chn)
chn->buf->i = 0;
chn->buf->p = chn->buf->data;
chn->to_forward = 0;
+ chn->last_read = now_ms;
chn->xfer_small = chn->xfer_large = 0;
chn->total = 0;
chn->pipe = NULL;
diff --git a/include/types/channel.h b/include/types/channel.h
index 905308e..2eea3e8 100644
--- a/include/types/channel.h
+++ b/include/types/channel.h
@@ -172,6 +172,7 @@ struct channel {
struct stream_interface *prod; /* producer attached to this channel */
struct pipe *pipe; /* non-NULL only when data present */
unsigned int to_forward; /* number of bytes to forward after out
without a wake-up */
+ unsigned short last_read; /* 16 lower bits of last read date (max
pause=65s) */
unsigned char xfer_large; /* number of consecutive large xfers */
unsigned char xfer_small; /* number of consecutive small xfers */
unsigned long long total; /* total data read */
--
1.7.12.2.21.g234cd45.dirty
>From 603e536d63dbf2bd7cb87e52e5a8a747cee17fc5 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <[email protected]>
Date: Sun, 9 Feb 2014 17:47:01 +0100
Subject: MEDIUM: stream-int: automatically disable CF_STREAMER flags after
idle
Disabling the streamer flags after an idle period will help TCP proxies
to better adapt to the streams they're forwarding, especially with SSL
where this will allow the SSL sender to use smaller records. This is
typically used to optimally relay HTTP and derivatives such as SPDY or
HTTP/2 in pure TCP mode when haproxy is used as an SSL offloader.
---
src/stream_interface.c | 96 ++++++++++++++++++++++++++------------------------
1 file changed, 50 insertions(+), 46 deletions(-)
diff --git a/src/stream_interface.c b/src/stream_interface.c
index b3364a3..83e814c 100644
--- a/src/stream_interface.c
+++ b/src/stream_interface.c
@@ -1096,6 +1096,17 @@ static void si_conn_recv_cb(struct connection *conn)
cur_read = 0;
+ if ((chn->flags & (CF_STREAMER | CF_STREAMER_FAST)) && !chn->buf->o &&
+ (unsigned short)(now_ms - chn->last_read) >= 1000) {
+ /* The buffer was empty and nothing was transferred for more
+ * than one second. This was caused by a pause and not by
+ * congestion. Reset any streaming mode to reduce latency.
+ */
+ chn->xfer_small = 0;
+ chn->xfer_large = 0;
+ chn->flags &= ~(CF_STREAMER | CF_STREAMER_FAST);
+ }
+
/* First, let's see if we may splice data across the channel without
* using a buffer.
*/
@@ -1190,38 +1201,6 @@ static void si_conn_recv_cb(struct connection *conn)
chn->total += ret;
if (channel_full(chn)) {
- /* The buffer is now full, there's no point in going
through
- * the loop again.
- */
- if (!(chn->flags & CF_STREAMER_FAST) && (cur_read ==
buffer_len(chn->buf))) {
- chn->xfer_small = 0;
- chn->xfer_large++;
- if (chn->xfer_large >= 3) {
- /* we call this buffer a fast streamer
if it manages
- * to be filled in one call 3
consecutive times.
- */
- chn->flags |= (CF_STREAMER |
CF_STREAMER_FAST);
- //fputc('+', stderr);
- }
- }
- else if ((chn->flags & (CF_STREAMER |
CF_STREAMER_FAST)) &&
- (cur_read <= chn->buf->size / 2)) {
- chn->xfer_large = 0;
- chn->xfer_small++;
- if (chn->xfer_small >= 2) {
- /* if the buffer has been at least half
full twice,
- * we receive faster than we send, so
at least it
- * is not a "fast streamer".
- */
- chn->flags &= ~CF_STREAMER_FAST;
- //fputc('-', stderr);
- }
- }
- else {
- chn->xfer_small = 0;
- chn->xfer_large = 0;
- }
-
si->flags |= SI_FL_WAIT_ROOM;
break;
}
@@ -1237,20 +1216,6 @@ static void si_conn_recv_cb(struct connection *conn)
* not have them in buffers.
*/
if (ret < max) {
- if ((chn->flags & (CF_STREAMER | CF_STREAMER_FAST)) &&
- (cur_read <= chn->buf->size / 2)) {
- chn->xfer_large = 0;
- chn->xfer_small++;
- if (chn->xfer_small >= 3) {
- /* we have read less than half of the
buffer in
- * one pass, and this happened at least
3 times.
- * This is definitely not a streamer.
- */
- chn->flags &= ~(CF_STREAMER |
CF_STREAMER_FAST);
- //fputc('!', stderr);
- }
- }
-
/* if a streamer has read few data, it may be because we
* have exhausted system buffers. It's not worth trying
* again.
@@ -1269,6 +1234,45 @@ static void si_conn_recv_cb(struct connection *conn)
if (conn->flags & CO_FL_ERROR)
return;
+ if (cur_read) {
+ if ((chn->flags & (CF_STREAMER | CF_STREAMER_FAST)) &&
+ (cur_read <= chn->buf->size / 2)) {
+ chn->xfer_large = 0;
+ chn->xfer_small++;
+ if (chn->xfer_small >= 3) {
+ /* we have read less than half of the buffer in
+ * one pass, and this happened at least 3 times.
+ * This is definitely not a streamer.
+ */
+ chn->flags &= ~(CF_STREAMER | CF_STREAMER_FAST);
+ }
+ else if (chn->xfer_small >= 2) {
+ /* if the buffer has been at least half full
twice,
+ * we receive faster than we send, so at least
it
+ * is not a "fast streamer".
+ */
+ chn->flags &= ~CF_STREAMER_FAST;
+ }
+ }
+ else if (!(chn->flags & CF_STREAMER_FAST) &&
+ (cur_read >= chn->buf->size - global.tune.maxrewrite))
{
+ /* we read a full buffer at once */
+ chn->xfer_small = 0;
+ chn->xfer_large++;
+ if (chn->xfer_large >= 3) {
+ /* we call this buffer a fast streamer if it
manages
+ * to be filled in one call 3 consecutive times.
+ */
+ chn->flags |= (CF_STREAMER | CF_STREAMER_FAST);
+ }
+ }
+ else {
+ chn->xfer_small = 0;
+ chn->xfer_large = 0;
+ }
+ chn->last_read = now_ms;
+ }
+
if (conn_data_read0_pending(conn))
/* connection closed */
goto out_shutdown_r;
--
1.7.12.2.21.g234cd45.dirty
