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

httpAgent.absoluteMaxSockets #31942

Closed
seansd-zz opened this issue Feb 25, 2020 · 3 comments
Closed

httpAgent.absoluteMaxSockets #31942

seansd-zz opened this issue Feb 25, 2020 · 3 comments
Labels
feature request Issues that request new features to be added to Node.js. http Issues or PRs related to the http subsystem.

Comments

@seansd-zz
Copy link

We are trying to ensure the maximum number of active connections from NodeJS to backend systems is maintained. httpAgent.maxSockets behaves at a "per origin" level which is also dynamic and not completely deterministic. This makes it difficult to create logic and patterns as the number of connections grows. Instead we propose absoluteMaxSockets (or some other clever name), that enforces a maximum number of active sockets regardless of the origin.

Doing this now is possible in the user space, but requires a great deal of code and logic to bind to each and every socket connection event and do the counting and enforcement, etc.

Describe the solution you'd like
Assuming httpAgent.absoluteMaxSockets is implemented, if the number of open sockets across all origins grows beyond this number, the http agent would first attempt to check and see if any sockets can be closed, and if so attempt to close those sockets prior to opening a new one.
Assuming one cannot be open, an exception would be thrown at the time a request is made since a new socket connection cannot be opened.

Describe alternatives you've considered
As mentioned, we could implement this in user space, but it's tricky and cumbersome.

@bnoordhuis bnoordhuis added feature request Issues that request new features to be added to Node.js. http Issues or PRs related to the http subsystem. labels Mar 1, 2020
@bnoordhuis
Copy link
Member

Can I suggest you open a pull request with your proposed changes? It might get rejected but feature requests with working code stand a better chance of getting accepted than those without.

@rickyes
Copy link
Contributor

rickyes commented May 23, 2020

@bnoordhuis I'm willing to try to work on that if the demand is reasonable.

@bnoordhuis
Copy link
Member

@rickyes Sure, go for it. :-)

@ronag ronag closed this as completed in 7a5d3a2 Jun 21, 2020
codebytere pushed a commit that referenced this issue Jun 27, 2020
Add maxTotalSockets to determine how many sockets an agent can open.
Unlike maxSockets, The maxTotalSockets does not count by per origin.

PR-URL: #33617
Fixes: #31942
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere pushed a commit that referenced this issue Jun 30, 2020
Add maxTotalSockets to determine how many sockets an agent can open.
Unlike maxSockets, The maxTotalSockets does not count by per origin.

PR-URL: #33617
Fixes: #31942
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
rickyes added a commit to rickyes/node that referenced this issue Sep 28, 2020
Add maxTotalSockets to determine how many sockets an agent can open.
Unlike maxSockets, The maxTotalSockets does not count by per origin.

PR-URL: nodejs#33617
Fixes: nodejs#31942
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this issue Sep 28, 2020
Add maxTotalSockets to determine how many sockets an agent can open.
Unlike maxSockets, The maxTotalSockets does not count by per origin.

PR-URL: #33617
Backport-PR-URL: #35396
Fixes: #31942
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this issue Sep 28, 2020
PR-URL: #34013
Backport-PR-URL: #35396
Refs: #33617
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: #33617
Fixes: #31942
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. http Issues or PRs related to the http subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants