Skip to content

Commit

Permalink
Fix for initial routers DNS resolution
Browse files Browse the repository at this point in the history
This update fixes the following issue: neo4j#833

The desired behaviour for getting a routing table from the initial router (either on bootstrap or when all known routers have failed) is:
- resolve the domain name to all IPs
- attempt getting a routing table from all of them until first one succeeds by:
  - getting a connection
  - trying to get a successful routing table response

Prior to this change, the connection pools were created for host and port pairs. When domain name of the host resolves to multiple IP addresses, such pools provide connections to those IPs as a group. While this works for readers and writers, it negatively impacts the routing table fetching process as there is no guarantee which IP address the provided connection is setup for.

This update delivers the following changes:
- connection pools for routers are IP address based, which allows for deterministic connection retrieval
- the resolved IP address set is kept up-to-date (in case known router IPs change) to make sure that the unused connection pools are flushed
- the domain name resolution logic has been made configurable (it is private at the moment and is used to facilitate testing)
- the testkit backend has been updated to support the domain name resolution configuration (a new test has been added to testkit to cover the issue described above)
- the testkit backend has been updated to support connection timeout driver configuration
- several tests have been updated to adopt the new changes
  • Loading branch information
injectives committed Mar 15, 2021
1 parent 47db433 commit 8049f7e
Show file tree
Hide file tree
Showing 36 changed files with 797 additions and 294 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,14 @@
import java.net.InetSocketAddress;
import java.net.SocketAddress;
import java.net.URI;
import java.net.UnknownHostException;
import java.util.List;
import java.util.Collections;
import java.util.LinkedHashSet;
import java.util.Objects;
import java.util.stream.Stream;
import java.util.Set;

import org.neo4j.driver.net.ServerAddress;

import static java.util.Objects.requireNonNull;
import static java.util.stream.Collectors.toList;

/**
* Holds a host and port pair that denotes a Bolt server address.
Expand All @@ -43,8 +42,8 @@ public class BoltServerAddress implements ServerAddress
private final String host; // This could either be the same as originalHost or it is an IP address resolved from the original host.
private final int port;
private final String stringValue;

private InetAddress resolved;
private final Set<BoltServerAddress> resolved;

public BoltServerAddress( String address )
{
Expand All @@ -58,15 +57,15 @@ public BoltServerAddress( URI uri )

public BoltServerAddress( String host, int port )
{
this( host, null, port );
this( host, port, Collections.emptySet() );
}

private BoltServerAddress( String host, InetAddress resolved, int port )
public BoltServerAddress( String host, int port, Set<BoltServerAddress> resolved )
{
this.host = requireNonNull( host, "host" );
this.resolved = resolved;
this.port = requireValidPort( port );
this.stringValue = resolved != null ? String.format( "%s(%s):%d", host, resolved.getHostAddress(), port ) : String.format( "%s:%d", host, port );
this.stringValue = String.format( "%s:%d", host, port );
this.resolved = Collections.unmodifiableSet( new LinkedHashSet<>( resolved ) );
}

public static BoltServerAddress from( ServerAddress address )
Expand Down Expand Up @@ -112,33 +111,7 @@ public String toString()
*/
public SocketAddress toSocketAddress()
{
return resolved == null ? new InetSocketAddress( host, port ) : new InetSocketAddress( resolved, port );
}

/**
* Resolve the host name down to an IP address
*
* @return a new address instance
* @throws UnknownHostException if no IP address for the host could be found
* @see InetAddress#getByName(String)
*/
public BoltServerAddress resolve() throws UnknownHostException
{
return new BoltServerAddress( host, InetAddress.getByName( host ), port );
}

/**
* Resolve the host name down to all IP addresses that can be resolved to
*
* @return an array of new address instances that holds resolved addresses
* @throws UnknownHostException if no IP address for the host could be found
* @see InetAddress#getAllByName(String)
*/
public List<BoltServerAddress> resolveAll() throws UnknownHostException
{
return Stream.of( InetAddress.getAllByName( host ) )
.map( address -> new BoltServerAddress( host, address, port ) )
.collect( toList() );
return new InetSocketAddress( host, port );
}

@Override
Expand All @@ -153,9 +126,9 @@ public int port()
return port;
}

public boolean isResolved()
public Set<BoltServerAddress> resolved()
{
return resolved != null;
return this.resolved;
}

private static String hostFrom( URI uri )
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* Copyright (c) "Neo4j"
* Neo4j Sweden AB [http://neo4j.com]
*
* This file is part of Neo4j.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.neo4j.driver.internal;

import java.net.InetAddress;
import java.net.UnknownHostException;

public class DefaultDomainNameResolver implements DomainNameResolver
{
private static final DefaultDomainNameResolver INSTANCE = new DefaultDomainNameResolver();

public static DefaultDomainNameResolver getInstance()
{
return INSTANCE;
}

private DefaultDomainNameResolver()
{
}

@Override
public InetAddress[] resolve( String name ) throws UnknownHostException
{
return InetAddress.getAllByName( name );
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Copyright (c) "Neo4j"
* Neo4j Sweden AB [http://neo4j.com]
*
* This file is part of Neo4j.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.neo4j.driver.internal;

import java.net.InetAddress;
import java.net.UnknownHostException;

/**
* A resolver function used by the driver to resolve domain names.
*/
@FunctionalInterface
public interface DomainNameResolver
{
/**
* Resolve the given domain name to a set of addresses.
*
* @param name the name to resolve.
* @return the resolved addresses.
* @throws UnknownHostException must be thrown if the given name can not be resolved to at least one address.
*/
InetAddress[] resolve( String name ) throws UnknownHostException;
}
15 changes: 13 additions & 2 deletions driver/src/main/java/org/neo4j/driver/internal/DriverFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ protected static MetricsProvider createDriverMetrics( Config config, Clock clock
protected ChannelConnector createConnector( ConnectionSettings settings, SecurityPlan securityPlan,
Config config, Clock clock, RoutingContext routingContext )
{
return new ChannelConnectorImpl( settings, securityPlan, config.logging(), clock, routingContext );
return new ChannelConnectorImpl( settings, securityPlan, config.logging(), clock, routingContext, getDomainNameResolver() );
}

private InternalDriver createDriver( URI uri, SecurityPlan securityPlan, BoltServerAddress address, ConnectionPool connectionPool,
Expand Down Expand Up @@ -210,7 +210,7 @@ protected LoadBalancer createLoadBalancer( BoltServerAddress address, Connection
LoadBalancingStrategy loadBalancingStrategy = new LeastConnectedLoadBalancingStrategy( connectionPool, config.logging() );
ServerAddressResolver resolver = createResolver( config );
return new LoadBalancer( address, routingSettings, connectionPool, eventExecutorGroup, createClock(),
config.logging(), loadBalancingStrategy, resolver );
config.logging(), loadBalancingStrategy, resolver, getDomainNameResolver() );
}

private static ServerAddressResolver createResolver( Config config )
Expand Down Expand Up @@ -271,6 +271,17 @@ protected Bootstrap createBootstrap( EventLoopGroup eventLoopGroup )
return BootstrapFactory.newBootstrap( eventLoopGroup );
}

/**
* Provides an instance of {@link DomainNameResolver} that is used for domain name resolution.
* <p>
* <b>This method is protected only for testing</b>
*
* @return the instance of {@link DomainNameResolver}.
*/
protected DomainNameResolver getDomainNameResolver()
{
return DefaultDomainNameResolver.getInstance();
}

private static void assertNoRoutingContext( URI uri, RoutingSettings routingSettings )
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,21 +24,22 @@
import io.netty.channel.ChannelOption;
import io.netty.channel.ChannelPipeline;
import io.netty.channel.ChannelPromise;
import io.netty.resolver.AddressResolverGroup;

import java.util.Map;
import java.net.InetSocketAddress;

import org.neo4j.driver.AuthToken;
import org.neo4j.driver.AuthTokens;
import org.neo4j.driver.Logging;
import org.neo4j.driver.exceptions.ClientException;
import org.neo4j.driver.internal.BoltServerAddress;
import org.neo4j.driver.internal.ConnectionSettings;
import org.neo4j.driver.internal.DomainNameResolver;
import org.neo4j.driver.internal.async.inbound.ConnectTimeoutHandler;
import org.neo4j.driver.internal.cluster.RoutingContext;
import org.neo4j.driver.internal.security.InternalAuthToken;
import org.neo4j.driver.internal.security.SecurityPlan;
import org.neo4j.driver.internal.util.Clock;
import org.neo4j.driver.AuthToken;
import org.neo4j.driver.AuthTokens;
import org.neo4j.driver.Logging;
import org.neo4j.driver.Value;
import org.neo4j.driver.exceptions.ClientException;

import static java.util.Objects.requireNonNull;

Expand All @@ -52,15 +53,17 @@ public class ChannelConnectorImpl implements ChannelConnector
private final int connectTimeoutMillis;
private final Logging logging;
private final Clock clock;
private final AddressResolverGroup<InetSocketAddress> addressResolverGroup;

public ChannelConnectorImpl( ConnectionSettings connectionSettings, SecurityPlan securityPlan, Logging logging,
Clock clock, RoutingContext routingContext )
Clock clock, RoutingContext routingContext, DomainNameResolver domainNameResolver )
{
this( connectionSettings, securityPlan, new ChannelPipelineBuilderImpl(), logging, clock, routingContext );
this( connectionSettings, securityPlan, new ChannelPipelineBuilderImpl(), logging, clock, routingContext, domainNameResolver );
}

public ChannelConnectorImpl( ConnectionSettings connectionSettings, SecurityPlan securityPlan,
ChannelPipelineBuilder pipelineBuilder, Logging logging, Clock clock, RoutingContext routingContext )
ChannelPipelineBuilder pipelineBuilder, Logging logging, Clock clock, RoutingContext routingContext,
DomainNameResolver domainNameResolver )
{
this.userAgent = connectionSettings.userAgent();
this.authToken = requireValidAuthToken( connectionSettings.authToken() );
Expand All @@ -70,13 +73,15 @@ public ChannelConnectorImpl( ConnectionSettings connectionSettings, SecurityPlan
this.pipelineBuilder = pipelineBuilder;
this.logging = requireNonNull( logging );
this.clock = requireNonNull( clock );
this.addressResolverGroup = new NettyDomainNameResolverGroup( requireNonNull( domainNameResolver ) );
}

@Override
public ChannelFuture connect( BoltServerAddress address, Bootstrap bootstrap )
{
bootstrap.option( ChannelOption.CONNECT_TIMEOUT_MILLIS, connectTimeoutMillis );
bootstrap.handler( new NettyChannelInitializer( address, securityPlan, connectTimeoutMillis, clock, logging ) );
bootstrap.resolver( addressResolverGroup );

ChannelFuture channelConnected = bootstrap.connect( address.toSocketAddress() );

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/*
* Copyright (c) "Neo4j"
* Neo4j Sweden AB [http://neo4j.com]
*
* This file is part of Neo4j.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.neo4j.driver.internal.async.connection;

import io.netty.resolver.InetNameResolver;
import io.netty.util.concurrent.EventExecutor;
import io.netty.util.concurrent.Promise;

import java.net.InetAddress;
import java.net.UnknownHostException;
import java.util.Arrays;
import java.util.List;

import org.neo4j.driver.internal.DomainNameResolver;

public class NettyDomainNameResolver extends InetNameResolver
{
private final DomainNameResolver domainNameResolver;

public NettyDomainNameResolver( EventExecutor executor, DomainNameResolver domainNameResolver )
{
super( executor );
this.domainNameResolver = domainNameResolver;
}

@Override
protected void doResolve( String inetHost, Promise<InetAddress> promise )
{
try
{
promise.setSuccess( domainNameResolver.resolve( inetHost )[0] );
}
catch ( UnknownHostException e )
{
promise.setFailure( e );
}
}

@Override
protected void doResolveAll( String inetHost, Promise<List<InetAddress>> promise )
{
try
{
promise.setSuccess( Arrays.asList( domainNameResolver.resolve( inetHost ) ) );
}
catch ( UnknownHostException e )
{
promise.setFailure( e );
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* Copyright (c) "Neo4j"
* Neo4j Sweden AB [http://neo4j.com]
*
* This file is part of Neo4j.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.neo4j.driver.internal.async.connection;

import io.netty.resolver.AddressResolver;
import io.netty.resolver.AddressResolverGroup;
import io.netty.util.concurrent.EventExecutor;

import java.net.InetSocketAddress;

import org.neo4j.driver.internal.DomainNameResolver;

public class NettyDomainNameResolverGroup extends AddressResolverGroup<InetSocketAddress>
{
private final DomainNameResolver domainNameResolver;

public NettyDomainNameResolverGroup( DomainNameResolver domainNameResolver )
{
this.domainNameResolver = domainNameResolver;
}

@Override
protected AddressResolver<InetSocketAddress> newResolver( EventExecutor executor ) throws Exception
{
return new NettyDomainNameResolver( executor, domainNameResolver ).asAddressResolver();
}
}
Loading

0 comments on commit 8049f7e

Please sign in to comment.