Repository navigation
Allow choosing the ManagedChannelProvider based on the NameResolver #3085
Description
ManagedChannelBuilder.forTarget knows the scheme of the target, but doesn't pass it to the ManagedChannelProvider. This makes it hard for the service loader to pick a proper Provider, because it may pick incorrectly.
Consider passing the scheme to the MCP, so that it can decide whether or not its available.
Activity
ejona86 commentedon Jun 12, 2017
I don't understand how you've been led to think this.
ManagedChannelBuilder.forTargetimmediately turns intoManagedChannelProvider.builderForTarget.ManagedChannelBuilderbasically knows nothing and letsManagedChannelProviderdo all the work.In addition, we don't actually use the scheme in the channel at all;
NameResolver.Factoryinstances are the only thing making use of the scheme today. While I'd like to change that so that we choose theNameResolver.Factorybased on the scheme, it's still name resolver logic.If this is so that
ManagedChannelProvidercan choose a compatible provider, then I've been considering allowing theNameResolverandManagedChannelProviderto return aCollection<Class<? extends SocketAddress>>to coordinate selecting a compatible channel. There could be a InProcessNameResolver that returnsInProcessSocketAddressor a UnixNameResolver that returns Netty'sDomainSocketAddress.carl-mastrangelo commentedon Jun 12, 2017
Channel builders may wish to exclude themselves from the running if they know they cannot support certain names. For example, if the target passed was uds:///var/grpc.socket, OkHTTP would not be an appropriate transport. It cannot exclude itself except via priority, which is kind of hacky.
Both
priorityandisAvailabletry to return the best answer, but they really don't know what they are signing up for. MCB knows only the target being connected to, so its the only thing the Provider has to go on.ejona86 commentedon Jun 12, 2017
The transport should not hard-code a white/black list of schemes/name resolvers; that assumes you know all name resolvers and how they behave.
My suggestion about having a
Collection<Class<? extends SocketAddress>>negotiation seems like it is for exactly the use case you mentioned. Since OkHttp wouldn't supportDomainSocketAddressit would be skipped for a uds name resolver.[-]Consider passing scheme to ManagedChannelProvider[/-][+]Allow choosing the ManagedChannelProvider based on the NameResolver[/+]carl-mastrangelo commentedon Aug 17, 2018
Thinking about this some, there is some complication where the API doesn't work out so great. NameResolvers (in my branch) produce a
Set<? extends Class<? extends SocketAddress>>. This set contains all the possible socketAddress types that could be returned. The default implementation just returns a singleton set of SocketAddress, implying no restriction. This is overridden by each NameResolver.Factory.The immediate issue is that when trying to pick a NameResolver, each one could return several types of Addresses which could intersect the Channel supported types. It isn't clear how to rank these intersections and pick a name resolver. On the flip side, there may be multiple kinds of channels, each which consume different kinds sockets. If resolver 1 can produce socket addresses A+B, resolver 2 can produce socket addresses B+C+D, and channel 1 accepts sockets of type B+C, and channel 2 accepts sockets of type A+D, it isn't clear which resolver to use. Channel 1 and resolver 1 could be the higher priority, but an error could occur when the resolver produces an A. The problem exists too when switching to resolver 2 which might produce a D, still causing an error.
Maybe I'm overthinking it, but I think we need to know the Socket type before we can pick ClientTransportFactory (oh yeah, Netty might pick a different transport type based on the address, such as Nio or Epoll, a decision which is made at channel build, not at transport creation). Maybe it's good enough if there is any intersection. To support this, the NR could say what kind of SocketAddress would be produced for a given name, and then see if it works with the channels in priority order. Roughly:
@ejona86 WDYT?
ejona86 commentedon Aug 17, 2018
Don't choose a name resolver by this mechanism. Name resolvers should still be selected by their scheme+priority. Instead, the chosen name resolver should influence which transport is chosen.
If resolver 1 was selected, then there is no compatible channel. If resolver 2 was selected, there is no compatible channel. The channel needs to support all possible return types, because it doesn't know which one it will get.
carl-mastrangelo commentedon Aug 17, 2018
The compatibility is not clear here. A NR may overwhelmingly return one type of socket address vs. another, so it would be surprising to a user who see an error on channel creation. Resolver 1 could produce InetSocketAddresses or LocalAddresses. The user may be passing in only domain names, which would mean the second kind would never be returned in reality, but it would preclude OkHTTP from being supported.
Likewise, if we made a super NameResolver that could return Inprocess, Inet, and Unix domain addresses, no channel would support all of them. Forcing the Channel to support all possible addresses, would also prevent adding support for NRs for new socket types, for fear of breaking channel build.
ejona86 commentedon Aug 17, 2018
Not really. But let's say it is surprising. Wouldn't it be even more surprising if you're in production and then all of a sudden your application is broken because the NR started returning different results?
Don't do that. Easy. "Quit hitting yourself."
In general, I would expect a NR to only return one type. It's unclear whether we consider IPv4 and IPv6 as two types, but that's the sort of differences I'm expecting from a NR.
carl-mastrangelo commentedon Aug 17, 2018
This is very much the case. Consider a hypothetical NettyResolver which can resolve DNS, and Unix Addresses. The NettyClientTransport may only support DNS because Epoll isn't available, but the NR doesn't know that. We would end up not using the NettyName Resolver with the Netty Channel. That's wrong. (and, forcing the NR to know about all supported transports to say which sockets can be resolved is wrong).
Not so easy. We would start getting into special casing about what is a socket address anyways? For example, DnsNameResolver can return one of two types of addresses. (when you go look you'll say "oh well that's different", but try expressing that in the javadoc)
ejona86 commentedon Aug 17, 2018
I'm going to stop you there. Why is this name resolver wanting to do that? Is it essential that we support that use case?
If you feel that it is essential to support, then it seems it requires that the NameResolver do I/O before address negotiation can take place. That would require us to re-architect ManagedChannelBuilder and likely move ManagedChannelImpl into
io.grpc. While maybe an option, I would expect a really strong reason as to why it was necessary.carl-mastrangelo commentedon Aug 17, 2018
To do automatically do the right thing? To pass a single "target" string and gRPC figure out how to construct a channel that just works. Otherwise, we might as well not have ManagedChannelProvider or NameResolverProvider at all and force people to be up front about how they want it to work.
Doing a half hearted automatic channel construction is worse than not at all. Seeing as #4750 clearly needs to be spec'd out more clearly, I'm going to unassign myself.
idelvall commentedon Jan 17, 2019
From my perspective, the root of the problem is that both, channels and NRs, are loaded via service loaders. This means that they are loaded in a complete independent way, when the reality is that, as you pointed out @carl-mastrangelo, there exist certain entanglement between them.
Maybe i am missing something, but wouldn't it be simpler if NR providers don't exist at all and only explicit NR factories are used?
I mean, channel implementations could define a default NR factory by themselves, one granted to be coherent with their supported address types, and on the other hand application developers already can specify a different NR factory instance if they are interested in doing so.
So why NR providers exist?
ST-DDT commentedon Oct 4, 2019
Channeltype selection (in-process/netty) viaNameResolvers or a similar class would be a very nice feature for config based application setups. E.g. use in-process for tests and netty for production without changing a single line of code.dapengzhang0 commentedon Oct 4, 2019
@ST-DDT
NameResolvershould only be able to distinguish target types (e.g. in-process or not in-process), but should not distinguish transport types (e.g. netty or okhttp).