Re: [PATCH] BUG/MINOR: checks: prevent http keep-alive with http-check expect

2015-01-31 Thread Willy Tarreau
On Fri, Jan 30, 2015 at 12:07:07AM +0100, Cyril Bonté wrote:
> Sébastien Rohaut reported that string negation in http-check expect didn't
> work as expected.
> 
> The misbehaviour is caused by responses with HTTP keep-alive. When the
> condition is not met, haproxy awaits more data until the buffer is full or the
> connection is closed, resulting in a check timeout when "timeout check" is
> lower than the keep-alive timeout on the server side.
> 
> In order to avoid the issue, when a "http-check expect" is used, haproxy will
> ask the server to disable keep-alive by automatically appending a
> "Connection: close" header to the request.

Very nice, thank you Cyril for this work. I've applied it to both 1.6 and 1.5.

Cheers,
Willy




[PATCH] BUG/MINOR: checks: prevent http keep-alive with http-check expect

2015-01-31 Thread Cyril Bonté
Sébastien Rohaut reported that string negation in http-check expect didn't
work as expected.

The misbehaviour is caused by responses with HTTP keep-alive. When the
condition is not met, haproxy awaits more data until the buffer is full or the
connection is closed, resulting in a check timeout when "timeout check" is
lower than the keep-alive timeout on the server side.

In order to avoid the issue, when a "http-check expect" is used, haproxy will
ask the server to disable keep-alive by automatically appending a
"Connection: close" header to the request.
---
 doc/configuration.txt | 4 
 src/checks.c  | 3 +++
 2 files changed, 7 insertions(+)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 2295744..7c1edd8 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -2905,6 +2905,10 @@ http-check expect [!]  
   waste some CPU cycles, especially when regular expressions are used, and that
   it is always better to focus the checks on smaller resources.
 
+  Also "http-check expect" doesn't support HTTP keep-alive. Keep in mind that 
it
+  will automatically append a "Connection: close" header, meaning that this
+  header should not be present in the request provided by "option httpchk".
+
   Last, if "http-check expect" is combined with "http-check disable-on-404",
   then this last one has precedence when the server responds with 404.
 
diff --git a/src/checks.c b/src/checks.c
index feca96e..1b5b731 100644
--- a/src/checks.c
+++ b/src/checks.c
@@ -1427,6 +1427,9 @@ static int connect_conn_chk(struct task *t)
else if ((check->type) == PR_O2_HTTP_CHK) {
if (s->proxy->options2 & PR_O2_CHK_SNDST)
bo_putblk(check->bo, trash.str, 
httpchk_build_status_header(s, trash.str, trash.size));
+   /* prevent HTTP keep-alive when "http-check expect" is 
used */
+   if (s->proxy->options2 & PR_O2_EXP_TYPE)
+   bo_putstr(check->bo, "Connection: close\r\n");
bo_putstr(check->bo, "\r\n");
*check->bo->p = '\0'; /* to make gdb output easier to 
read */
}
-- 
2.1.4