[FIX] http-pretend-keepalive and httpclose/tunnel mode

2010-12-28 Thread Cyril Bonté
Since haproxy 1.4.9, combining option httpclose and option
http-pretend-keepalive can leave the connections opened until the backend
keep-alive timeout is reached, providing bad performances.
The same can occur when the proxy is in tunnel mode.

This patch ensures that the server side connection is closed after the
response and ignore http-pretend-keepalive in tunnel mode.
---
 src/proto_http.c |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/proto_http.c b/src/proto_http.c
index 3ea4c8a..bcae5d8 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -3064,7 +3064,8 @@ int http_process_req_common(struct session *s, struct 
buffer *req, int an_bit, s
 
if ((s-fe-options|s-be-options)  PR_O_KEEPALIVE)
tmp = TX_CON_WANT_KAL;
-   if ((s-fe-options|s-be-options)  PR_O_SERVER_CLO)
+   if ((s-fe-options|s-be-options)  PR_O_SERVER_CLO ||
+   ((s-fe-options2|s-be-options2)  PR_O2_FAKE_KA))
tmp = TX_CON_WANT_SCL;
if ((s-fe-options|s-be-options)  PR_O_FORCE_CLO)
tmp = TX_CON_WANT_CLO;
@@ -3541,8 +3542,7 @@ int http_process_request(struct session *s, struct buffer 
*req, int an_bit)
 
/* 11: add Connection: close or Connection: keep-alive if needed 
and not yet set. */
if (((txn-flags  TX_CON_WANT_MSK) != TX_CON_WANT_TUN) ||
-   ((s-fe-options|s-be-options)  PR_O_HTTP_CLOSE) ||
-   ((s-fe-options2|s-be-options2)  PR_O2_FAKE_KA)) {
+   ((s-fe-options|s-be-options)  PR_O_HTTP_CLOSE)) {
unsigned int want_flags = 0;
 
if (txn-flags  TX_REQ_VER_11) {
-- 
1.7.2.3




Re: [FIX] http-pretend-keepalive and httpclose/tunnel mode

2010-12-28 Thread Willy Tarreau
Hi Cyril,

OK, now I understand better your logics on this one. Initially I didn't
understand because I did not read the patch in its whole context, but
given the remaining checks on the PR_O2_FAKE_KA later, I agree that your
solution looks like the best one. I have tried it and I can confirm it
does what we want in every situation I could make up (with both HTTP/1.0
and HTTP/1.1).

However, I was worried about the impact once we have server-side keep-alive.

So I tried to change it slightly so that it sets TX_CON_WANT_KAL instead of
TX_CON_WANT_SCL, since after all it matches what we're looking for : announce
keep-alive to the server. And doing so gave me the exact same results. Here's
a copy-paste of the first chunk of the patch :

@@ -3062,7 +3062,8 @@ int http_process_req_common(struct session *s, struct 
buffer *req, int an_bit, s
 (s-be-options  
(PR_O_KEEPALIVE|PR_O_SERVER_CLO|PR_O_HTTP_CLOSE|PR_O_FORCE_CLO {
int tmp = TX_CON_WANT_TUN;
 
-   if ((s-fe-options|s-be-options)  PR_O_KEEPALIVE)
+   if ((s-fe-options|s-be-options)  PR_O_KEEPALIVE ||
+   ((s-fe-options2|s-be-options2)  PR_O2_FAKE_KA))
tmp = TX_CON_WANT_KAL;
if ((s-fe-options|s-be-options)  PR_O_SERVER_CLO)
tmp = TX_CON_WANT_SCL;

I also find it easier to understand that it will announce keep-alive to the
server if we set WANT_KAL than if we set WANT_SCL. To me, it means announce
keep-alive to the server, whatever you'll use with the client.

If you agree with this change, I'll apply it that way. I'll also remove the
leftover from a previous fix consisting in a  1 to replace a condition
checking FAKE_KA.

I think that a 1.4.12 will be needed, considering we already had a report for
this issue :-/

Thanks !
Willy




Re: [FIX] http-pretend-keepalive and httpclose/tunnel mode

2010-12-28 Thread Cyril Bonté
Le mardi 28 décembre 2010 21:35:09, Willy Tarreau a écrit :
 I also find it easier to understand that it will announce keep-alive to the
 server if we set WANT_KAL than if we set WANT_SCL. To me, it means
 announce keep-alive to the server, whatever you'll use with the client.
 
 If you agree with this change, I'll apply it that way.

It's OK for me, all the tests passed with nginx in front of haproxy with this 
update (and also using ab).

 I think that a 1.4.12 will be needed, considering we already had a report
 for this issue :-/

1.4.11, or I missed something :-)

-- 
Cyril Bonté



Re: [FIX] http-pretend-keepalive and httpclose/tunnel mode

2010-12-28 Thread Willy Tarreau
On Tue, Dec 28, 2010 at 09:50:15PM +0100, Cyril Bonté wrote:
 Le mardi 28 décembre 2010 21:35:09, Willy Tarreau a écrit :
  I also find it easier to understand that it will announce keep-alive to the
  server if we set WANT_KAL than if we set WANT_SCL. To me, it means
  announce keep-alive to the server, whatever you'll use with the client.
  
  If you agree with this change, I'll apply it that way.
 
 It's OK for me, all the tests passed with nginx in front of haproxy with this 
 update (and also using ab).

Perfect, thanks for the tests !

  I think that a 1.4.12 will be needed, considering we already had a report
  for this issue :-/
 
 1.4.11, or I missed something :-)

Oh yes you're right, seems like it's time to go back home :-)

Cheers,
Willy