Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #570: Use http.globalAgent as agent if no agent is explicitly specified. #572

Closed
wants to merge 1 commit into from

Conversation

jwalton
Copy link

@jwalton jwalton commented Feb 3, 2014

Make it so we default to http.globalAgent as the agent if no agent is explicitly specified. This is the agent that HTTP requests use by default when no agent is specified, so this is more in keeping with the http module, and it also fixes #570. :)

globalAgent was introduced way back in node 0.5.3, so this should be a reasonably safe change as far as compatibility goes.

@Rush
Copy link
Contributor

Rush commented Feb 3, 2014

I think it is not safe at all. Using agent may severely degrade performance. And no agent works well on Node 0.11.x.

@jwalton
Copy link
Author

jwalton commented Feb 3, 2014

As things stand, in node 0.10.x, using no agent will cause you to run out of file handles under heavy load, which will absolutely degrade performance. :P

If node 0.11 isn't recycling connections, it's performance is likely going to be worse than not using an Agent. If they are recycling connections, they're probably using http.globalAgent anyways.

@jwalton
Copy link
Author

jwalton commented Feb 3, 2014

node-http-proxy is calling http.request() to proxy connections. In node 0.10.25, this creates a new ClientRequest. In node 0.11.11, http.request calls into globalAgent.request(). If you pass in false as the agent, then globalAgent.request() will modify your options and assign you a new agent for each request, however the new agent will turn off keep-alive. That's why it works in 0.11.

@jcrugzz
Copy link
Contributor

jcrugzz commented Feb 3, 2014

@jwalton if you plan to receive heavy load, you should be specifying an agent with a large amount of sockets (enough where you wont run into the case where you run out of file handles). Regardless, if you get to this point, it is an indicator you should probably scale to multiple servers. At nodejitsu we use an agent for our load balancers and specify around 320,000 for maxSockets in the agent.

One thing we cannot assume here is that it will be an http agent and the reason the default is set to false is because we do not want people just getting started to hit the caveat of having a default socket pool of 5. The agent: false covers the widest range of use cases and we will be in the proper situation when node 0.12.x hits.

@jwalton
Copy link
Author

jwalton commented Feb 3, 2014

@jcrugzz Well, allow me to define "heavy load". On OS/X Mavericks, http-proxy will currently stop processing new requests after two minutes if you exceed a sustained 2 requests/second, assuming you haven't bumped up your max file handle count. :P

@jwalton
Copy link
Author

jwalton commented Feb 3, 2014

On a Linux machine, assuming you have no user-level limits imposed by your host, assuming your system-wide file handle limit is 75K (this is distro dependent), and assuming the service you are proxying to is not on the same machine, then http-proxy can sustain ~625 connections/second for two minutes. At this point, the Kernel will be unable to open new files. You won't be able to SSH into the machine. If you are SSHed in, you won't be able to run commands like "ls" because the shell won't be able to stat ls, ls won't be able to stat any of the libraries it needs to load to run, and even if it could it wouldn't be able to read the directory you're currently in.

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

Successfully merging this pull request may close these issues.

node-http-proxy leaks file descriptors
3 participants