DNS: use Netty's asynchronous DNS resolver

[erimatnor]

The current Netty-based transport doesn't use Netty's asynchronous DNS resolver, which means the client hangs if hostnames cannot be resolved. This is really bad if you are reusing an eventloop from other parts of a program and this eventloop is not supposed to block. gRPC should at least expose the necessary APIs to be able to override the default DNS resolver.

[carl-mastrangelo]

I see a possibility of it hanging if an error unrelated to resolution happens inside the resolver, but I don't think a bad host name would cause this. DnsNameResolver can be overridden when constructing the channel.

[ejona86]

The current DNS name resolver does not block any event loop. Instead, it uses another thread:
https://github.com/grpc/grpc-java/blob/master/core/src/main/java/io/grpc/DnsNameResolver.java#L90

The executor used is semi-private. Users can't access it directly, but user work does get scheduled to it. But it is also created via Executors.newCachedThreadPool, so the behavior is similar to just creating a new Thread in some ways (since heavy use just creates more threads).

While it could be nice in some ways to use Netty's async DNS resolver, we still need a DNS name resolver when using OkHttp. It's also a bit difficult to share event loops/configuration given the design, but not impossible. Netty's DNS resolver doesn't support UDP, so it would also likely take longer to do resolutions, especially when outside a data center.

So I see this as possibly future work, but it seems today there shouldn't be any strong problems with the current implementation in a threading model regard. Is there something I'm missing?

[erimatnor]

@ejona86 Looking at the code, it appears that you are right that the DNS resolver isn't blocking the client eventloop. Still, it is blocking the shared executor, which I guess is fine because it is a cached thread pool.

In any case, I am seeing the client hanging for a long time if no DNS is available or hosts cannot be resolved. Since the standard resolver is using plain Java DNS resolution this is not unexpected I guess because timeouts are long. The problem is, however, that there is no error reported in the client callback within the given deadline when sending with the client.

I think it would be nice if the DNS resolution would timeout with an error within the given deadline when sending with withDeadlineAfter().

[erimatnor]

@ejona86 btw, I think you are wrong that Netty's DNS resolver doesn't support UDP.

http://netty.io/4.1/api/io/netty/resolver/dns/DnsNameResolverBuilder.html#channelFactory(io.netty.channel.ChannelFactory)

Also, FYI. I was able to implement an alternative DnsNameResolver with a Netty backend that has nicer behavior. So, definitely good that you can override the default resolver at least.

[ejona86]

Leaving this open as a feature request. We don't necessarily have to have the Netty-based DNS resolver as the default.

I think it would be nice if the DNS resolution would timeout with an error within the given deadline when sending with withDeadlineAfter().

Hmm... That would be nice, although it is quite hard to describe precisely. If the timeout is 1ns, then blaming DNS isn't quite appropriate. I'd believe there is work to do here still, including tuning the DNS timeout.

I think you are wrong that Netty's DNS resolver doesn't support UDP.

Oh, that's interesting and good to know. I'll have to look at it more some time, and figure out how you're informed when to swap from UDP to TCP when the response is too large and other such details. I still have uncertainty on whether it is a good default (even when already using Netty), but it is worth looking into.

I was able to implement an alternative DnsNameResolver with a Netty backend that has nicer behavior.

Good to hear. Something like that we could probably put in the netty package for others that are interested, if you're willing/able to contribute it. I would believe you aren't the only person that would want it. Most of the reservations I have about it are having it as a default; if the user explicitly enables it then I have virtually no concerns and see it as really useful.