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

Does needle support dns failovers? #198

Open
BadgerBadgerBadgerBadger opened this issue Jan 18, 2017 · 9 comments
Open

Does needle support dns failovers? #198

BadgerBadgerBadgerBadger opened this issue Jan 18, 2017 · 9 comments

Comments

@BadgerBadgerBadgerBadger

Node http does not and neither does request. Does needle have a way to handle dns round-robin failover and try all ips?

@tomas
Copy link
Owner

tomas commented Jan 18, 2017

No it doesn't. We'd need to hack some low-level Node.js internals for that, wouldn't we?

@BadgerBadgerBadgerBadger
Copy link
Author

@tomas Not really. They've now introduced an option in their dns.lookup function called all: <boolean>. It's false by default but if set to true, it'll return all ips for a hostname. So if the dns lookup is decoupled from the http request, you could fetch all ips and try them sequentially for one that works and fail the entire request if none of them works.

@rchipka
Copy link

rchipka commented Jan 18, 2017

@ScionOfBytes Interesting Idea. Any idea what the API for this might be?

My opinion is usually "if it's possible to do it elsewhere, keep it out of needle", but in this case it's something that has to be integrated into the API.

Lately I've been debating whether it's worth the headache to migrate node-osmosis to something other than needle. This feature might give me reason to stay on needle. It'd probably be a good feature to have on production web scrapers.

@tomas
Copy link
Owner

tomas commented Jan 18, 2017

My opinion is pretty much the same as @rchipka's (to keep needle nimble), but if this might help make it more robust and less prone to dns lookup errors, I'm all for it.

I'll dive into this later or tomorrow and see if it can be implemented in a clean way.

@BadgerBadgerBadgerBadger
Copy link
Author

@rchipka Ideally this wouldn't need a change in the API as failovers would be handled behind the scenes. Since node introduced the all flag for their dns.lookup function in an opt-in manner, so should needle do the same. Maybe introduce an opt-in flag in the options in the style of dns_lookup_resolve_all: <boolean>.

When this flag is passed, needle passes the all flag to the dns.lookup function and sequentially tries all ips before returning.

There's a discussion (now closed) on nodejs/node#708 about this issue and how to handle errors returned from multiple ip tries is something of a concern (nodejs/node#708 (comment)) and this (nodejs/node#708 (comment)) seems like a way to go. From the point of view of the client, there should only be one server and not multiple ones to worry about. And only if all of them fail should the client even be concerned about handling errors.

@tomas
Copy link
Owner

tomas commented Jan 19, 2017

Ok, I just wrote an initial implementation here. You can pass a host_ip option to needle which can either be a specific IP address, an array of IP addresses, or 'any', in which case needle will use dns.lookup() with any: true in order to get a list of IP addresses to try sequentially (as with the IP array option).

So far it seems to work pretty well. Check it out and let me know what you think.

@BadgerBadgerBadgerBadger
Copy link
Author

@tomas Will test ASAP and report results.

@tomas
Copy link
Owner

tomas commented Jan 26, 2017 via email

@tomas
Copy link
Owner

tomas commented Feb 13, 2018

ping @ScionOfBytes !

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

No branches or pull requests

3 participants