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

Master #18

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Master #18

wants to merge 9 commits into from

Conversation

waysay
Copy link
Contributor

@waysay waysay commented Apr 8, 2017

Added a few extra request handlers to avoid connection timeouts with http and https requests.

@megastef
Copy link
Owner

Are you able to solve conflicts?

I've got a few thoughts about this issue. Please note JAVA (crate db) might start 100 java threads when nodejs uses 100 sockets. This might kill your crate instance (and gets you more http timeouts ..). I have seen many times that Java servers (e.g. Elasticsearch) got overloaded or started to reject connections when hitting thread limits. Now imagine 100 clients using this lib connect with 100 sockets ... it would be nice to make maxSockets configurable e.g. via an environment variable. We could set a new httpAgent https://nodejs.org/api/http.html#http_new_agent_options, instead of modifying global httpAgent settings (because other parts of the node program might need more sockets for other backends ...). In a long run might be better to queue requests (with a limited number of parallel actions to keep Java side healthy).

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.

3 participants