Re: HTX & tune.maxrewrite [1.9.2]
Hi Christopher, I can confirm the patches fixed the issue. Thanks again for fixing this up! Best, Luke — Luke Seelenbinder Stadia Maps | Founder stadiamaps.com ‐‐‐ Original Message ‐‐‐ On Monday, January 21, 2019 2:07 PM, Christopher Faulet wrote: > 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 publickey - luke.seelenbinder@stadiamaps.com - 0xB23C1E8A.asc Description: application/pgp-keys signature.asc Description: OpenPGP digital signature
Re: HTX & tune.maxrewrite [1.9.2]
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
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
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 backporte
Re: HTX & tune.maxrewrite [1.9.2]
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! Hi Luke, I'm able to reproduce the bug. The H2/HTX multiplexer is not aware of the buffer reserve. So tweaking tune.maxrewrite won't have any effect. I'll work on a fix. Thanks, -- Christopher Faulet
Re: HTX & tune.maxrewrite [1.9.2]
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 ‐‐‐ Original Message ‐‐‐ On Friday, January 18, 2019 1:10 PM, Luke Seelenbinder wrote: > Hello all, > > I just rolled out 1.9.2 compiled from the official tarball to a subset of our > servers, and I'm observing some odd behavior in the logs. I'm seeing the > following warning (with accompanying warnings about failed hdr rewrites in > the stats page): > > Proxy foo failed to add or set the response header 'bar' for request #1380. > You might need to increase tune.maxrewrite > > I've tweaked tune.maxrewrite to 2048 with no apparent affect (it was not > previously set). The further odd thing is that the header is present on the > client side (seemingly every time). This is does not happen with an identical > config in 1.8.x (obviously sans h2 on both ends & option http-use-htx). > > Any ideas regarding what I should investigate? The line emitting the warning > seems to be > https://github.com/haproxy/haproxy/blob/master/src/proto_http.c#L1630. > > I'm happy to try patches additional configuration changes if that would > assist. I assume it's something slightly amiss in the HTX setup or my > configuration thereof. > > Best, > Luke > > — > Luke Seelenbinder > Stadia Maps | Founder > stadiamaps.com publickey - luke.seelenbinder@stadiamaps.com - 0xB23C1E8A.asc Description: application/pgp-keys signature.asc Description: OpenPGP digital signature
HTX & tune.maxrewrite [1.9.2]
Hello all, I just rolled out 1.9.2 compiled from the official tarball to a subset of our servers, and I'm observing some odd behavior in the logs. I'm seeing the following warning (with accompanying warnings about failed hdr rewrites in the stats page): Proxy foo failed to add or set the response header 'bar' for request #1380. You might need to increase tune.maxrewrite I've tweaked tune.maxrewrite to 2048 with no apparent affect (it was not previously set). The further odd thing is that the header is present on the client side (seemingly every time). This is does not happen with an identical config in 1.8.x (obviously sans h2 on both ends & option http-use-htx). Any ideas regarding what I should investigate? The line emitting the warning seems to be https://github.com/haproxy/haproxy/blob/master/src/proto_http.c#L1630. I'm happy to try patches additional configuration changes if that would assist. I assume it's something slightly amiss in the HTX setup or my configuration thereof. Best, Luke — Luke Seelenbinder Stadia Maps | Founder stadiamaps.com publickey - luke.seelenbinder@stadiamaps.com - 0xB23C1E8A.asc Description: application/pgp-keys signature.asc Description: OpenPGP digital signature
