From 055ba4f505e5117f59d3b843afc4c737dd2e21d5 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Tue, 24 Jul 2018 17:05:54 +0200 Subject: [PATCH] BUG/MEDIUM: stats: don't ask for more data as long as we're responding The stats applet is still a bit hackish. It uses the HTTP txn to parse the POST contents. Due to this it pretends not having parsed the request from the buffer so that the HTTP parser continues to work fine on these data. This comes with a side effect : the request lies pending in the channel's buffer, and because of this, stream_int_update_applet() always wakes the applet up. It's very visible when retrieving a large stats page over a slow link as haproxy eats 100% of the CPU waiting for the data to leave. While the proper long term solution definitely is to consume these data and parse the body from the applet, changing this is not suitable for a fix. What this patch does instead is to disable request polling as long as there are pending data in the response buffer. Given that for almost all cases, the applet remains busy sending data, this is at least enough to ensure that we don't wake up for the pending request data while we're waiting for the client to receive these data. Now a 5k backend stats page is dumped at 1% CPU over a 10 Mbps link instead of 100%, using 1500 epoll_wait() calls instead of 80000. Note that the previous fix (BUG/MEDIUM: stream-int: don't immediately enable reading when the buffer was reportedly full) is necessary for the effects of the fix to be noticed since both bugs have the exact same effect. This fix must be backported at least as far as 1.5. --- src/stats.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/stats.c b/src/stats.c index 2632ef780..4cea77686 100644 --- a/src/stats.c +++ b/src/stats.c @@ -3148,7 +3148,15 @@ static void http_stats_io_handler(struct appctx *appctx) } } out: - /* just to make gcc happy */ ; + /* we have left the request in the buffer for the case where we + * process a POST, and this automatically re-enables activity on + * read. It's better to indicate that we want to stop reading when + * we're sending, so that we know there's at most one direction + * deciding to wake the applet up. It saves it from looping when + * emitting large blocks into small TCP windows. + */ + if (!channel_is_empty(res)) + si_applet_stop_get(si); } /* Dump all fields from into using the "show info" format (name: value) */