Skip to content

Commit

Permalink
all: avoid DNS with GRPC_PROXY_EXP
Browse files Browse the repository at this point in the history
In some environments DNS is not available and is performed by the
CONNECT proxy. Nothing "special" should need to be done for these
environments, but the previous support took shortcuts which knowingly
would not support such environments.

This change should fix both OkHttp and Netty. Netty's
Bootstrap.connect() resolved the name immediately whereas using
ChannelPipeline.connect() waits until the address reaches the end of the
pipeline. Netty uses NetUtil.toSocketAddressString() to get the name of
the address, which uses InetSocketAddress.getHostString() when
available.

OkHttp is still using InetSocketAddress.getHostName() which may issue
reverse DNS lookups. However, if the reverse DNS lookup fails, it should
convert the IP to a textual string like getHostString(). So as long as
the reverse DNS maps to the same machine as the IP, there should only be
performance concerns, not correctness issues. Since the DnsNameResolver
is creating unresolved addresses, the reverse DNS lookups shouldn't
occur in the common case.
  • Loading branch information
ejona86 committed Jan 27, 2017
1 parent 23f5a6f commit 65e4d9f
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 16 deletions.
9 changes: 9 additions & 0 deletions core/src/main/java/io/grpc/internal/DnsNameResolver.java
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,15 @@ public void run() {
resolving = true;
}
try {
if (System.getenv("GRPC_PROXY_EXP") != null) {
ResolvedServerInfoGroup servers = ResolvedServerInfoGroup.builder()
.add(new ResolvedServerInfo(
InetSocketAddress.createUnresolved(host, port), Attributes.EMPTY))
.build();
savedListener.onUpdate(Collections.singletonList(servers), Attributes.EMPTY);
return;
}

try {
inetAddrs = getAllByName(host);
} catch (UnknownHostException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,6 @@
import io.netty.handler.ssl.SslContext;
import java.io.File;
import java.io.FileInputStream;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.UnknownHostException;
import java.nio.charset.Charset;
import javax.net.ssl.SSLSocketFactory;

Expand Down Expand Up @@ -304,16 +301,6 @@ private class Tester extends AbstractInteropTest {
@Override
protected ManagedChannel createChannel() {
if (!useOkHttp) {
InetAddress address;
try {
address = InetAddress.getByName(serverHost);
if (serverHostOverride != null) {
// Force the hostname to match the cert the server uses.
address = InetAddress.getByAddress(serverHostOverride, address.getAddress());
}
} catch (UnknownHostException ex) {
throw new RuntimeException(ex);
}
SslContext sslContext = null;
if (useTestCa) {
try {
Expand All @@ -323,7 +310,8 @@ protected ManagedChannel createChannel() {
throw new RuntimeException(ex);
}
}
return NettyChannelBuilder.forAddress(new InetSocketAddress(address, serverPort))
return NettyChannelBuilder.forAddress(serverHost, serverPort)
.overrideAuthority(serverHostOverride)
.flowControlWindow(65 * 1024)
.negotiationType(useTls ? NegotiationType.TLS : NegotiationType.PLAINTEXT)
.sslContext(sslContext)
Expand Down
5 changes: 3 additions & 2 deletions netty/src/main/java/io/grpc/netty/NettyClientTransport.java
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,9 @@ public Runnable start(Listener transportListener) {
* that it may begin buffering writes.
*/
b.handler(negotiationHandler);
channel = b.register().channel();
// Start the connection operation to the server.
channel = b.connect(address).addListener(new ChannelFutureListener() {
channel.connect(address).addListener(new ChannelFutureListener() {
@Override
public void operationComplete(ChannelFuture future) throws Exception {
if (!future.isSuccess()) {
Expand All @@ -209,7 +210,7 @@ public void operationComplete(ChannelFuture future) throws Exception {
future.channel().pipeline().fireExceptionCaught(future.cause());
}
}
}).channel();
});
// Start the write queue as soon as the channel is constructed
handler.startWriteQueue(channel);
// This write will have no effect, yet it will only complete once the negotiationHandler
Expand Down

0 comments on commit 65e4d9f

Please sign in to comment.