diff --git a/driver/src/main/java/org/neo4j/driver/internal/cluster/Rediscovery.java b/driver/src/main/java/org/neo4j/driver/internal/cluster/Rediscovery.java index 56db7d6a88..1828dd9710 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/cluster/Rediscovery.java +++ b/driver/src/main/java/org/neo4j/driver/internal/cluster/Rediscovery.java @@ -39,10 +39,10 @@ import static java.lang.String.format; import static java.util.Collections.emptySet; -import static java.util.Collections.singletonList; import static java.util.concurrent.CompletableFuture.completedFuture; import static java.util.stream.Collectors.toList; import static org.neo4j.driver.internal.util.Futures.completedWithNull; +import static org.neo4j.driver.internal.util.Futures.failedFuture; public class Rediscovery { @@ -204,7 +204,15 @@ private CompletionStage lookupOnKnownRouters( RoutingTable r private CompletionStage lookupOnInitialRouter( RoutingTable routingTable, ConnectionPool connectionPool, Set seenServers ) { - List addresses = resolve( initialRouter ); + List addresses; + try + { + addresses = resolve( initialRouter ); + } + catch ( Throwable error ) + { + return failedFuture( error ); + } addresses.removeAll( seenServers ); CompletableFuture result = completedWithNull(); @@ -260,17 +268,9 @@ private ClusterComposition handleRoutingProcedureError( Throwable error, Routing private List resolve( BoltServerAddress address ) { - try - { - return resolver.resolve( address ) - .stream() - .map( BoltServerAddress::from ) - .collect( toList() ); // collect to list to preserve the order - } - catch ( Throwable error ) - { - logger.error( "Resolver function failed to resolve '" + address + "'. The address will be used as is", error ); - return singletonList( address ); - } + return resolver.resolve( address ) + .stream() + .map( BoltServerAddress::from ) + .collect( toList() ); // collect to list to preserve the order } } diff --git a/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverBoltKitTest.java b/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverBoltKitTest.java index c0a1df6371..25cbd73e5e 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverBoltKitTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverBoltKitTest.java @@ -23,8 +23,9 @@ import java.io.IOException; import java.net.URI; import java.util.ArrayList; -import java.util.Arrays; +import java.util.HashSet; import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import org.neo4j.driver.internal.cluster.RoutingSettings; @@ -45,10 +46,12 @@ import org.neo4j.driver.v1.TransactionWork; import org.neo4j.driver.v1.exceptions.ServiceUnavailableException; import org.neo4j.driver.v1.exceptions.SessionExpiredException; +import org.neo4j.driver.v1.net.ServerAddress; +import org.neo4j.driver.v1.net.ServerAddressResolver; import org.neo4j.driver.v1.util.StubServer; import static java.util.Arrays.asList; -import static java.util.logging.Level.INFO; +import static java.util.Collections.singleton; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.core.IsEqual.equalTo; import static org.hamcrest.junit.MatcherAssert.assertThat; @@ -57,13 +60,18 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.neo4j.driver.v1.Logging.console; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.neo4j.driver.v1.Logging.none; class RoutingDriverBoltKitTest { private static final Config config = Config.build() .withoutEncryption() - .withLogging( console( INFO ) ).toConfig(); + .withLogging( none() ) + .toConfig(); @Test void shouldHandleAcquireReadSession() throws IOException, InterruptedException, StubServer.ForceKilled @@ -917,7 +925,7 @@ void shouldSendMultipleBookmarks() throws Exception StubServer router = StubServer.start( "acquire_endpoints.script", 9001 ); StubServer writer = StubServer.start( "multiple_bookmarks.script", 9007 ); - List bookmarks = Arrays.asList( "neo4j:bookmark:v1:tx5", "neo4j:bookmark:v1:tx29", + List bookmarks = asList( "neo4j:bookmark:v1:tx5", "neo4j:bookmark:v1:tx29", "neo4j:bookmark:v1:tx94", "neo4j:bookmark:v1:tx56", "neo4j:bookmark:v1:tx16", "neo4j:bookmark:v1:tx68" ); @@ -963,10 +971,82 @@ void shouldForgetAddressOnDatabaseUnavailableError() throws Exception } finally { - assertEquals( router1.exitStatus(), 0 ); - assertEquals( writer1.exitStatus(), 0 ); - assertEquals( router2.exitStatus(), 0 ); - assertEquals( writer2.exitStatus(), 0 ); + assertEquals( 0, router1.exitStatus() ); + assertEquals( 0, writer1.exitStatus() ); + assertEquals( 0, router2.exitStatus() ); + assertEquals( 0, writer2.exitStatus() ); + } + } + + @Test + void shouldFailInitialDiscoveryWhenConfiguredResolverThrows() + { + ServerAddressResolver resolver = mock( ServerAddressResolver.class ); + when( resolver.resolve( any( ServerAddress.class ) ) ).thenThrow( new RuntimeException( "Resolution failure!" ) ); + + Config config = Config.build() + .withLogging( none() ) + .withoutEncryption() + .withResolver( resolver ) + .toConfig(); + + RuntimeException error = assertThrows( RuntimeException.class, () -> GraphDatabase.driver( "bolt+routing://my.server.com:9001", config ) ); + assertEquals( "Resolution failure!", error.getMessage() ); + verify( resolver ).resolve( ServerAddress.of( "my.server.com", 9001 ) ); + } + + @Test + void shouldUseResolverDuringRediscoveryWhenExistingRoutersFail() throws Exception + { + StubServer router1 = StubServer.start( "get_routing_table.script", 9001 ); + StubServer router2 = StubServer.start( "acquire_endpoints.script", 9042 ); + StubServer reader = StubServer.start( "read_server.script", 9005 ); + + AtomicBoolean resolverInvoked = new AtomicBoolean(); + ServerAddressResolver resolver = address -> + { + if ( resolverInvoked.compareAndSet( false, true ) ) + { + // return the address first time + return singleton( address ); + } + if ( "127.0.0.1".equals( address.host() ) && address.port() == 9001 ) + { + // return list of addresses where onl 9042 is functional + return new HashSet<>( asList( + ServerAddress.of( "127.0.0.1", 9010 ), + ServerAddress.of( "127.0.0.1", 9011 ), + ServerAddress.of( "127.0.0.1", 9042 ) ) ); + } + throw new AssertionError(); + }; + + Config config = Config.build() + .withLogging( none() ) + .withoutEncryption() + .withResolver( resolver ) + .toConfig(); + + try ( Driver driver = GraphDatabase.driver( "bolt+routing://127.0.0.1:9001", config ) ) + { + try ( Session session = driver.session( AccessMode.READ ) ) + { + // run first query against 9001, which should return result and exit + List names1 = session.run( "MATCH (n) RETURN n.name AS name" ) + .list( record -> record.get( "name" ).asString() ); + assertEquals( asList( "Alice", "Bob", "Eve" ), names1 ); + + // run second query with retries, it should rediscover using 9042 returned by the resolver and read from 9005 + List names2 = session.readTransaction( tx -> tx.run( "MATCH (n) RETURN n.name" ) ) + .list( record -> record.get( 0 ).asString() ); + assertEquals( asList( "Bob", "Alice", "Tina" ), names2 ); + } + } + finally + { + assertEquals( 0, router1.exitStatus() ); + assertEquals( 0, router2.exitStatus() ); + assertEquals( 0, reader.exitStatus() ); } } diff --git a/driver/src/test/java/org/neo4j/driver/internal/cluster/RediscoveryTest.java b/driver/src/test/java/org/neo4j/driver/internal/cluster/RediscoveryTest.java index 95aee9a3d4..b5b3b16f07 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/cluster/RediscoveryTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/cluster/RediscoveryTest.java @@ -227,7 +227,7 @@ void shouldResolveInitialRouterAddressUsingCustomResolver() } @Test - void shouldUseInitialRouterAddressAsIsWhenResolverFails() + void shouldPropagateFailureWhenResolverFails() { ClusterComposition expectedComposition = new ClusterComposition( 42, asOrderedSet( A, B ), asOrderedSet( A, B ), asOrderedSet( A, B ) ); @@ -242,9 +242,9 @@ void shouldUseInitialRouterAddressAsIsWhenResolverFails() Rediscovery rediscovery = newRediscovery( A, compositionProvider, resolver ); RoutingTable table = routingTableMock(); - ClusterComposition actualComposition = await( rediscovery.lookupClusterComposition( table, pool ) ); + RuntimeException error = assertThrows( RuntimeException.class, () -> await( rediscovery.lookupClusterComposition( table, pool ) ) ); + assertEquals( "Resolver fails!", error.getMessage() ); - assertEquals( expectedComposition, actualComposition ); verify( resolver ).resolve( A ); verify( table, never() ).forget( any() ); } diff --git a/driver/src/test/resources/get_routing_table.script b/driver/src/test/resources/get_routing_table.script index 2acb2013ab..b19fc76e7d 100644 --- a/driver/src/test/resources/get_routing_table.script +++ b/driver/src/test/resources/get_routing_table.script @@ -15,3 +15,4 @@ S: SUCCESS {"fields": ["name"]} RECORD ["Bob"] RECORD ["Eve"] SUCCESS {} +S: