Skip to content

Commit 714b5a2

Browse files
author
Zhen Li
authored
Merge pull request #282 from neo4j/1.1-remove-ping
Removed ping
2 parents e57e38d + 6cd6a84 commit 714b5a2

19 files changed

+163
-119
lines changed

‎driver/src/main/java/org/neo4j/driver/internal/net/SocketConnection.java‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ public class SocketConnection implements Connection
5050
{
5151
private final Queue<Message> pendingMessages = new LinkedList<>();
5252
private final SocketResponseHandler responseHandler;
53-
private AtomicBoolean isInterrupted = new AtomicBoolean( false );
54-
private AtomicBoolean isAckFailureMuted = new AtomicBoolean( false );
53+
private final AtomicBoolean isInterrupted = new AtomicBoolean( false );
54+
private final AtomicBoolean isAckFailureMuted = new AtomicBoolean( false );
5555
private InternalServerInfo serverInfo;
5656

5757
private final SocketClient socket;
@@ -66,10 +66,19 @@ public SocketConnection( BoltServerAddress address, SecurityPlan securityPlan, L
6666
this.socket.start();
6767
}
6868

69-
// for mocked socket testing
70-
SocketConnection( SocketClient socket, Logger logger )
69+
/**
70+
* Create new connection backed by the given socket.
71+
* <p>
72+
* <b>Note:</b> this constructor should be used <b>only</b> for testing.
73+
*
74+
* @param socket the socket to use for network interactions.
75+
* @param serverInfo the info about server this connection points to.
76+
* @param logger the logger.
77+
*/
78+
public SocketConnection( SocketClient socket, InternalServerInfo serverInfo, Logger logger )
7179
{
7280
this.socket = socket;
81+
this.serverInfo = serverInfo;
7382
this.logger = logger;
7483
this.responseHandler = createResponseHandler( logger );
7584
this.socket.start();

‎driver/src/main/java/org/neo4j/driver/internal/net/pooling/PoolSettings.java‎

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,38 +21,20 @@
2121

2222
public class PoolSettings
2323
{
24-
public static PoolSettings defaultSettings()
25-
{
26-
return new PoolSettings( DEFAULT_MAX_IDLE_CONNECTION_POOL_SIZE, DEFAULT_IDLE_TIME_BEFORE_CONNECTION_TEST );
27-
}
28-
2924
public static final int DEFAULT_MAX_IDLE_CONNECTION_POOL_SIZE = 10;
30-
public static final long DEFAULT_IDLE_TIME_BEFORE_CONNECTION_TEST = 200;
3125

3226
/**
3327
* Maximum number of idle connections per pool.
3428
*/
3529
private final int maxIdleConnectionPoolSize;
3630

37-
/**
38-
* Connections that have been idle longer than this threshold will have a ping test performed on them.
39-
*/
40-
private final long idleTimeBeforeConnectionTest;
41-
42-
public PoolSettings( int maxIdleConnectionPoolSize, long idleTimeBeforeConnectionTest )
31+
public PoolSettings( int maxIdleConnectionPoolSize )
4332
{
4433
this.maxIdleConnectionPoolSize = maxIdleConnectionPoolSize;
45-
this.idleTimeBeforeConnectionTest = idleTimeBeforeConnectionTest;
4634
}
4735

4836
public int maxIdleConnectionPoolSize()
4937
{
5038
return maxIdleConnectionPoolSize;
5139
}
52-
53-
public long idleTimeBeforeConnectionTest()
54-
{
55-
return idleTimeBeforeConnectionTest;
56-
}
57-
5840
}

‎driver/src/main/java/org/neo4j/driver/internal/net/pooling/PooledConnectionValidator.java‎

Lines changed: 3 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -18,24 +18,16 @@
1818
*/
1919
package org.neo4j.driver.internal.net.pooling;
2020

21-
import java.util.HashMap;
22-
import java.util.Map;
23-
24-
import org.neo4j.driver.internal.spi.Collector;
2521
import org.neo4j.driver.internal.spi.ConnectionPool;
26-
import org.neo4j.driver.v1.Value;
2722
import org.neo4j.driver.v1.util.Function;
2823

2924
class PooledConnectionValidator implements Function<PooledConnection,Boolean>
3025
{
3126
private final ConnectionPool pool;
32-
private final PoolSettings poolSettings;
33-
private static final Map<String,Value> NO_PARAMETERS = new HashMap<>();
3427

35-
PooledConnectionValidator( ConnectionPool pool, PoolSettings poolSettings )
28+
PooledConnectionValidator( ConnectionPool pool )
3629
{
3730
this.pool = pool;
38-
this.poolSettings = poolSettings;
3931
}
4032

4133
@Override
@@ -45,9 +37,7 @@ public Boolean apply( PooledConnection pooledConnection )
4537
// and we should close the conn without bothering to reset the conn at all
4638
return pool.hasAddress( pooledConnection.boltServerAddress() ) &&
4739
!pooledConnection.hasUnrecoverableErrors() &&
48-
reset( pooledConnection ) &&
49-
(pooledConnection.idleTime() <= poolSettings.idleTimeBeforeConnectionTest() ||
50-
ping( pooledConnection ));
40+
reset( pooledConnection );
5141
}
5242

5343
/**
@@ -57,7 +47,7 @@ public Boolean apply( PooledConnection pooledConnection )
5747
* @param conn the PooledConnection
5848
* @return true if the connection is reset successfully without any error, otherwise false.
5949
*/
60-
private boolean reset( PooledConnection conn )
50+
private static boolean reset( PooledConnection conn )
6151
{
6252
try
6353
{
@@ -70,19 +60,4 @@ private boolean reset( PooledConnection conn )
7060
return false;
7161
}
7262
}
73-
74-
private boolean ping( PooledConnection conn )
75-
{
76-
try
77-
{
78-
conn.run( "RETURN 1 // JavaDriver poll to test connection", NO_PARAMETERS, Collector.NO_OP );
79-
conn.pullAll( Collector.NO_OP );
80-
conn.sync();
81-
return true;
82-
}
83-
catch ( Throwable e )
84-
{
85-
return false;
86-
}
87-
}
8863
}

‎driver/src/main/java/org/neo4j/driver/internal/net/pooling/SocketConnectionPool.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import java.util.List;
2222
import java.util.Map;
2323
import java.util.concurrent.ConcurrentHashMap;
24-
import java.util.concurrent.atomic.AtomicBoolean;
2524

2625
import org.neo4j.driver.internal.ConnectionSettings;
2726
import org.neo4j.driver.internal.net.BoltServerAddress;
@@ -113,10 +112,11 @@ public Connection acquire( final BoltServerAddress address )
113112
@Override
114113
public PooledConnection get()
115114
{
116-
return new PooledConnection( connect( address ), new
117-
PooledConnectionReleaseConsumer( connections,
118-
new PooledConnectionValidator( SocketConnectionPool.this, poolSettings ) ), clock );
119-
115+
PooledConnectionValidator connectionValidator =
116+
new PooledConnectionValidator( SocketConnectionPool.this );
117+
PooledConnectionReleaseConsumer releaseConsumer =
118+
new PooledConnectionReleaseConsumer( connections, connectionValidator );
119+
return new PooledConnection( connect( address ), releaseConsumer, clock );
120120
}
121121
};
122122
PooledConnection conn = connections.acquire( supplier );

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

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,6 @@ public class Config
5151

5252
private final int maxIdleConnectionPoolSize;
5353

54-
/** Connections that have been idle longer than this threshold will have a ping test performed on them. */
55-
private final long idleTimeBeforeConnectionTest;
56-
5754
/** Level of encryption we need to adhere to */
5855
private final EncryptionLevel encryptionLevel;
5956

@@ -68,7 +65,6 @@ private Config( ConfigBuilder builder)
6865
this.logging = builder.logging;
6966

7067
this.maxIdleConnectionPoolSize = builder.maxIdleConnectionPoolSize;
71-
this.idleTimeBeforeConnectionTest = builder.idleTimeBeforeConnectionTest;
7268

7369
this.encryptionLevel = builder.encryptionLevel;
7470
this.trustStrategy = builder.trustStrategy;
@@ -107,11 +103,15 @@ public int maxIdleConnectionPoolSize()
107103
/**
108104
* Pooled connections that have been unused for longer than this timeout will be tested before they are
109105
* used again, to ensure they are still live.
106+
*
110107
* @return idle time in milliseconds
108+
* @deprecated pooled sessions are automatically checked for validity before being returned to the pool. This
109+
* method will always return <code>-1</code> and will be possibly removed in future.
111110
*/
111+
@Deprecated
112112
public long idleTimeBeforeConnectionTest()
113113
{
114-
return idleTimeBeforeConnectionTest;
114+
return -1;
115115
}
116116

117117
/**
@@ -159,7 +159,6 @@ public static class ConfigBuilder
159159
{
160160
private Logging logging = new JULogging( Level.INFO );
161161
private int maxIdleConnectionPoolSize = PoolSettings.DEFAULT_MAX_IDLE_CONNECTION_POOL_SIZE;
162-
private long idleTimeBeforeConnectionTest = PoolSettings.DEFAULT_IDLE_TIME_BEFORE_CONNECTION_TEST;
163162
private EncryptionLevel encryptionLevel = EncryptionLevel.REQUIRED;
164163
private TrustStrategy trustStrategy = trustAllCertificates();
165164
private int routingFailureLimit = 1;
@@ -229,10 +228,12 @@ public ConfigBuilder withMaxIdleSessions( int size )
229228
*
230229
* @param timeout minimum idle time in milliseconds
231230
* @return this builder
231+
* @deprecated pooled sessions are automatically checked for validity before being returned to the pool.
232+
* This setting will be ignored and possibly removed in future.
232233
*/
234+
@Deprecated
233235
public ConfigBuilder withSessionLivenessCheckTimeout( long timeout )
234236
{
235-
this.idleTimeBeforeConnectionTest = timeout;
236237
return this;
237238
}
238239

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,9 +177,7 @@ public static Driver driver( URI uri, AuthToken authToken, Config config )
177177
}
178178

179179
// Establish pool settings
180-
PoolSettings poolSettings = new PoolSettings(
181-
config.maxIdleConnectionPoolSize(),
182-
config.idleTimeBeforeConnectionTest() );
180+
PoolSettings poolSettings = new PoolSettings( config.maxIdleConnectionPoolSize() );
183181

184182
// And finally, construct the driver proper
185183
ConnectionPool connectionPool =

‎driver/src/test/java/org/neo4j/driver/internal/ConfigTest.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,7 @@
2626
import org.neo4j.driver.v1.util.FileTools;
2727

2828
import static java.lang.System.getProperty;
29-
import static org.hamcrest.CoreMatchers.equalTo;
3029
import static org.junit.Assert.assertEquals;
31-
import static org.junit.Assert.assertThat;
3230

3331
public class ConfigTest
3432
{
@@ -79,13 +77,13 @@ public void shouldChangeToTrustedCert()
7977
}
8078

8179
@Test
82-
public void shouldConfigureMinIdleTime() throws Throwable
80+
public void shouldIgnoreLivenessCheckTimeoutSetting() throws Throwable
8381
{
8482
// when
8583
Config config = Config.build().withSessionLivenessCheckTimeout( 1337 ).toConfig();
8684

8785
// then
88-
assertThat( config.idleTimeBeforeConnectionTest(), equalTo( 1337L ) );
86+
assertEquals( -1, config.idleTimeBeforeConnectionTest() );
8987
}
9088

9189
public static void deleteDefaultKnownCertFileIfExists()

‎driver/src/test/java/org/neo4j/driver/internal/net/SocketConnectionTest.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828

2929
import org.neo4j.driver.internal.messaging.Message;
3030
import org.neo4j.driver.internal.messaging.SuccessMessage;
31+
import org.neo4j.driver.internal.summary.InternalServerInfo;
3132
import org.neo4j.driver.v1.Logger;
3233
import org.neo4j.driver.v1.Values;
3334
import org.neo4j.driver.v1.summary.ServerInfo;
@@ -48,7 +49,7 @@ public void shouldReceiveServerInfoAfterInit() throws Throwable
4849
{
4950
// Given
5051
SocketClient socket = mock( SocketClient.class );
51-
SocketConnection conn = new SocketConnection( socket, mock( Logger.class ) );
52+
SocketConnection conn = new SocketConnection( socket, mock( InternalServerInfo.class ), mock( Logger.class ) );
5253

5354
when( socket.address() ).thenReturn( BoltServerAddress.from( URI.create( "http://neo4j.com:9000" ) ) );
5455

‎driver/src/test/java/org/neo4j/driver/internal/net/pooling/ConnectionInvalidationTest.java‎

Lines changed: 16 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,13 @@
2323

2424
import java.io.IOException;
2525
import java.util.HashMap;
26-
import java.util.concurrent.atomic.AtomicBoolean;
2726

2827
import org.neo4j.driver.internal.net.BoltServerAddress;
2928
import org.neo4j.driver.internal.spi.Collector;
3029
import org.neo4j.driver.internal.spi.Connection;
3130
import org.neo4j.driver.internal.spi.ConnectionPool;
3231
import org.neo4j.driver.internal.util.Clock;
3332
import org.neo4j.driver.internal.util.Consumers;
34-
import org.neo4j.driver.v1.Config;
3533
import org.neo4j.driver.v1.Value;
3634
import org.neo4j.driver.v1.exceptions.ClientException;
3735
import org.neo4j.driver.v1.exceptions.Neo4jException;
@@ -51,72 +49,57 @@
5149
import static org.mockito.Mockito.never;
5250
import static org.mockito.Mockito.verify;
5351
import static org.mockito.Mockito.when;
52+
import static org.neo4j.driver.internal.net.BoltServerAddress.LOCAL_DEFAULT;
5453

5554
public class ConnectionInvalidationTest
5655
{
5756
private final Connection delegate = mock( Connection.class );
58-
Clock clock = mock( Clock.class );
57+
private final Clock clock = mock( Clock.class );
5958

6059
private final PooledConnection conn =
6160
new PooledConnection( delegate, Consumers.<PooledConnection>noOp(), Clock.SYSTEM );
6261

6362
@SuppressWarnings( "unchecked" )
6463
@Test
65-
public void shouldInvalidateConnectionThatIsOld() throws Throwable
64+
public void shouldNotInvalidateConnectionThatIsUnableToRun() throws Throwable
6665
{
6766
// Given a connection that's broken
6867
Mockito.doThrow( new ClientException( "That didn't work" ) )
6968
.when( delegate ).run( anyString(), anyMap(), any( Collector.class ) );
70-
PoolSettings poolSettings = PoolSettings.defaultSettings();
71-
when( clock.millis() ).thenReturn( 0L, poolSettings.idleTimeBeforeConnectionTest() + 1L );
7269
PooledConnection conn = new PooledConnection( delegate, Consumers.<PooledConnection>noOp(), clock );
70+
PooledConnectionValidator validator = new PooledConnectionValidator( pool( true ) );
7371

7472
// When/Then
7573
BlockingPooledConnectionQueue
7674
queue = mock( BlockingPooledConnectionQueue.class );
77-
PooledConnectionValidator validator =
78-
new PooledConnectionValidator( pool( true ), poolSettings );
79-
8075
PooledConnectionReleaseConsumer consumer =
81-
new PooledConnectionReleaseConsumer( queue, validator);
76+
new PooledConnectionReleaseConsumer( queue,validator );
8277
consumer.accept( conn );
8378

84-
verify( queue, never() ).offer( conn );
79+
verify( queue ).offer( conn );
8580
}
8681

87-
@SuppressWarnings( "unchecked" )
8882
@Test
89-
public void shouldNotInvalidateConnectionThatIsNotOld() throws Throwable
83+
public void shouldInvalidateConnectionWithUnknownAddress()
9084
{
91-
// Given a connection that's broken
92-
Mockito.doThrow( new ClientException( "That didn't work" ) )
93-
.when( delegate ).run( anyString(), anyMap(), any( Collector.class ) );
94-
Config config = Config.defaultConfig();
95-
PoolSettings poolSettings = PoolSettings.defaultSettings();
96-
when( clock.millis() ).thenReturn( 0L, poolSettings.idleTimeBeforeConnectionTest() - 1L );
97-
PooledConnection conn = new PooledConnection( delegate, Consumers.<PooledConnection>noOp(), clock );
98-
PooledConnectionValidator validator =
99-
new PooledConnectionValidator( pool( true ), poolSettings );
85+
when( delegate.boltServerAddress() ).thenReturn( LOCAL_DEFAULT );
10086

101-
// When/Then
102-
BlockingPooledConnectionQueue
103-
queue = mock( BlockingPooledConnectionQueue.class );
104-
PooledConnectionReleaseConsumer consumer =
105-
new PooledConnectionReleaseConsumer( queue,validator );
87+
BlockingPooledConnectionQueue queue = mock( BlockingPooledConnectionQueue.class );
88+
PooledConnectionValidator validator = new PooledConnectionValidator( pool( false ) );
89+
90+
PooledConnectionReleaseConsumer consumer = new PooledConnectionReleaseConsumer( queue, validator );
10691
consumer.accept( conn );
10792

108-
verify( queue ).offer( conn );
93+
verify( queue, never() ).offer( conn );
10994
}
11095

11196
@Test
11297
public void shouldInvalidConnectionIfFailedToReset() throws Throwable
11398
{
11499
// Given a connection that's broken
115100
Mockito.doThrow( new ClientException( "That didn't work" ) ).when( delegate ).reset();
116-
PoolSettings poolSettings = PoolSettings.defaultSettings();
117101
PooledConnection conn = new PooledConnection( delegate, Consumers.<PooledConnection>noOp(), clock );
118-
PooledConnectionValidator validator =
119-
new PooledConnectionValidator( pool( true ), poolSettings );
102+
PooledConnectionValidator validator = new PooledConnectionValidator( pool( true ) );
120103
// When/Then
121104
BlockingPooledConnectionQueue
122105
queue = mock( BlockingPooledConnectionQueue.class );
@@ -165,9 +148,7 @@ private void assertUnrecoverable( Neo4jException exception )
165148
{
166149
assertThat( e, equalTo( exception ) );
167150
}
168-
PoolSettings poolSettings = PoolSettings.defaultSettings();
169-
PooledConnectionValidator validator =
170-
new PooledConnectionValidator( pool( true ), poolSettings );
151+
PooledConnectionValidator validator = new PooledConnectionValidator( pool( true ) );
171152

172153
// Then
173154
assertTrue( conn.hasUnrecoverableErrors() );
@@ -198,9 +179,7 @@ private void assertRecoverable( Neo4jException exception )
198179

199180
// Then
200181
assertFalse( conn.hasUnrecoverableErrors() );
201-
PoolSettings poolSettings = PoolSettings.defaultSettings();
202-
PooledConnectionValidator validator =
203-
new PooledConnectionValidator( pool( true ), poolSettings );
182+
PooledConnectionValidator validator = new PooledConnectionValidator( pool( true ) );
204183
BlockingPooledConnectionQueue
205184
queue = mock( BlockingPooledConnectionQueue.class );
206185
PooledConnectionReleaseConsumer consumer =

0 commit comments

Comments
 (0)