search

twitter / finagle

A fault tolerant, protocol-agnostic RPC system
https://twitter.github.io/finagle
Apache License 2.0
8.78k stars 1.45k forks source link

Non-blocking DNS resolution #929

Closed fwbrasil closed 2 years ago

fwbrasil commented 2 years ago

Is your feature request related to a problem? Please describe.

We use DNS at Nubank to resolve names and the blocking behavior of the DNS resolution is a problem for one of our services that distributes requests to several other services. It often has a large number of threads doing DNS resolution and that contributes to CPU throttling. Example:

image

Describe the solution you'd like

Finagle should support non-blocking DNS resolution by default or by configuration.

Describe alternatives you've considered

We can use the service loading mechanism introduced by https://github.com/twitter/finagle/commit/e9e2239e7fa0bc15dabd5a84ac3a9c3575e09844 and have a separate implementation but I it'd be great if Finagle had this feature built in.

Additional context

I worked on an async DNS resolver as part of the fiber scheduler project (see earlier phab versions, I think I removed it eventually when the kafka client wasn't a problem anymore). It'd be great if it could be open sourced.

heligw commented 2 years ago

we have created a ticket to look into it, we'll reach out to you when we have updates. thanks!

vkostyukov commented 2 years ago

Hey @fwbrasil, I'm working on it. I have a draft phab that I'm testing internally at the moment.

fwbrasil commented 2 years ago

awesome news folks! Thank you very much :)

vkostyukov commented 2 years ago

It took us a while but it's landed! https://github.com/twitter/finagle/commit/ca456b8e1f905f02e032b52a773af3c7fdaa4d3b

It's behind a toggle if you want to enable it for your apps. Will do more testing internally before enabling it by default.

fwbrasil commented 2 years ago

nice! this is going to be very helpful, thank you!!!

fwbrasil commented 2 years ago

@vkostyukov are there plans to make a new release with this change?

vkostyukov commented 2 years ago

Yea, this should make it into the July release.

Update: We do publish snapshots nightly, maybe you want to try depending on them in the meantime?