Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecated 'idleConnectionPoolSize' config option #432

Merged
merged 3 commits into from
Nov 24, 2017
Merged

Deprecated 'idleConnectionPoolSize' config option #432

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
06d1d8a
Deprecated 'idleConnectionPoolSize' config option
lutovich Nov 20, 2017
239ff36
Improved javadoc for couple Config methods
lutovich Nov 21, 2017
b8db2f0
Ignore `maxIdleSessions` and `maxIdleConnections`
lutovich Nov 23, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions driver/src/main/java/org/neo4j/driver/internal/DriverFactory.java
View file
Open in desktop
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
20 changes: 6 additions & 14 deletions driver/src/main/java/org/neo4j/driver/internal/async/pool/PoolSettings.java
View file
Open in desktop
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
View file
Open in desktop
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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest changing to Please consider to use #maxConnectionPoolSize and #MaxConnectionLifetime to replace this method instead.

Same applies for comments of all MaxIdle methods

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improved javadoc with your suggestion for withMaxSessions, withMaxIdleSessions, and withMaxIdleConnections.

*/
@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
2 changes: 1 addition & 1 deletion driver/src/test/java/org/neo4j/driver/internal/async/pool/ConnectionPoolImplTest.java
View file
Open in desktop
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
25 changes: 10 additions & 15 deletions driver/src/test/java/org/neo4j/driver/internal/async/pool/NettyChannelHealthCheckerTest.java
View file
Open in desktop
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
12 changes: 5 additions & 7 deletions driver/src/test/java/org/neo4j/driver/internal/async/pool/PoolSettingsTest.java
View file
Open in desktop
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() );
}
}
11 changes: 5 additions & 6 deletions driver/src/test/java/org/neo4j/driver/v1/integration/ConnectionHandlingIT.java
View file
Open in desktop
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