diff --git a/driver/src/main/java/org/neo4j/driver/internal/NetworkSession.java b/driver/src/main/java/org/neo4j/driver/internal/NetworkSession.java index 345bda1fe7..d4ad9da136 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/NetworkSession.java +++ b/driver/src/main/java/org/neo4j/driver/internal/NetworkSession.java @@ -34,6 +34,7 @@ import org.neo4j.driver.v1.Value; import org.neo4j.driver.v1.Values; import org.neo4j.driver.v1.exceptions.ClientException; +import org.neo4j.driver.v1.exceptions.ConnectionFailureException; import org.neo4j.driver.v1.types.TypeSystem; import static org.neo4j.driver.v1.Values.value; @@ -296,9 +297,9 @@ private void ensureConnectionIsOpen() { if ( !connection.isOpen() ) { - throw new ClientException( "The current session cannot be reused as the underlying connection with the " + - "server has been closed due to unrecoverable errors. " + - "Please close this session and retry your statement in another new session." ); + throw new ConnectionFailureException( "The current session cannot be reused as the underlying connection with the " + + "server has been closed due to unrecoverable errors. " + + "Please close this session and retry your statement in another new session." ); } } diff --git a/driver/src/main/java/org/neo4j/driver/internal/RoutingDriver.java b/driver/src/main/java/org/neo4j/driver/internal/RoutingDriver.java index 615d1d83bd..b3d710ce7b 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/RoutingDriver.java +++ b/driver/src/main/java/org/neo4j/driver/internal/RoutingDriver.java @@ -241,7 +241,7 @@ public BoltServerAddress apply( Value value ) //must be called from a synchronized method private boolean call( BoltServerAddress address, String procedureName, Consumer recorder ) { - Connection acquire = null; + Connection acquire; Session session = null; try { @@ -271,11 +271,6 @@ private boolean call( BoltServerAddress address, String procedureName, Consumer< { session.close(); } - if ( acquire != null ) - { - acquire.close(); - } - } return true; } diff --git a/driver/src/main/java/org/neo4j/driver/internal/net/pooling/PooledConnection.java b/driver/src/main/java/org/neo4j/driver/internal/net/pooling/PooledConnection.java index 2d43c8ac8e..4ef39b3a74 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/net/pooling/PooledConnection.java +++ b/driver/src/main/java/org/neo4j/driver/internal/net/pooling/PooledConnection.java @@ -67,7 +67,7 @@ public PooledConnection( Connection delegate, Consumer release this.lastUsed = clock.millis(); } - public void updateUsageTimestamp() + public void updateTimestamp() { lastUsed = clock.millis(); } diff --git a/driver/src/main/java/org/neo4j/driver/internal/net/pooling/SocketConnectionPool.java b/driver/src/main/java/org/neo4j/driver/internal/net/pooling/SocketConnectionPool.java index ceb62b6814..763b471f86 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/net/pooling/SocketConnectionPool.java +++ b/driver/src/main/java/org/neo4j/driver/internal/net/pooling/SocketConnectionPool.java @@ -18,6 +18,8 @@ */ package org.neo4j.driver.internal.net.pooling; +import java.util.ArrayList; +import java.util.List; import java.util.Map; import java.util.concurrent.BlockingQueue; import java.util.concurrent.ConcurrentHashMap; @@ -39,6 +41,8 @@ import org.neo4j.driver.v1.Value; import org.neo4j.driver.v1.exceptions.ClientException; +import static java.util.Collections.emptyList; + /** * The pool is designed to buffer certain amount of free sessions into session pool. When closing a session, we first * try to return the session into the session pool, however if we failed to return it back, either because the pool @@ -115,7 +119,7 @@ public Connection acquire( BoltServerAddress address ) conn = new PooledConnection( connect( address ), new PooledConnectionReleaseConsumer( connections, stopped, new PooledConnectionValidator( this, poolSettings ) ), clock ); } - conn.updateUsageTimestamp(); + conn.updateTimestamp(); return conn; } @@ -184,4 +188,19 @@ public void close() pools.clear(); } + //for testing + public List connectionsForAddress(BoltServerAddress address) + { + LinkedBlockingQueue pooledConnections = + (LinkedBlockingQueue) pools.get( address ); + if (pooledConnections == null) + { + return emptyList(); + } + else + { + return new ArrayList<>( pooledConnections ); + } + } + } diff --git a/driver/src/main/java/org/neo4j/driver/internal/spi/Collector.java b/driver/src/main/java/org/neo4j/driver/internal/spi/Collector.java index d6a5a20405..adf2829193 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/spi/Collector.java +++ b/driver/src/main/java/org/neo4j/driver/internal/spi/Collector.java @@ -41,13 +41,6 @@ public void doneFailure( Neo4jException error ) throw new ClientException( "Invalid server response message `FAILURE` received for client message `ACK_FAILURE`.", error ); } - - @Override - public void doneIgnored() - { - throw new ClientException( - "Invalid server response message `IGNORED` received for client message `ACK_FAILURE`." ); - } }; class InitCollector extends NoOperationCollector diff --git a/driver/src/test/java/org/neo4j/driver/internal/NetworkSessionTest.java b/driver/src/test/java/org/neo4j/driver/internal/NetworkSessionTest.java index 74e04533fb..882fcd1ea4 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/NetworkSessionTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/NetworkSessionTest.java @@ -27,6 +27,7 @@ import org.neo4j.driver.v1.Logger; import org.neo4j.driver.v1.Transaction; import org.neo4j.driver.v1.exceptions.ClientException; +import org.neo4j.driver.v1.exceptions.ConnectionFailureException; import static junit.framework.Assert.fail; import static junit.framework.TestCase.assertNotNull; @@ -121,7 +122,7 @@ public void shouldNotAllowMoreStatementsInSessionWhileConnectionClosed() throws when( mock.isOpen() ).thenReturn( false ); // Expect - exception.expect( ClientException.class ); + exception.expect( ConnectionFailureException.class ); // When sess.run( "whatever" ); @@ -134,7 +135,7 @@ public void shouldNotAllowMoreTransactionsInSessionWhileConnectionClosed() throw when( mock.isOpen() ).thenReturn( false ); // Expect - exception.expect( ClientException.class ); + exception.expect( ConnectionFailureException.class ); // When sess.beginTransaction(); diff --git a/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverStubTest.java b/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverStubTest.java index 484d4a981e..773104c9fe 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverStubTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverStubTest.java @@ -35,6 +35,8 @@ import org.neo4j.driver.internal.logging.ConsoleLogging; import org.neo4j.driver.internal.net.BoltServerAddress; +import org.neo4j.driver.internal.net.pooling.PooledConnection; +import org.neo4j.driver.internal.net.pooling.SocketConnectionPool; import org.neo4j.driver.internal.spi.Connection; import org.neo4j.driver.v1.AccessMode; import org.neo4j.driver.v1.Config; @@ -86,6 +88,26 @@ public void shouldDiscoverServers() throws IOException, InterruptedException, St assertThat( server.exitStatus(), equalTo( 0 ) ); } + @Test + public void shouldOnlyPutConnectionInPoolOnce() throws IOException, InterruptedException, StubServer.ForceKilled + { + // Given + StubServer server = StubServer.start( "discover_servers.script", 9001 ); + URI uri = URI.create( "bolt+routing://127.0.0.1:9001" ); + + // When + try ( RoutingDriver driver = (RoutingDriver) GraphDatabase.driver( uri, config ) ) + { + // Then + SocketConnectionPool pool = (SocketConnectionPool) driver.connectionPool(); + List pooledConnections = pool.connectionsForAddress( address( 9001 ) ); + assertThat(pooledConnections, hasSize( 1 )); + } + + // Finally + assertThat( server.exitStatus(), equalTo( 0 ) ); + } + @Test public void shouldDiscoverNewServers() throws IOException, InterruptedException, StubServer.ForceKilled { diff --git a/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverTest.java b/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverTest.java index f4531eead4..13063827eb 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverTest.java @@ -43,7 +43,6 @@ import org.neo4j.driver.v1.exceptions.NoSuchRecordException; import org.neo4j.driver.v1.exceptions.ServiceUnavailableException; import org.neo4j.driver.v1.summary.ResultSummary; -import org.neo4j.driver.v1.util.BiFunction; import org.neo4j.driver.v1.util.Function; import static java.util.Arrays.asList; diff --git a/driver/src/test/java/org/neo4j/driver/internal/net/pooling/PooledConnectionTest.java b/driver/src/test/java/org/neo4j/driver/internal/net/pooling/PooledConnectionTest.java index be4bdd40b8..022dc0cf76 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/net/pooling/PooledConnectionTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/net/pooling/PooledConnectionTest.java @@ -120,6 +120,7 @@ public void dispose() assertThat( flags[0], equalTo( false ) ); } + @Test public void shouldDisposeConnectionIfValidConnectionAndIdlePoolIsFull() throws Throwable {