Le 18/01/2019 à 14:23, Luke Seelenbinder a écrit :
Quick clarification on the previous message.
The code emitting the warning is almost assuredly here:
https://github.com/haproxy/haproxy/blob/ed7a066b454f09fee07a9ffe480407884496461b/src/proto_htx.c#L3242
not in proto_http.c, seeing how this is in htx mode not http mode.
I've traced the issue to likely being caused by the following condition false:
https://github.com/haproxy/haproxy/blob/202c6ce1a27c92d21995ee82c71b2f70c636e3ea/src/htx.c#L93
We are dealing with a lot of larger responses (PNGs, 50-100KB/request on avg)
with perhaps 10 simultaneous initial requests on the same h2 connection being
very common. That sounds like I may in fact need to tweak some buffer settings
somewhere. In http/1.1 mode, these requests were spread out across four
connections with browsers blocking until the previous connection finished.
The documentation is only somewhat helpful for tune.bufsize and
tune.maxrewrite, http/2 and large requests. If this isn't a bug, would someone
be willing to offer some guidance into good values for these buffer sizes?
Thanks for your help!
Best,
Luke
Hi Luke,
Could you try following patches please ?
Thanks,
--
Christopher Faulet
>From ccea4c140c8958507e8c91f14354e986eb8aabe6 Mon Sep 17 00:00:00 2001
From: Christopher Faulet <[email protected]>
Date: Mon, 21 Jan 2019 11:24:38 +0100
Subject: [PATCH 1/3] BUG/MINOR: proto-htx: Return an error if all headers
cannot be receied at once
When an HTX stream is waiting for a request or a response, it reports an error
(400 for the request or 502 for the response) if a parsing error is reported by
the mux (HTX_FL_PARSING_ERROR). The mux-h1 uses this error, among other things,
when the headers are too big to be analyzed at once. But the mux-h2 doesn't. So
the stream must also report an error if the multiplexer is unable to emit all
headers at once. The multiplexers must always emit all the headers at once
otherwise it is an error.
There are 2 ways to detect this error:
* The end-of-headers marker was not received yet _AND_ the HTX message is not
empty.
* The end-of-headers marker was not received yet _AND_ the multiplexer have
some data to emit but it is waiting for more space in the channel's buffer.
Note the mux-h2 is buggy for now when HTX is enabled. It does not respect the
reserve. So there is no way to hit this bug.
This patch must be backported to 1.9.
---
src/proto_htx.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/src/proto_htx.c b/src/proto_htx.c
index 9fa820653..9e285f216 100644
--- a/src/proto_htx.c
+++ b/src/proto_htx.c
@@ -126,9 +126,12 @@ int htx_wait_for_request(struct stream *s, struct channel *req, int an_bit)
*/
if (unlikely(htx_is_empty(htx) || htx_get_tail_type(htx) < HTX_BLK_EOH)) {
/*
- * First catch invalid request
+ * First catch invalid request because of a parsing error or
+ * because only part of headers have been transfered.
+ * Multiplexers have the responsibility to emit all headers at
+ * once.
*/
- if (htx->flags & HTX_FL_PARSING_ERROR) {
+ if ((htx->flags & HTX_FL_PARSING_ERROR) || htx_is_not_empty(htx) || (s->si[0].flags & SI_FL_RXBLK_ROOM)) {
stream_inc_http_req_ctr(s);
stream_inc_http_err_ctr(s);
proxy_inc_fe_req_ctr(sess->fe);
@@ -1086,7 +1089,7 @@ int htx_wait_for_request_body(struct stream *s, struct channel *req, int an_bit)
* been received or if the buffer is full.
*/
if (htx_get_tail_type(htx) >= HTX_BLK_EOD ||
- htx_used_space(htx) + global.tune.maxrewrite >= htx->size)
+ channel_htx_full(req, htx, global.tune.maxrewrite))
goto http_end;
missing_data:
@@ -1457,9 +1460,14 @@ int htx_wait_for_response(struct stream *s, struct channel *rep, int an_bit)
*/
if (unlikely(co_data(rep) || htx_is_empty(htx) || htx_get_tail_type(htx) < HTX_BLK_EOH)) {
/*
- * First catch invalid response
+ * First catch invalid response because of a parsing error or
+ * because only part of headers have been transfered.
+ * Multiplexers have the responsibility to emit all headers at
+ * once. We must be sure to have forwarded all outgoing data
+ * first.
*/
- if (htx->flags & HTX_FL_PARSING_ERROR)
+ if (!co_data(rep) &&
+ ((htx->flags & HTX_FL_PARSING_ERROR) || htx_is_not_empty(htx) || (s->si[1].flags & SI_FL_RXBLK_ROOM)))
goto return_bad_res;
/* 1: have we encountered a read error ? */
--
2.20.1
>From 1654f144f9815a46be126ded3ac04db7fc68c40e Mon Sep 17 00:00:00 2001
From: Christopher Faulet <[email protected]>
Date: Mon, 21 Jan 2019 11:49:37 +0100
Subject: [PATCH 2/3] BUG/MEDIUM: mux-h2/htx: Respect the channel's reserve by
checking the flag CO_RFL_KEEP_RSV
When data are pushed in the channel's buffer, in h2_rcv_buf(), the mux-h2 must
respect the reserve if the flag CO_RFL_KEEP_RSV is set. In HTX, because the
stream-interface always sees the buffer as full, there is no other way to know
the reserve must be respected.
This patch must be backported to 1.9.
---
src/mux_h2.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/mux_h2.c b/src/mux_h2.c
index 20ff98821..acc38e8e2 100644
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -5051,7 +5051,12 @@ static size_t h2_rcv_buf(struct conn_stream *cs, struct buffer *buf, size_t coun
}
buf_htx = htx_from_buf(buf);
- count = htx_free_space(buf_htx);
+ count = htx_free_data_space(buf_htx);
+ if (flags & CO_RFL_KEEP_RSV) {
+ if (count <= global.tune.maxrewrite)
+ goto end;
+ count -= global.tune.maxrewrite;
+ }
htx_ret = htx_xfer_blks(buf_htx, h2s_htx, count, HTX_BLK_EOM);
--
2.20.1
>From c2b34108bafc67692cf2d7c7ac3c9622f502fa0a Mon Sep 17 00:00:00 2001
From: Christopher Faulet <[email protected]>
Date: Mon, 21 Jan 2019 11:55:02 +0100
Subject: [PATCH 3/3] BUG/MINOR: mux-h1: Apply the reserve on the channel's
buffer only
There is no reason to use the reserve to limit the amount of data of the input
buffer that we can parse at a time. The only important thing is to keep the
reserve free in the channel's buffer.
This patch should be backported to 1.9.
---
src/mux_h1.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/src/mux_h1.c b/src/mux_h1.c
index 4b0f46b63..a5d60c3c4 100644
--- a/src/mux_h1.c
+++ b/src/mux_h1.c
@@ -1064,12 +1064,17 @@ static size_t h1_process_headers(struct h1s *h1s, struct h1m *h1m, struct htx *h
*/
static size_t h1_process_data(struct h1s *h1s, struct h1m *h1m, struct htx *htx,
struct buffer *buf, size_t *ofs, size_t max,
- struct buffer *htxbuf)
+ struct buffer *htxbuf, size_t reserve)
{
- uint32_t data_space = htx_free_data_space(htx);
+ uint32_t data_space;
size_t total = 0;
int ret = 0;
+ data_space = htx_free_data_space(htx);
+ if (data_space <= reserve)
+ goto end;
+ data_space -= reserve;
+
if (h1m->flags & H1_MF_XFER_LEN) {
if (h1m->flags & H1_MF_CLEN) {
/* content-length: read only h2m->body_len */
@@ -1175,6 +1180,10 @@ static size_t h1_process_data(struct h1s *h1s, struct h1m *h1m, struct htx *htx,
}
if (!h1m->curr_len) {
h1m->state = H1_MSG_CHUNK_CRLF;
+ data_space = htx_free_data_space(htx);
+ if (data_space <= reserve)
+ goto end;
+ data_space -= reserve;
goto new_chunk;
}
goto end;
@@ -1303,22 +1312,14 @@ static size_t h1_process_input(struct h1c *h1c, struct buffer *buf, int flags)
struct htx *htx;
size_t total = 0;
size_t ret = 0;
- size_t count, max;
+ size_t count, rsv;
int errflag;
htx = htx_from_buf(buf);
count = b_data(&h1c->ibuf);
- max = htx_free_space(htx);
- if (flags & CO_RFL_KEEP_RSV) {
- if (max < global.tune.maxrewrite)
- goto end;
- max -= global.tune.maxrewrite;
- }
- if (count > max)
- count = max;
-
if (!count)
goto end;
+ rsv = ((flags & CO_RFL_KEEP_RSV) ? global.tune.maxrewrite : 0);
if (!conn_is_back(h1c->conn)) {
h1m = &h1s->req;
@@ -1336,7 +1337,7 @@ static size_t h1_process_input(struct h1c *h1c, struct buffer *buf, int flags)
break;
}
else if (h1m->state <= H1_MSG_TRAILERS) {
- ret = h1_process_data(h1s, h1m, htx, &h1c->ibuf, &total, count, buf);
+ ret = h1_process_data(h1s, h1m, htx, &h1c->ibuf, &total, count, buf, rsv);
htx = htx_from_buf(buf);
if (!ret)
break;
@@ -1346,7 +1347,7 @@ static size_t h1_process_input(struct h1c *h1c, struct buffer *buf, int flags)
break;
}
else if (h1m->state == H1_MSG_TUNNEL) {
- ret = h1_process_data(h1s, h1m, htx, &h1c->ibuf, &total, count, buf);
+ ret = h1_process_data(h1s, h1m, htx, &h1c->ibuf, &total, count, buf, rsv);
htx = htx_from_buf(buf);
if (!ret)
break;
--
2.20.1
