OK, I managed to reproduce it and to find the cause. It was a side effect of an optimization of the chunked-encoding mode which was globally applied instead of being applied only to chunked-encoded data. With chunks, we always have to see the last empty chunk so we can decide to enable TCP delayed ACKs or not on a chunk-by-chunk basis. In content-length mode, we cannot (and must not).
I'll release 1.4.20 shortly with this fix as well as another one. In the mean time you can apply the attached patch (which works for both 1.4 and 1.5). Thanks Randy for the details you shared, there were helpful to reproduce the issue ! Willy
>From 869fc1edc2acfa8ff88de2f4e638ad48dca5d246 Mon Sep 17 00:00:00 2001 From: Willy Tarreau <[email protected]> Date: Mon, 5 Mar 2012 08:29:20 +0100 Subject: BUG: http: disable TCP delayed ACKs when forwarding content-length data Commits 5c6209 and 072930 were aimed at avoiding undesirable PUSH flags when forwarding chunked data, but had the undesired effect of causing data advertised by content-length to be affected by the delayed ACK too. This can happen when the data to be forwarded are small enough to fit into a single send() call, otherwise the BF_EXPECT_MORE flag would be removed. Content-length data don't need the BF_EXPECT_MORE flag since the low-level forwarder already knows it can safely rely on bf->to_forward to set the appropriate TCP flags. Note that the issue is only observed in requests at the moment, though the later introduction of server-side keep-alive could trigger the issue on the response path too. Special thanks to Randy Shults for reporting this issue with a lot of details helping to reproduce it. The fix must be backported to 1.4. --- src/proto_http.c | 16 ++++++++++++---- 1 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/proto_http.c b/src/proto_http.c index f36dc3c..c71b839 100644 --- a/src/proto_http.c +++ b/src/proto_http.c @@ -4684,9 +4684,13 @@ int http_request_forward_body(struct session *s, struct buffer *req, int an_bit) /* We know that more data are expected, but we couldn't send more that * what we did. So we always set the BF_EXPECT_MORE flag so that the * system knows it must not set a PUSH on this first part. Interactive - * modes are already handled by the stream sock layer. + * modes are already handled by the stream sock layer. We must not do + * this in content-length mode because it could present the MSG_MORE + * flag with the last block of forwarded data, which would cause an + * additional delay to be observed by the receiver. */ - req->flags |= BF_EXPECT_MORE; + if (txn->flags & TX_REQ_TE_CHNK) + req->flags |= BF_EXPECT_MORE; http_silent_debug(__LINE__, s); return 0; @@ -5731,9 +5735,13 @@ int http_response_forward_body(struct session *s, struct buffer *res, int an_bit /* We know that more data are expected, but we couldn't send more that * what we did. So we always set the BF_EXPECT_MORE flag so that the * system knows it must not set a PUSH on this first part. Interactive - * modes are already handled by the stream sock layer. + * modes are already handled by the stream sock layer. We must not do + * this in content-length mode because it could present the MSG_MORE + * flag with the last block of forwarded data, which would cause an + * additional delay to be observed by the receiver. */ - res->flags |= BF_EXPECT_MORE; + if (txn->flags & TX_RES_TE_CHNK) + res->flags |= BF_EXPECT_MORE; /* the session handler will take care of timeouts and errors */ http_silent_debug(__LINE__, s); -- 1.7.2.1.45.g54fbc
