Skip to content

Commit de7dab0

Browse files
zhenlineozhenlineo
authored
Merge pull request #609 from zhenlineo/2.0-verify-conn
Make connectivity verification not part of driver creation.
2 parents 39d00db + df314fe commit de7dab0

28 files changed

+259
-158
lines changed

‎driver/src/main/java/org/neo4j/driver/Config.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,9 @@ RetrySettings retrySettings()
285285
return retrySettings;
286286
}
287287

288+
/**
289+
* @return if the metrics is enabled or not on this driver.
290+
*/
288291
public boolean isMetricsEnabled()
289292
{
290293
return isMetricsEnabled;

‎driver/src/main/java/org/neo4j/driver/Driver.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,4 +164,27 @@ public interface Driver extends AutoCloseable
164164
*/
165165
@Experimental
166166
TypeSystem defaultTypeSystem();
167+
168+
/**
169+
* This verifies if the driver can connect to a remote server or a cluster
170+
* by establishing a network connection with the remote and possibly exchanging a few data before closing the connection.
171+
*
172+
* It throws exception if fails to connect. Use the exception to further understand the cause of the connectivity problem.
173+
* Note: Even if this method throws an exception, the driver still need to be closed via {@link #close()} to free up all resources.
174+
*/
175+
void verifyConnectivity();
176+
177+
/**
178+
* This verifies if the driver can connect to a remote server or cluster
179+
* by establishing a network connection with the remote and possibly exchanging a few data before closing the connection.
180+
*
181+
* This operation is asynchronous and returns a {@link CompletionStage}. This stage is completed with
182+
* {@code null} when the driver connects to the remote server or cluster successfully.
183+
* It is completed exceptionally if the driver failed to connect the remote server or cluster.
184+
* This exception can be used to further understand the cause of the connectivity problem.
185+
* Note: Even if this method complete exceptionally, the driver still need to be closed via {@link #closeAsync()} to free up all resources.
186+
*
187+
* @return a {@link CompletionStage completion stage} that represents the asynchronous verification.
188+
*/
189+
CompletionStage<Void> verifyConnectivityAsync();
167190
}

‎driver/src/main/java/org/neo4j/driver/GraphDatabase.java

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,19 +154,40 @@ public static Driver routingDriver( Iterable<URI> routingUris, AuthToken authTok
154154

155155
for ( URI uri : routingUris )
156156
{
157+
final Driver driver = driver( uri, authToken, config );
157158
try
158159
{
159-
return driver( uri, authToken, config );
160+
driver.verifyConnectivity();
161+
return driver;
160162
}
161163
catch ( ServiceUnavailableException e )
162164
{
163165
log.warn( "Unable to create routing driver for URI: " + uri, e );
166+
closeDriver( driver, uri, log );
167+
}
168+
catch ( Throwable e )
169+
{
170+
// for any other errors, we first close the driver and then rethrow the original error out.
171+
closeDriver( driver, uri, log );
172+
throw e;
164173
}
165174
}
166175

167176
throw new ServiceUnavailableException( "Failed to discover an available server" );
168177
}
169178

179+
private static void closeDriver( Driver driver, URI uri, Logger log )
180+
{
181+
try
182+
{
183+
driver.close();
184+
}
185+
catch ( Throwable closeError )
186+
{
187+
log.warn( "Unable to close driver towards URI: " + uri, closeError );
188+
}
189+
}
190+
170191
private static void assertRoutingUris( Iterable<URI> uris )
171192
{
172193
for ( URI uri : uris )

‎driver/src/main/java/org/neo4j/driver/internal/DriverFactory.java

Lines changed: 9 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,13 @@
2727
import java.net.URI;
2828
import java.security.GeneralSecurityException;
2929

30+
import org.neo4j.driver.AuthToken;
31+
import org.neo4j.driver.AuthTokens;
32+
import org.neo4j.driver.Config;
33+
import org.neo4j.driver.Driver;
34+
import org.neo4j.driver.Logger;
35+
import org.neo4j.driver.Logging;
36+
import org.neo4j.driver.exceptions.ClientException;
3037
import org.neo4j.driver.internal.async.connection.BootstrapFactory;
3138
import org.neo4j.driver.internal.async.connection.ChannelConnector;
3239
import org.neo4j.driver.internal.async.connection.ChannelConnectorImpl;
@@ -49,19 +56,11 @@
4956
import org.neo4j.driver.internal.spi.ConnectionProvider;
5057
import org.neo4j.driver.internal.util.Clock;
5158
import org.neo4j.driver.internal.util.Futures;
52-
import org.neo4j.driver.AuthToken;
53-
import org.neo4j.driver.AuthTokens;
54-
import org.neo4j.driver.Config;
55-
import org.neo4j.driver.Driver;
56-
import org.neo4j.driver.Logger;
57-
import org.neo4j.driver.Logging;
58-
import org.neo4j.driver.exceptions.ClientException;
59-
import org.neo4j.driver.exceptions.ServiceUnavailableException;
6059
import org.neo4j.driver.net.ServerAddressResolver;
6160

6261
import static java.lang.String.format;
63-
import static org.neo4j.driver.internal.metrics.MetricsProvider.METRICS_DISABLED_PROVIDER;
6462
import static org.neo4j.driver.internal.cluster.IdentityResolver.IDENTITY_RESOLVER;
63+
import static org.neo4j.driver.internal.metrics.MetricsProvider.METRICS_DISABLED_PROVIDER;
6564
import static org.neo4j.driver.internal.security.SecurityPlan.insecure;
6665
import static org.neo4j.driver.internal.util.ErrorUtil.addSuppressed;
6766

@@ -105,11 +104,7 @@ public final Driver newInstance ( URI uri, AuthToken authToken, RoutingSettings
105104
MetricsProvider metricsProvider = createDriverMetrics( config, createClock() );
106105
ConnectionPool connectionPool = createConnectionPool( authToken, securityPlan, bootstrap, metricsProvider, config, ownsEventLoopGroup );
107106

108-
InternalDriver driver = createDriver( uri, securityPlan, address, connectionPool, eventExecutorGroup, newRoutingSettings, retryLogic, metricsProvider, config );
109-
110-
verifyConnectivity( driver, connectionPool, config );
111-
112-
return driver;
107+
return createDriver( uri, securityPlan, address, connectionPool, eventExecutorGroup, newRoutingSettings, retryLogic, metricsProvider, config );
113108
}
114109

115110
protected ConnectionPool createConnectionPool( AuthToken authToken, SecurityPlan securityPlan, Bootstrap bootstrap,
@@ -366,30 +361,6 @@ private static void assertNoRoutingContext( URI uri, RoutingSettings routingSett
366361
}
367362
}
368363

369-
private static void verifyConnectivity( InternalDriver driver, ConnectionPool connectionPool, Config config )
370-
{
371-
try
372-
{
373-
// block to verify connectivity, close connection pool if thread gets interrupted
374-
Futures.blockingGet( driver.verifyConnectivity(),
375-
() -> closeConnectionPoolOnThreadInterrupt( connectionPool, config.logging() ) );
376-
}
377-
catch ( Throwable connectionError )
378-
{
379-
if ( Thread.currentThread().isInterrupted() )
380-
{
381-
// current thread has been interrupted while verifying connectivity
382-
// connection pool should've been closed
383-
throw new ServiceUnavailableException( "Unable to create driver. Thread has been interrupted.",
384-
connectionError );
385-
}
386-
387-
// we need to close the connection pool if driver creation threw exception
388-
closeConnectionPoolAndSuppressError( connectionPool, connectionError );
389-
throw connectionError;
390-
}
391-
}
392-
393364
private static void closeConnectionPoolAndSuppressError( ConnectionPool connectionPool, Throwable mainError )
394365
{
395366
try
@@ -401,11 +372,4 @@ private static void closeConnectionPoolAndSuppressError( ConnectionPool connecti
401372
addSuppressed( mainError, closeError );
402373
}
403374
}
404-
405-
private static void closeConnectionPoolOnThreadInterrupt( ConnectionPool pool, Logging logging )
406-
{
407-
Logger log = logging.getLog( Driver.class.getSimpleName() );
408-
log.warn( "Driver creation interrupted while verifying connectivity. Connection pool will be closed" );
409-
pool.close();
410-
}
411375
}

‎driver/src/main/java/org/neo4j/driver/internal/InternalDriver.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,11 +128,18 @@ public final TypeSystem defaultTypeSystem()
128128
return InternalTypeSystem.TYPE_SYSTEM;
129129
}
130130

131-
public CompletionStage<Void> verifyConnectivity()
131+
@Override
132+
public CompletionStage<Void> verifyConnectivityAsync()
132133
{
133134
return sessionFactory.verifyConnectivity();
134135
}
135136

137+
@Override
138+
public void verifyConnectivity()
139+
{
140+
Futures.blockingGet( verifyConnectivityAsync() );
141+
}
142+
136143
/**
137144
* Get the underlying session factory.
138145
* <p>

‎driver/src/main/java/org/neo4j/driver/internal/cluster/RediscoveryImpl.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@
5353
*/
5454
public class RediscoveryImpl implements Rediscovery
5555
{
56-
private static final String NO_ROUTERS_AVAILABLE = "Could not perform discovery for database '%s'. No routing servers available.";
56+
private static final String NO_ROUTERS_AVAILABLE = "Could not perform discovery for database '%s'. No routing server available.";
5757

5858
private final BoltServerAddress initialRouter;
5959
private final RoutingSettings settings;

‎driver/src/main/java/org/neo4j/driver/internal/cluster/loadbalancing/LoadBalancer.java

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,19 @@ public CompletionStage<Connection> acquireConnection( String databaseName, Acces
8787
@Override
8888
public CompletionStage<Void> verifyConnectivity()
8989
{
90-
return routingTables.refreshRoutingTable( ABSENT_DB_NAME, AccessMode.READ ).thenApply( ignored -> null );
90+
return routingTables.refreshRoutingTable( ABSENT_DB_NAME, AccessMode.READ ).handle( ( ignored, error ) -> {
91+
if ( error != null )
92+
{
93+
Throwable cause = Futures.completionExceptionCause( error );
94+
if ( cause instanceof ServiceUnavailableException )
95+
{
96+
throw Futures.asCompletionException( new ServiceUnavailableException(
97+
"Unable to connect to database, ensure the database is running and that there is a working network connection to it.", cause ) );
98+
}
99+
throw Futures.asCompletionException( cause );
100+
}
101+
return null;
102+
} );
91103
}
92104

93105
@Override

‎driver/src/test/java/org/neo4j/driver/GraphDatabaseTest.java

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232

3333
import static java.util.Arrays.asList;
3434
import static java.util.concurrent.TimeUnit.MILLISECONDS;
35+
import static org.hamcrest.Matchers.containsString;
3536
import static org.hamcrest.Matchers.is;
3637
import static org.hamcrest.core.IsEqual.equalTo;
3738
import static org.hamcrest.junit.MatcherAssert.assertThat;
@@ -60,6 +61,7 @@ void boltSchemeShouldInstantiateDirectDriver() throws Exception
6061

6162
// When
6263
Driver driver = GraphDatabase.driver( uri, INSECURE_CONFIG );
64+
driver.verifyConnectivity();
6365

6466
// Then
6567
assertThat( driver, is( directDriver() ) );
@@ -78,6 +80,7 @@ void boltPlusDiscoverySchemeShouldInstantiateClusterDriver() throws Exception
7880

7981
// When
8082
Driver driver = GraphDatabase.driver( uri, INSECURE_CONFIG );
83+
driver.verifyConnectivity();
8184

8285
// Then
8386
assertThat( driver, is( clusterDriver() ) );
@@ -146,9 +149,10 @@ void shouldRespondToInterruptsWhenConnectingToUnresponsiveServer() throws Except
146149
// setup other thread to interrupt current thread when it blocks
147150
TestUtil.interruptWhenInWaitingState( Thread.currentThread() );
148151

152+
final Driver driver = GraphDatabase.driver( "bolt://localhost:" + serverSocket.getLocalPort() );
149153
try
150154
{
151-
assertThrows( ServiceUnavailableException.class, () -> GraphDatabase.driver( "bolt://localhost:" + serverSocket.getLocalPort() ) );
155+
assertThrows( ServiceUnavailableException.class, driver::verifyConnectivity );
152156
}
153157
finally
154158
{
@@ -158,6 +162,33 @@ void shouldRespondToInterruptsWhenConnectingToUnresponsiveServer() throws Except
158162
}
159163
}
160164

165+
@Test
166+
void shouldPrintNiceErrorWhenConnectingToUnresponsiveServer() throws Exception
167+
{
168+
int localPort = -1;
169+
try ( ServerSocket serverSocket = new ServerSocket( 0 ) )
170+
{
171+
localPort = serverSocket.getLocalPort();
172+
}
173+
final Driver driver = GraphDatabase.driver( "bolt://localhost:" + localPort, INSECURE_CONFIG );
174+
final ServiceUnavailableException error = assertThrows( ServiceUnavailableException.class, driver::verifyConnectivity );
175+
assertThat( error.getMessage(), containsString( "Unable to connect to" ) );
176+
}
177+
178+
@Test
179+
void shouldPrintNiceRoutingErrorWhenConnectingToUnresponsiveServer() throws Exception
180+
{
181+
int localPort = -1;
182+
try ( ServerSocket serverSocket = new ServerSocket( 0 ) )
183+
{
184+
localPort = serverSocket.getLocalPort();
185+
}
186+
final Driver driver = GraphDatabase.driver( "neo4j://localhost:" + localPort, INSECURE_CONFIG );
187+
final ServiceUnavailableException error = assertThrows( ServiceUnavailableException.class, driver::verifyConnectivity );
188+
error.printStackTrace();
189+
assertThat( error.getMessage(), containsString( "Unable to connect to" ) );
190+
}
191+
161192
@Test
162193
void shouldFailToCreateUnencryptedDriverWhenServerDoesNotRespond() throws IOException
163194
{
@@ -176,9 +207,9 @@ private static void testFailureWhenServerDoesNotRespond( boolean encrypted ) thr
176207
{
177208
int connectionTimeoutMillis = 1_000;
178209
Config config = createConfig( encrypted, connectionTimeoutMillis );
210+
final Driver driver = GraphDatabase.driver( URI.create( "bolt://localhost:" + server.getLocalPort() ), config );
179211

180-
ServiceUnavailableException e = assertThrows( ServiceUnavailableException.class,
181-
() -> GraphDatabase.driver( URI.create( "bolt://localhost:" + server.getLocalPort() ), config ) );
212+
ServiceUnavailableException e = assertThrows( ServiceUnavailableException.class, driver::verifyConnectivity );
182213
assertEquals( e.getMessage(), "Unable to establish connection in " + connectionTimeoutMillis + "ms" );
183214
}
184215
}

‎driver/src/test/java/org/neo4j/driver/integration/CredentialsIT.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,11 @@ void shouldBePossibleToChangePassword() throws Exception
8181
}
8282

8383
// verify old password does not work
84-
assertThrows( AuthenticationException.class,
85-
() -> GraphDatabase.driver( neo4j.uri(), AuthTokens.basic( "neo4j", PASSWORD ) ) );
84+
final Driver badDriver = GraphDatabase.driver( CredentialsIT.neo4j.uri(), basic( "neo4j", PASSWORD ) );
85+
assertThrows( AuthenticationException.class, badDriver::verifyConnectivity );
8686

8787
// verify new password works
88-
try ( Driver driver = GraphDatabase.driver( neo4j.uri(), AuthTokens.basic( "neo4j", newPassword ) );
88+
try ( Driver driver = GraphDatabase.driver( CredentialsIT.neo4j.uri(), AuthTokens.basic( "neo4j", newPassword ) );
8989
Session session = driver.session() )
9090
{
9191
session.run( "RETURN 2" ).consume();
@@ -177,6 +177,7 @@ private void testDriverFailureOnWrongCredentials( String uri )
177177
Config config = Config.builder().withLogging( DEV_NULL_LOGGING ).build();
178178
AuthToken authToken = AuthTokens.basic( "neo4j", "wrongSecret" );
179179

180-
assertThrows( AuthenticationException.class, () -> GraphDatabase.driver( uri, authToken, config ) );
180+
final Driver driver = GraphDatabase.driver( uri, authToken, config );
181+
assertThrows( AuthenticationException.class, driver::verifyConnectivity );
181182
}
182183
}

‎driver/src/test/java/org/neo4j/driver/integration/EncryptionIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ private void testMismatchingEncryption( BoltTlsLevel tlsLevel, boolean driverEnc
119119
Config config = newConfig( driverEncrypted );
120120

121121
RuntimeException e = assertThrows( RuntimeException.class,
122-
() -> GraphDatabase.driver( neo4j.uri(), neo4j.authToken(), config ).close() );
122+
() -> GraphDatabase.driver( neo4j.uri(), neo4j.authToken(), config ).verifyConnectivity() );
123123

124124
// pre 3.1 neo4j throws different exception when encryption required but not used
125125
if ( neo4jVersion.lessThan( v3_1_0 ) && tlsLevel == BoltTlsLevel.REQUIRED )

0 commit comments

Comments
 (0)