Skip to content

Commit

Permalink
Merge pull request #432 from lutovich/1.5-deprecate-idleConnectionPoo…
Browse files Browse the repository at this point in the history
…lSize

Deprecated 'idleConnectionPoolSize' config option
  • Loading branch information
zhenlineo authored Nov 24, 2017
2 parents 0bfa545 + b8db2f0 commit 5613e17
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 60 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,10 @@ protected ConnectionPool createConnectionPool( AuthToken authToken, SecurityPlan
Clock clock = createClock();
ConnectionSettings settings = new ConnectionSettings( authToken, config.connectionTimeoutMillis() );
ChannelConnector connector = createConnector( settings, securityPlan, config, clock );
PoolSettings poolSettings = new PoolSettings( config.maxIdleConnectionPoolSize(),
config.idleTimeBeforeConnectionTest(), config.maxConnectionLifetimeMillis(),
config.maxConnectionPoolSize(),
config.connectionAcquisitionTimeoutMillis() );
PoolSettings poolSettings = new PoolSettings( config.maxConnectionPoolSize(),
config.connectionAcquisitionTimeoutMillis(), config.maxConnectionLifetimeMillis(),
config.idleTimeBeforeConnectionTest()
);
return new ConnectionPoolImpl( connector, bootstrap, poolSettings, config.logging(), clock );
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,30 +25,22 @@ public class PoolSettings
public static final int NOT_CONFIGURED = -1;

public static final int DEFAULT_MAX_CONNECTION_POOL_SIZE = 100;
public static final int DEFAULT_MAX_IDLE_CONNECTION_POOL_SIZE = DEFAULT_MAX_CONNECTION_POOL_SIZE;
public static final long DEFAULT_IDLE_TIME_BEFORE_CONNECTION_TEST = NOT_CONFIGURED;
public static final long DEFAULT_MAX_CONNECTION_LIFETIME = TimeUnit.HOURS.toMillis( 1 );
public static final long DEFAULT_CONNECTION_ACQUISITION_TIMEOUT = TimeUnit.SECONDS.toMillis( 60 );

private final int maxIdleConnectionPoolSize;
private final long idleTimeBeforeConnectionTest;
private final long maxConnectionLifetime;
private final int maxConnectionPoolSize;
private final long connectionAcquisitionTimeout;
private final long maxConnectionLifetime;
private final long idleTimeBeforeConnectionTest;

public PoolSettings( int maxIdleConnectionPoolSize, long idleTimeBeforeConnectionTest, long maxConnectionLifetime,
int maxConnectionPoolSize, long connectionAcquisitionTimeout )
public PoolSettings( int maxConnectionPoolSize, long connectionAcquisitionTimeout,
long maxConnectionLifetime, long idleTimeBeforeConnectionTest )
{
this.maxIdleConnectionPoolSize = maxIdleConnectionPoolSize;
this.idleTimeBeforeConnectionTest = idleTimeBeforeConnectionTest;
this.maxConnectionLifetime = maxConnectionLifetime;
this.maxConnectionPoolSize = maxConnectionPoolSize;
this.connectionAcquisitionTimeout = connectionAcquisitionTimeout;
}

public int maxIdleConnectionPoolSize()
{
return maxIdleConnectionPoolSize;
this.maxConnectionLifetime = maxConnectionLifetime;
this.idleTimeBeforeConnectionTest = idleTimeBeforeConnectionTest;
}

public long idleTimeBeforeConnectionTest()
Expand Down
51 changes: 38 additions & 13 deletions driver/src/main/java/org/neo4j/driver/v1/Config.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
/**
* A configuration class to config driver properties.
* <p>
* To create a config:
* To build a simple config with custom logging implementation:
* <pre>
* {@code
* Config config = Config
Expand All @@ -47,6 +47,20 @@
* .toConfig();
* }
* </pre>
* <p>
* To build a more complicated config with tuned connection pool options:
* <pre>
* {@code
* Config config = Config.build()
* .withEncryption()
* .withConnectionTimeout(10, TimeUnit.SECONDS)
* .withMaxConnectionLifetime(30, TimeUnit.MINUTES)
* .withMaxConnectionPoolSize(10)
* .withConnectionAcquisitionTimeout(20, TimeUnit.SECONDS)
* .toConfig();
* }
* </pre>
*
* @since 1.0
*/
@Immutable
Expand All @@ -56,7 +70,6 @@ public class Config
private final Logging logging;
private final boolean logLeakedSessions;

private final int maxIdleConnectionPoolSize;
private final int maxConnectionPoolSize;

private final long idleTimeBeforeConnectionTest;
Expand All @@ -83,7 +96,6 @@ private Config( ConfigBuilder builder)

this.idleTimeBeforeConnectionTest = builder.idleTimeBeforeConnectionTest;
this.maxConnectionLifetimeMillis = builder.maxConnectionLifetimeMillis;
this.maxIdleConnectionPoolSize = builder.maxIdleConnectionPoolSize;
this.maxConnectionPoolSize = builder.maxConnectionPoolSize;
this.connectionAcquisitionTimeoutMillis = builder.connectionAcquisitionTimeoutMillis;

Expand Down Expand Up @@ -117,21 +129,26 @@ public boolean logLeakedSessions()

/**
* Max number of connections per URL for this driver.
*
* @return the max number of connections
* @deprecated please use {@link #maxConnectionPoolSize()} instead.
*/
@Deprecated
public int connectionPoolSize()
{
return maxIdleConnectionPoolSize;
return maxConnectionPoolSize;
}

/**
* Max number of idle connections per URL for this driver.
*
* @return the max number of connections
* @deprecated please use {@link #maxConnectionPoolSize()} instead.
*/
@Deprecated
public int maxIdleConnectionPoolSize()
{
return maxIdleConnectionPoolSize;
return maxConnectionPoolSize;
}

/**
Expand Down Expand Up @@ -243,7 +260,6 @@ public static class ConfigBuilder
{
private Logging logging = new JULogging( Level.INFO );
private boolean logLeakedSessions;
private int maxIdleConnectionPoolSize = PoolSettings.DEFAULT_MAX_IDLE_CONNECTION_POOL_SIZE;
private int maxConnectionPoolSize = PoolSettings.DEFAULT_MAX_CONNECTION_POOL_SIZE;
private long idleTimeBeforeConnectionTest = PoolSettings.DEFAULT_IDLE_TIME_BEFORE_CONNECTION_TEST;
private long maxConnectionLifetimeMillis = PoolSettings.DEFAULT_MAX_CONNECTION_LIFETIME;
Expand Down Expand Up @@ -311,46 +327,56 @@ public ConfigBuilder withLeakedSessionsLogging()
* The max number of sessions to keep open at once. Configure this
* higher if you want more concurrent sessions, or lower if you want
* to lower the pressure on the database instance.
*
* <p>
* If the driver is asked to provide more sessions than this, it will
* block waiting for another session to be closed, with a timeout.
* <p>
* Method is deprecated and will forward the given argument to {@link #withMaxConnectionPoolSize(int)}.
*
* @param size the max number of sessions to keep open
* @return this builder
* @deprecated please use a combination of {@link #withMaxConnectionPoolSize(int)} and
* {@link #withConnectionAcquisitionTimeout(long, TimeUnit)} instead.
*/
@Deprecated
public ConfigBuilder withMaxSessions( int size )
{
return this;
return withMaxConnectionPoolSize( size );
}

/**
* The max number of idle sessions to keep open at once. Configure this
* higher if you want more concurrent sessions, or lower if you want
* to lower the pressure on the database instance.
* <p>
* Method is deprecated and will not change the driver configuration.
*
* @param size the max number of idle sessions to keep open
* @return this builder
* @deprecated please use {@link #withMaxIdleConnections(int)} instead.
* @deprecated please use a combination of {@link #withMaxConnectionPoolSize(int)} and
* {@link #withConnectionAcquisitionTimeout(long, TimeUnit)} instead.
*/
@Deprecated
public ConfigBuilder withMaxIdleSessions( int size )
{
this.maxIdleConnectionPoolSize = size;
return this;
}

/**
* The max number of idle connections to keep open at once. Configure this
* higher for greater concurrency, or lower to reduce the pressure on the
* database instance.
* <p>
* Method is deprecated and will not change the driver configuration.
*
* @param size the max number of idle connections to keep open
* @return this builder
* @deprecated please use a combination of {@link #withMaxConnectionPoolSize(int)} and
* {@link #withConnectionAcquisitionTimeout(long, TimeUnit)} instead.
*/
@Deprecated
public ConfigBuilder withMaxIdleConnections( int size )
{
this.maxIdleConnectionPoolSize = size;
return this;
}

Expand All @@ -366,8 +392,7 @@ public ConfigBuilder withMaxIdleConnections( int size )
@Deprecated
public ConfigBuilder withSessionLivenessCheckTimeout( long timeout )
{
withConnectionLivenessCheckTimeout( timeout, TimeUnit.MILLISECONDS );
return this;
return withConnectionLivenessCheckTimeout( timeout, TimeUnit.MILLISECONDS );
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ private ConnectionPoolImpl newPool() throws Exception
ChannelConnectorImpl connector =
new ChannelConnectorImpl( connectionSettings, SecurityPlan.forAllCertificates(),
DEV_NULL_LOGGING, clock );
PoolSettings poolSettings = new PoolSettings( 5, -1, -1, 10, 5000 );
PoolSettings poolSettings = new PoolSettings( 10, 5000, -1, -1 );
Bootstrap bootstrap = BootstrapFactory.newBootstrap( 1 );
return new ConnectionPoolImpl( connector, bootstrap, poolSettings, DEV_NULL_LOGGING, clock );
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
import static org.neo4j.driver.internal.async.pool.PoolSettings.DEFAULT_CONNECTION_ACQUISITION_TIMEOUT;
import static org.neo4j.driver.internal.async.pool.PoolSettings.DEFAULT_IDLE_TIME_BEFORE_CONNECTION_TEST;
import static org.neo4j.driver.internal.async.pool.PoolSettings.DEFAULT_MAX_CONNECTION_POOL_SIZE;
import static org.neo4j.driver.internal.async.pool.PoolSettings.DEFAULT_MAX_IDLE_CONNECTION_POOL_SIZE;
import static org.neo4j.driver.internal.async.pool.PoolSettings.NOT_CONFIGURED;
import static org.neo4j.driver.internal.logging.DevNullLogging.DEV_NULL_LOGGING;
import static org.neo4j.driver.internal.util.Iterables.single;
Expand All @@ -67,14 +66,13 @@ public void tearDown()
@Test
public void shouldDropTooOldChannelsWhenMaxLifetimeEnabled()
{
int maxConnectionLifetime = 1000;
PoolSettings settings = new PoolSettings( DEFAULT_MAX_IDLE_CONNECTION_POOL_SIZE,
DEFAULT_IDLE_TIME_BEFORE_CONNECTION_TEST, maxConnectionLifetime, DEFAULT_MAX_CONNECTION_POOL_SIZE,
DEFAULT_CONNECTION_ACQUISITION_TIMEOUT );
int maxLifetime = 1000;
PoolSettings settings = new PoolSettings( DEFAULT_MAX_CONNECTION_POOL_SIZE,
DEFAULT_CONNECTION_ACQUISITION_TIMEOUT, maxLifetime, DEFAULT_IDLE_TIME_BEFORE_CONNECTION_TEST );
Clock clock = Clock.SYSTEM;
NettyChannelHealthChecker healthChecker = newHealthChecker( settings, clock );

setCreationTimestamp( channel, clock.millis() - maxConnectionLifetime * 2 );
setCreationTimestamp( channel, clock.millis() - maxLifetime * 2 );
Future<Boolean> healthy = healthChecker.isHealthy( channel );

assertThat( await( healthy ), is( false ) );
Expand All @@ -83,9 +81,8 @@ public void shouldDropTooOldChannelsWhenMaxLifetimeEnabled()
@Test
public void shouldAllowVeryOldChannelsWhenMaxLifetimeDisabled()
{
PoolSettings settings = new PoolSettings( DEFAULT_MAX_IDLE_CONNECTION_POOL_SIZE,
DEFAULT_IDLE_TIME_BEFORE_CONNECTION_TEST, NOT_CONFIGURED, DEFAULT_MAX_CONNECTION_POOL_SIZE,
DEFAULT_CONNECTION_ACQUISITION_TIMEOUT );
PoolSettings settings = new PoolSettings( DEFAULT_MAX_CONNECTION_POOL_SIZE,
DEFAULT_CONNECTION_ACQUISITION_TIMEOUT, NOT_CONFIGURED, DEFAULT_IDLE_TIME_BEFORE_CONNECTION_TEST );
NettyChannelHealthChecker healthChecker = newHealthChecker( settings, Clock.SYSTEM );

setCreationTimestamp( channel, 0 );
Expand Down Expand Up @@ -121,9 +118,8 @@ public void shouldDropInactiveConnections()
private void testPing( boolean resetMessageSuccessful )
{
int idleTimeBeforeConnectionTest = 1000;
PoolSettings settings = new PoolSettings( DEFAULT_MAX_IDLE_CONNECTION_POOL_SIZE,
idleTimeBeforeConnectionTest, NOT_CONFIGURED, DEFAULT_MAX_CONNECTION_POOL_SIZE,
DEFAULT_CONNECTION_ACQUISITION_TIMEOUT );
PoolSettings settings = new PoolSettings( DEFAULT_MAX_CONNECTION_POOL_SIZE,
DEFAULT_CONNECTION_ACQUISITION_TIMEOUT, NOT_CONFIGURED, idleTimeBeforeConnectionTest );
Clock clock = Clock.SYSTEM;
NettyChannelHealthChecker healthChecker = newHealthChecker( settings, clock );

Expand All @@ -149,9 +145,8 @@ private void testPing( boolean resetMessageSuccessful )

private void testActiveConnectionCheck( boolean channelActive )
{
PoolSettings settings = new PoolSettings( DEFAULT_MAX_IDLE_CONNECTION_POOL_SIZE,
DEFAULT_IDLE_TIME_BEFORE_CONNECTION_TEST, NOT_CONFIGURED, DEFAULT_MAX_CONNECTION_POOL_SIZE,
DEFAULT_CONNECTION_ACQUISITION_TIMEOUT );
PoolSettings settings = new PoolSettings( DEFAULT_MAX_CONNECTION_POOL_SIZE,
DEFAULT_CONNECTION_ACQUISITION_TIMEOUT, NOT_CONFIGURED, DEFAULT_IDLE_TIME_BEFORE_CONNECTION_TEST );
Clock clock = Clock.SYSTEM;
NettyChannelHealthChecker healthChecker = newHealthChecker( settings, clock );

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@

import org.junit.Test;

import org.neo4j.driver.internal.async.pool.PoolSettings;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
Expand All @@ -31,7 +29,7 @@ public class PoolSettingsTest
@Test
public void idleTimeBeforeConnectionTestWhenConfigured()
{
PoolSettings settings = new PoolSettings( 10, 42, 10, 5, -1 );
PoolSettings settings = new PoolSettings( 5, -1, 10, 42 );
assertTrue( settings.idleTimeBeforeConnectionTestEnabled() );
assertEquals( 42, settings.idleTimeBeforeConnectionTest() );
}
Expand All @@ -40,7 +38,7 @@ public void idleTimeBeforeConnectionTestWhenConfigured()
public void idleTimeBeforeConnectionTestWhenSetToZero()
{
//Always test idle time during acquisition
PoolSettings settings = new PoolSettings( 10, 0, 10, 5, -1 );
PoolSettings settings = new PoolSettings( 5, -1, 10, 0 );
assertTrue( settings.idleTimeBeforeConnectionTestEnabled() );
assertEquals( 0, settings.idleTimeBeforeConnectionTest() );
}
Expand All @@ -57,7 +55,7 @@ public void idleTimeBeforeConnectionTestWhenSetToNegativeValue()
@Test
public void maxConnectionLifetimeWhenConfigured()
{
PoolSettings settings = new PoolSettings( 10, 10, 42, 5, -1 );
PoolSettings settings = new PoolSettings( 5, -1, 42, 10 );
assertTrue( settings.maxConnectionLifetimeEnabled() );
assertEquals( 42, settings.maxConnectionLifetime() );
}
Expand All @@ -73,13 +71,13 @@ public void maxConnectionLifetimeWhenSetToZeroOrNegativeValue()

private static void testIdleTimeBeforeConnectionTestWithIllegalValue( int value )
{
PoolSettings settings = new PoolSettings( 10, value, 10, 5, -1 );
PoolSettings settings = new PoolSettings( 5, -1, 10, value );
assertFalse( settings.idleTimeBeforeConnectionTestEnabled() );
}

private static void testMaxConnectionLifetimeWithIllegalValue( int value )
{
PoolSettings settings = new PoolSettings( 10, 10, value, 5, -1 );
PoolSettings settings = new PoolSettings( 5, -1, value, 10 );
assertFalse( settings.maxConnectionLifetimeEnabled() );
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -298,9 +298,9 @@ protected ConnectionPool createConnectionPool( AuthToken authToken, SecurityPlan
Bootstrap bootstrap, Config config )
{
ConnectionSettings connectionSettings = new ConnectionSettings( authToken, 1000 );
PoolSettings poolSettings = new PoolSettings( config.maxIdleConnectionPoolSize(),
config.idleTimeBeforeConnectionTest(), config.maxConnectionLifetimeMillis(),
config.maxConnectionPoolSize(), config.connectionAcquisitionTimeoutMillis() );
PoolSettings poolSettings = new PoolSettings( config.maxConnectionPoolSize(),
config.connectionAcquisitionTimeoutMillis(), config.maxConnectionLifetimeMillis(),
config.idleTimeBeforeConnectionTest() );
Clock clock = createClock();
ChannelConnector connector = super.createConnector( connectionSettings, securityPlan, config, clock );
connectionPool =
Expand All @@ -314,9 +314,8 @@ private static class MemorizingConnectionPool extends ConnectionPoolImpl
Connection lastAcquiredConnectionSpy;
boolean memorize;

public MemorizingConnectionPool( ChannelConnector connector,
Bootstrap bootstrap, PoolSettings settings, Logging logging,
Clock clock )
MemorizingConnectionPool( ChannelConnector connector, Bootstrap bootstrap, PoolSettings settings,
Logging logging, Clock clock )
{
super( connector, bootstrap, settings, logging, clock );
}
Expand Down

0 comments on commit 5613e17

Please sign in to comment.