Skip to content

Keepalive connections closed by server #225

Closed
wtfuzz opened this Issue Mar 23, 2013 · 6 comments

2 participants

@wtfuzz
wtfuzz commented Mar 23, 2013

I've been trying to track down why errback was being called with no error being reported on keepalive connections.

I'm testing against Passenger on Nginx. In the debugger I could see that the EM reactor was calling unbind on the connection. It turns out it's caused by nginx imposing a 100 request limit by default per keepalive connection. (Even though nginx log files imply the client has closed the connection, you can clearly see in tcpdump that the server sends the FIN).

The last response from the server does include 'Connection: close', so I think it would make sense for em-http-request to handle this, and/or provide some feedback that the remote end has dropped the connection.

@igrigorik
Owner

Interesting, didn't realize nginx has this behavior -- is that configurable?

It's not clear to me how to handle this in an intuitive way to the user in em-http. The last request will finish successfully, so we have to fire the callback. However, it's only if and when you try to reuse the connection that it will fail - that's the behavior you're running into.

@wtfuzz
wtfuzz commented Mar 24, 2013

Yes, it's configurable with keepalive_requests in http, server or location contexts of the nginx config file. You can't seem to disable it, but can set it to a huge number.

Would it make sense to flag the connection as closing when Connection: close is present in a response, and set some state in the unbind callback once the connection has been torn down?

It seems the example that EM shows (http://eventmachine.rubyforge.org/EventMachine/Connection.html#unbind-instance_method) for distinguishing between client/server close of a socket requires a state instance variable.

The errback is what gets called when trying to reuse the connection, but there's no indication as to why the request failed (error is nil).

@wtfuzz
wtfuzz commented Mar 24, 2013

The test code I've been playing with is here: https://gist.github.com/wtfuzz/5230558

It will perform up to count (10000) get requests over a single keepalive connection without pipelining. With default nginx errback will be called after 100 requests and it will call EventMachine.stop

@igrigorik
Owner

Well, we could definitely add a variable to indicate that the connection has been closed, but it's not clear to me how this would actually help an average user: this would require that you check that variable before submitting any new request, which is functionally equivalent to just defining an errback function..

@wtfuzz
wtfuzz commented Mar 25, 2013

I didn't mean to check the state before each request, but rather have some way of checking why the errback was called.

Currently error is nil in the errback after the connection is closed by the peer, which isn't useful at all. I wonder if others here #190 have run into this issue as well and didn't realize it?

It may also make sense to by default or optionally transparently reconnect a keepalive connection if the server decides to tear it down on a subsequent request rather than failing.

@igrigorik
Owner

+1 for reporting an error in the callback.

Reconnecting is tricky. What happens to pipelined calls, which may have executed already on the server? There are too many edge (and undefined) cases here.. it's not really suitable for "sane defaults".

@igrigorik igrigorik closed this in 9615041 Jun 22, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.