Skip to content
This repository has been archived by the owner on Apr 7, 2021. It is now read-only.

Disable keep-alive connections. #70

Merged
merged 1 commit into from
Dec 14, 2014
Merged

Disable keep-alive connections. #70

merged 1 commit into from
Dec 14, 2014

Conversation

rogerhu
Copy link
Contributor

@rogerhu rogerhu commented Dec 12, 2014

If there are a sufficient # of connections opened/file descriptors exceeded, Camo will start returning back 404 because of the saturated number of connections.

If there are a sufficient # of connections opened/file descriptors exceeded, Camo will start returning back 404
because of the saturated number of connections.
@atmos
Copy link
Owner

atmos commented Dec 12, 2014

Can you write a test that explains what you're going for more clearly?

@rogerhu
Copy link
Contributor Author

rogerhu commented Dec 12, 2014

Bottom line: NodeJS sends Connection: keep-alive headers when you make outbound requests. If you have Camo making 1000+ simulataneous requests, you will be capped by the total # of file descriptors per process and will depend on the socket timeout of the connection to be reclaimed.

http://nodejs.org/api/http.html

image

I looked through the Ruby test suite and tried to run it just now, though there seems to be some incompat issues with Ruby v2.1 -- but essentially this is what I would want:

def test_closes_connections
  uri = request_uri("http://example.org/")
  response = RestClient.get("#{config['host']}#{path}")
  assert_equal "Connection", "close"
end

@rogerhu
Copy link
Contributor Author

rogerhu commented Dec 12, 2014

I looked through your tests and they appear to be integration tests. They don't verify what headers are being issued by NodeJS in https://github.com/atmos/camo/blob/master/server.js#L115. The agent: parameter is left to be default, which is causing keep-alive connections to occur for each proxy request.

@rogerhu
Copy link
Contributor Author

rogerhu commented Dec 12, 2014

I was able to verify by using a netcat listener on a proxied site that Camo accesses and could see Connection: close instead of Connection: keep-alive.

@rvarshney
Copy link

👍

1 similar comment
@iritmalka
Copy link

👍

@adepue
Copy link

adepue commented Dec 12, 2014

Yes please. I use a cronjob to cycle the process now to get back sockets.

@akshayjshah
Copy link

👍

atmos added a commit that referenced this pull request Dec 14, 2014
Disable keep-alive connections.
@atmos atmos merged commit 1717d2f into atmos:master Dec 14, 2014
@rogerhu
Copy link
Contributor Author

rogerhu commented Dec 16, 2014

Thanks! Wondering when the next release will be out.. :)

@atmos
Copy link
Owner

atmos commented Dec 16, 2014

There aren't any real releases. I can tag a newer version if that'll help you.

@rogerhu
Copy link
Contributor Author

rogerhu commented Dec 16, 2014

Would appreciate it...thanks!

@atmos
Copy link
Owner

atmos commented Dec 17, 2014

I tagged v2.2.0

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants