Skip to content

Resolve DNS to IP & IPv6 address support #332

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 22, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,7 @@ public synchronized Set<BoltServerAddress> update( ClusterComposition cluster )
@Override
public synchronized void forget( BoltServerAddress address )
{
// Don't remove it from the set of routers, since that might mean we lose our ability to re-discover,
// just remove it from the set of readers and writers, so that we don't use it for actual work without
// performing discovery first.
routers.remove( address );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This #forget() method is also called on connection errors. Do we really want to drop routers here? I'd vote for not changing this and keeping #removeRouter().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the point to keep in routing table? they were there to protect from running out of routers. Now we no longer have the problem with dns

readers.remove( address );
writers.remove( address );
}
Expand Down Expand Up @@ -115,11 +113,6 @@ public void removeWriter( BoltServerAddress toRemove )
writers.remove( toRemove );
}

@Override
public void removeRouter( BoltServerAddress toRemove )
{
routers.remove( toRemove );
}

@Override
public String toString()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* Copyright (c) 2002-2017 "Neo Technology,"
* Network Engine for Objects in Lund AB [http://neotechnology.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.cluster;

import java.net.InetAddress;
import java.net.UnknownHostException;
import java.util.HashSet;
import java.util.Set;

import org.neo4j.driver.internal.net.BoltServerAddress;
import org.neo4j.driver.v1.Logger;

public class DnsResolver implements HostNameResolver
{
private final Logger logger;

public DnsResolver( Logger logger )
{
this.logger = logger;
}

@Override
public Set<BoltServerAddress> resolve( BoltServerAddress initialRouter )
{
Set<BoltServerAddress> addresses = new HashSet<>();
try
{
InetAddress[] ipAddresses = InetAddress.getAllByName( initialRouter.host() );

for ( InetAddress ipAddress : ipAddresses )
{
addresses.add( new BoltServerAddress( ipAddress.getHostAddress(), initialRouter.port() ) );
}

return addresses;
}
catch ( UnknownHostException e )
{
logger.error( "Failed to resolve URI `" + initialRouter + "` to IPs due to error: " + e.getMessage(), e );

addresses.add( initialRouter );
return addresses;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Copyright (c) 2002-2017 "Neo Technology,"
* Network Engine for Objects in Lund AB [http://neotechnology.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.cluster;

import java.util.Set;

import org.neo4j.driver.internal.net.BoltServerAddress;

public interface HostNameResolver
{
Set<BoltServerAddress> resolve( BoltServerAddress initialRouter );
}
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,6 @@ private static Rediscovery createRediscovery( BoltServerAddress initialRouter, R
Clock clock, Logger log )
{
ClusterCompositionProvider clusterComposition = new GetServersProcedureClusterCompositionProvider( clock, log );
return new Rediscovery( initialRouter, settings, clock, log, clusterComposition );
return new Rediscovery( initialRouter, settings, clock, log, clusterComposition, new DnsResolver( log ) );
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
*/
package org.neo4j.driver.internal.cluster;

import java.util.HashSet;
import java.util.Set;

import org.neo4j.driver.internal.net.BoltServerAddress;
import org.neo4j.driver.internal.spi.Connection;
import org.neo4j.driver.internal.spi.ConnectionPool;
Expand All @@ -37,15 +40,17 @@ public class Rediscovery
private final Clock clock;
private final Logger logger;
private final ClusterCompositionProvider provider;
private final HostNameResolver hostNameResolver;

public Rediscovery( BoltServerAddress initialRouter, RoutingSettings settings, Clock clock, Logger logger,
ClusterCompositionProvider provider )
ClusterCompositionProvider provider, HostNameResolver hostNameResolver )
{
this.initialRouter = initialRouter;
this.settings = settings;
this.clock = clock;
this.logger = logger;
this.provider = provider;
this.hostNameResolver = hostNameResolver;
}

// Given the current routing table and connection pool, use the connection composition provider to fetch a new
Expand Down Expand Up @@ -76,8 +81,8 @@ public ClusterComposition lookupClusterComposition( ConnectionPool connections,
private ClusterComposition lookupClusterCompositionOnKnownRouters( ConnectionPool connections,
RoutingTable routingTable )
{
boolean triedInitialRouter = false;
int size = routingTable.routerSize();
Set<BoltServerAddress> triedServers = new HashSet<>();
for ( int i = 0; i < size; i++ )
{
BoltServerAddress address = routingTable.nextRouter();
Expand All @@ -86,23 +91,29 @@ private ClusterComposition lookupClusterCompositionOnKnownRouters( ConnectionPoo
break;
}

if ( address.equals( initialRouter ) )
ClusterComposition composition = lookupClusterCompositionOnRouter( address, connections, routingTable );
if ( composition != null )
{
return composition;
}
else
{
triedInitialRouter = true;
triedServers.add( address );
}
}

Set<BoltServerAddress> ips = hostNameResolver.resolve( initialRouter );
ips.removeAll( triedServers );
for ( BoltServerAddress address : ips )
{
ClusterComposition composition = lookupClusterCompositionOnRouter( address, connections, routingTable );
if ( composition != null )
{
return composition;
}
}

if ( triedInitialRouter )
{
return null;
}
return lookupClusterCompositionOnRouter( initialRouter, connections, routingTable );
return null;
}

private ClusterComposition lookupClusterCompositionOnRouter( BoltServerAddress routerAddress,
Expand All @@ -122,7 +133,7 @@ private ClusterComposition lookupClusterCompositionOnRouter( BoltServerAddress r
{
// connection turned out to be broken
logger.error( format( "Failed to connect to routing server '%s'.", routerAddress ), t );
routingTable.removeRouter( routerAddress );
routingTable.forget( routerAddress );
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,4 @@ public interface RoutingTable
int routerSize();

void removeWriter( BoltServerAddress toRemove );

void removeRouter( BoltServerAddress toRemove );
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ public static BoltServerAddress from( URI uri )
{
port = DEFAULT_PORT;
}

if( uri.getHost() == null )
{
throw new IllegalArgumentException( "Invalid URI format `" + uri.toString() + "`");
}

return new BoltServerAddress( uri.getHost(), port );
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import static org.hamcrest.Matchers.is;
import static org.hamcrest.core.IsEqual.equalTo;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;
import static org.neo4j.driver.internal.net.BoltServerAddress.LOCAL_DEFAULT;
import static org.neo4j.driver.internal.util.Matchers.directDriverWithAddress;
import static org.neo4j.driver.v1.Values.parameters;
Expand All @@ -52,6 +53,38 @@ public void shouldUseDefaultPortIfMissing()
assertThat( driver, is( directDriverWithAddress( LOCAL_DEFAULT ) ) );
}

@Test
public void shouldAllowIPv6Address()
{
// Given
URI uri = URI.create( "bolt://[::1]" );
BoltServerAddress address = BoltServerAddress.from( uri );

// When
Driver driver = GraphDatabase.driver( uri );

// Then
assertThat( driver, is( directDriverWithAddress( address ) ) );
}

@Test
public void shouldRejectInvalidAddress()
{
// Given
URI uri = URI.create( "*" );

// When & Then
try
{
Driver driver = GraphDatabase.driver( uri );
fail("Expecting error for wrong uri");
}
catch( IllegalArgumentException e )
{
assertThat( e.getMessage(), equalTo( "Invalid URI format `*`" ) );
}
}

@Test
public void shouldRegisterSingleServer()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/*
* Copyright (c) 2002-2017 "Neo Technology,"
* Network Engine for Objects in Lund AB [http://neotechnology.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.cluster;

import org.junit.Test;

import java.net.UnknownHostException;
import java.util.Set;

import org.neo4j.driver.internal.net.BoltServerAddress;
import org.neo4j.driver.v1.Logger;

import static org.hamcrest.Matchers.greaterThanOrEqualTo;
import static org.junit.Assert.assertThat;
import static org.mockito.Matchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;

public class DnsResolverTest
{
private DnsResolver resolver = new DnsResolver( mock( Logger.class ) );

@Test
public void shouldResolveDNSToIPs()
{
Set<BoltServerAddress> resolve = resolver.resolve( new BoltServerAddress( "google.com", 80 ) );
assertThat( resolve.size(), greaterThanOrEqualTo( 1 ) );
}

@Test
public void shouldResolveLocalhostIPDNSToIPs()
{
Set<BoltServerAddress> resolve = resolver.resolve( new BoltServerAddress( "127.0.0.1", 80 ) );
assertThat( resolve.size(), greaterThanOrEqualTo( 1 ) );
}

@Test
public void shouldResolveLocalhostDNSToIPs()
{
Set<BoltServerAddress> resolve = resolver.resolve( new BoltServerAddress( "localhost", 80 ) );
assertThat( resolve.size(), greaterThanOrEqualTo( 1 ) );
}

@Test
public void shouldResolveIPv6LocalhostDNSToIPs()
{
Set<BoltServerAddress> resolve = resolver.resolve( new BoltServerAddress( "[::1]", 80 ) );
assertThat( resolve.size(), greaterThanOrEqualTo( 1 ) );
}

@Test
public void shouldExceptionAndGiveDefaultValue()
{
Logger logger = mock( Logger.class );
DnsResolver resolver = new DnsResolver( logger );
Set<BoltServerAddress> resolve = resolver.resolve( new BoltServerAddress( "[/]", 80 ) );
verify( logger ).error( any( String.class ), any( UnknownHostException.class ) );
assertThat( resolve.size(), greaterThanOrEqualTo( 1 ) );
}
}
Loading