Hi Cyril,

On Mon, Dec 27, 2010 at 08:01:19PM +0100, Cyril Bonté wrote:
> Le lundi 27 décembre 2010 07:10:37, Willy Tarreau a écrit :
> > It's nice that you managed to reproduce it ! I'm not sure how you managed
> > to get it though because you're saying that you have it with and without
> > nginx.
> 
> Oh sorry, I thought it was clear enough with the first mail.
> Ok, here is the full test.
> 
> nginx is in front of haproxy with this configuration :
> events {
>       worker_connections 1000;
> }
> 
> http {
>       server {
>               listen 8080;
>               proxy_set_header Host $host;
>               proxy_set_header X-Real-IP $remote_addr;
>               proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
>               location /httpclose {
>                       proxy_pass http://127.0.0.1:8081;
>               } 
>               location /tunnel {
>                       proxy_pass http://127.0.0.1:8082;
>               } 
>               location /forceclose {
>                       proxy_pass http://127.0.0.1:8083;
>               } 
>       }
> }

OK, first, I did not understand you used nginx *in front* of haproxy,
I thought it was behind.

> haproxy is run with :
> listen httpclose_test :8081
>       mode http
>       option httpclose
>       option http-pretend-keepalive
>       server www localhost:80
> 
> listen tunnel_test :8082
>       mode http
>       option http-pretend-keepalive
>       server www localhost:80
> 
> listen forceclose_test :8083
>       mode http
>       option forceclose
>       option http-pretend-keepalive
>       server www localhost:80
> 
> The backend is apache with KeepAliveTimeout 15.

(...)
> Now, I can also observe the same behaviour using ab directly on haproxy :
> $ ab http://localhost:8081/
> => 15s delay
> 
> $ ab http://localhost:8082/
> => 15s delay
> 
> $ ab http://localhost:8083/
> => immediate response

OK it gets clearer now.

> > I must say I'm having difficulties to understand the logics. The rule is
> > becoming quite complex, so we should probably first try to define how
> > we'd like pretend-keep-alive to work in various cases.
> 
> Same here, maybe it should/can be simplified in the 1.5 branch (or future 
> 1.6).
> And i fear it will become more and more complex the day haproxy supports 
> client side keep-alive (if it will).

I intend to set holidays at the beginning of the year to complete the
keep-alive work. It requires a full week and it's impossible to work
on that once in a while. Once we get full keep-alive, I'll see if we
can make it the default and deprecate some other modes (eg: tunnel mode).

> Due to this complexity, I really wanted a second look...which was needed (the 
> typo in the variable name confirmed it :-)).
> 
> > In my opinion, this option's goal is to make a server think we're sending
> > it a keep-alive request so that it can announce a valid content-length or
> > chunking to the client, eventhough we intend to close its connection. So
> > what I'm thinking is that this option should only be considered if either
> > "httpclose", "forceclose" or "http-server-close" is enabled in either the
> > frontend or backend, or if the request indicates a close.
> > 
> > I also think that if we have httpclose (or the request indicated a close)
> > with the option set, then we can assume the server will not close after
> > its response, which can make the client wait for a timeout. Thus, we should
> > probably turn the response to a forced close in this case.
> > 
> > Do you agree with that ? Is it what you wanted to do ?
> 
> I agree. You precisely explained what I tried to fix.

OK, then I need to reread the code, because it was not obvious to me that
it could achieve that. But I know for sure that it's tricky and sometimes
many cases can factor into a single non-obvious test.

> > Do you see any other case that must be covered ?
> 
> I only see the "tunnel mode" case, where http-pretend-keepalive also 
> introduces this timeout side effect. With this patch, a forced close is 
> applied 
> in that case too.

I'm not very fond of performing a close in the tunnel mode. The reason why
it's still supported is that some people use it as the poor man's keepalive
because with many apps, if you don't need content-switching nor logs, it
can reliably work. If the patch only acts on requests with a "close" header
then I'm fine with it, but I don't want to close all requests when the option
is set.

> If it's ok for you, I'll resend the updated patch in a correct way.

Yes, feel free to resend. By then, I'll try to sort out why it works ;-)

Thanks!
Willy


Reply via email to