diff --git a/build.gradle b/build.gradle index 07aa953fa..ae4fa989c 100644 --- a/build.gradle +++ b/build.gradle @@ -52,6 +52,9 @@ dependencies { compile("org.codehaus.jackson:jackson-mapper-asl:$jacksonVersion", optional) compile("commons-beanutils:commons-beanutils-core:1.8.3", optional) + // Pool + compile("commons-pool:commons-pool:1.5.6", optional) + // Testing testCompile "junit:junit:$junitVersion" testCompile "org.springframework:spring-test:$springVersion" diff --git a/docs/src/reference/docbook/reference/redis.xml b/docs/src/reference/docbook/reference/redis.xml index 7c19e705e..8d13f14fc 100644 --- a/docs/src/reference/docbook/reference/redis.xml +++ b/docs/src/reference/docbook/reference/redis.xml @@ -134,7 +134,28 @@ p:host-name="server" p:port="6379"/> ]]> - As one can note, the configuration is quite similar to the Jedis one. + The configuration is quite similar to Jedis, with one notable exception. By default, the JedisConnectionFactory pools connections. + In order to use a connection pool with JRedis, configure the JredisConnectionFactory with an instance of JredisPool. For example: + + + + + + + + + + + + + + +]]> diff --git a/src/main/java/org/springframework/data/redis/connection/jredis/DefaultJredisPool.java b/src/main/java/org/springframework/data/redis/connection/jredis/DefaultJredisPool.java new file mode 100644 index 000000000..bdaae0275 --- /dev/null +++ b/src/main/java/org/springframework/data/redis/connection/jredis/DefaultJredisPool.java @@ -0,0 +1,213 @@ +/* + * Copyright 2011-2013 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.redis.connection.jredis; + +import org.apache.commons.pool.BasePoolableObjectFactory; +import org.apache.commons.pool.impl.GenericObjectPool; +import org.apache.commons.pool.impl.GenericObjectPool.Config; +import org.jredis.ClientRuntimeException; +import org.jredis.JRedis; +import org.jredis.connector.Connection; +import org.jredis.connector.ConnectionSpec; +import org.jredis.connector.Connection.Socket.Property; +import org.jredis.ri.alphazero.JRedisClient; +import org.jredis.ri.alphazero.connection.DefaultConnectionSpec; +import org.springframework.util.StringUtils; + +/** + * Default implementation of {@link JredisPool} + * + * @author Jennifer Hickey + * + */ +public class DefaultJredisPool implements JredisPool { + + private final GenericObjectPool internalPool; + + /** + * Uses the {@link Config} and {@link ConnectionSpec} defaults for + * configuring the connection pool + * + * @param hostName + * The Redis host + * @param port + * The Redis port + */ + public DefaultJredisPool(String hostName, int port) { + this(hostName, port, 0, null, 0, new Config()); + } + + /** + * Uses the {@link ConnectionSpec} defaults for configuring the connection + * pool + * + * @param hostName + * The Redis host + * @param port + * The Redis port + * @param poolConfig + * The pool {@link Config} + */ + public DefaultJredisPool(String hostName, int port, Config poolConfig) { + this(hostName, port, 0, null, 0, poolConfig); + } + + /** + * + * Uses the {@link Config} defaults for configuring the connection pool + * + * @param connectionSpec + * The {@link ConnectionSpec} for connecting to Redis + * + */ + public DefaultJredisPool(ConnectionSpec connectionSpec) { + this.internalPool = new GenericObjectPool(new JredisFactory(connectionSpec), new Config()); + } + + /** + * + * @param connectionSpec + * The {@link ConnectionSpec} for connecting to Redis + * + * @param poolConfig + * The pool {@link Config} + */ + public DefaultJredisPool(ConnectionSpec connectionSpec, Config poolConfig) { + this.internalPool = new GenericObjectPool(new JredisFactory(connectionSpec), poolConfig); + } + + /** + * Uses the {@link Config} defaults for configuring the connection pool + * + * @param hostName + * The Redis host + * @param port + * The Redis port + * @param dbIndex + * The index of the database all connections should use. The + * database will only be selected on initial creation of the + * pooled {@link JRedis} instances. Since calling select directly + * on {@link JRedis} is not supported, it is assumed that + * connections can be re-used without subsequent selects. + * @param password + * The password used for authenticating with the Redis server or + * null if no password required + * @param timeout + * The socket timeout or 0 to use the default socket timeout + */ + public DefaultJredisPool(String hostName, int port, int dbIndex, String password, int timeout) { + this(hostName, port, dbIndex, password, timeout, new Config()); + } + + /** + * + * @param hostName + * The Redis host + * @param port + * The Redis port + * @param dbIndex + * The index of the database all connections should use + * @param password + * The password used for authenticating with the Redis server or + * null if no password required + * @param timeout + * The socket timeout or 0 to use the default socket timeout + * @param poolConfig + * The pool {@link COnfig} + */ + public DefaultJredisPool(String hostName, int port, int dbIndex, String password, int timeout, + Config poolConfig) { + ConnectionSpec connectionSpec = DefaultConnectionSpec.newSpec(hostName, port, dbIndex, null); + connectionSpec.setConnectionFlag(Connection.Flag.RELIABLE, false); + if (StringUtils.hasLength(password)) { + connectionSpec.setCredentials(password); + } + if (timeout > 0) { + connectionSpec.setSocketProperty(Property.SO_TIMEOUT, timeout); + } + this.internalPool = new GenericObjectPool(new JredisFactory(connectionSpec), poolConfig); + } + + public JRedis getResource() { + try { + return (JRedis) internalPool.borrowObject(); + } catch (Exception e) { + throw new ClientRuntimeException("Could not get a resource from the pool", e); + } + } + + public void returnBrokenResource(final JRedis resource) { + try { + internalPool.invalidateObject(resource); + } catch (Exception e) { + throw new ClientRuntimeException("Could not invalidate the broken resource", e); + } + } + + public void returnResource(final JRedis resource) { + try { + internalPool.returnObject(resource); + } catch (Exception e) { + throw new ClientRuntimeException("Could not return the resource to the pool", e); + } + } + + public void destroy() { + try { + internalPool.close(); + } catch (Exception e) { + throw new ClientRuntimeException("Could not destroy the pool", e); + } + } + + private static class JredisFactory extends BasePoolableObjectFactory { + + private final ConnectionSpec connectionSpec; + + public JredisFactory(ConnectionSpec connectionSpec) { + super(); + this.connectionSpec = connectionSpec; + } + + public Object makeObject() throws Exception { + return new JRedisClient(connectionSpec); + } + + public void destroyObject(final Object obj) throws Exception { + if (obj instanceof JRedis) { + try { + ((JRedis) obj).quit(); + }catch(Exception e) { + // Errors may happen if returning a broken resource + } + } + } + + public boolean validateObject(final Object obj) { + if (obj instanceof JRedis) { + try { + ((JRedis) obj).ping(); + return true; + } catch (Exception e) { + return false; + } + } else { + return false; + } + } + } + +} diff --git a/src/main/java/org/springframework/data/redis/connection/jredis/JredisConnection.java b/src/main/java/org/springframework/data/redis/connection/jredis/JredisConnection.java index e65ffff6c..c5f3ecb6a 100644 --- a/src/main/java/org/springframework/data/redis/connection/jredis/JredisConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/jredis/JredisConnection.java @@ -109,6 +109,9 @@ public class JredisConnection implements RedisConnection { } public void close() throws RedisSystemException { + if(isClosed()) { + return; + } isClosed = true; if (pool != null) { diff --git a/src/test/java/org/springframework/data/redis/connection/jredis/DefaultJredisPoolTests.java b/src/test/java/org/springframework/data/redis/connection/jredis/DefaultJredisPoolTests.java new file mode 100644 index 000000000..0052241fd --- /dev/null +++ b/src/test/java/org/springframework/data/redis/connection/jredis/DefaultJredisPoolTests.java @@ -0,0 +1,143 @@ +/* + * Copyright 2013 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.redis.connection.jredis; + +import org.jredis.ClientRuntimeException; +import org.jredis.JRedis; +import org.jredis.RedisException; +import org.jredis.connector.ConnectionSpec; +import org.jredis.connector.NotConnectedException; +import org.jredis.ri.alphazero.connection.DefaultConnectionSpec; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.springframework.data.redis.SettingsUtils; +import org.apache.commons.pool.impl.GenericObjectPool.Config; +import static org.junit.Assert.fail; +import static org.junit.Assert.assertNotSame; + +import static org.junit.Assert.assertNotNull; + +/** + * Integration test of {@link DefaultJredisPool} + * + * @author Jennifer Hickey + * + */ +public class DefaultJredisPoolTests { + + private ConnectionSpec connectionSpec; + + private JredisPool pool; + + @Before + public void setUp() { + this.connectionSpec = DefaultConnectionSpec.newSpec(SettingsUtils.getHost(), SettingsUtils.getPort(), + 0, null); + } + + @After + public void tearDown() { + if (this.pool != null) { + this.pool.destroy(); + } + } + + @Test + public void testGetResource() throws RedisException { + this.pool = new DefaultJredisPool(connectionSpec); + JRedis client = pool.getResource(); + assertNotNull(client); + client.ping(); + } + + @Test + public void testGetResourcePoolExhausted() { + Config poolConfig = new Config(); + poolConfig.maxActive = 1; + poolConfig.maxWait = 1; + this.pool = new DefaultJredisPool(connectionSpec, poolConfig); + JRedis client = pool.getResource(); + assertNotNull(client); + try { + pool.getResource(); + fail("ClientRuntimeException should be thrown when pool exhausted"); + } catch (ClientRuntimeException e) { + + } + } + + @Test + public void testGetResourceValidate() { + Config poolConfig = new Config(); + poolConfig.testOnBorrow = true; + this.pool = new DefaultJredisPool(connectionSpec, poolConfig); + JRedis client = pool.getResource(); + assertNotNull(client); + } + + @Test(expected = ClientRuntimeException.class) + public void testGetResourceCreationUnsuccessful() { + // Config poolConfig = new Config(); + // poolConfig.testOnBorrow = true; + this.pool = new DefaultJredisPool(DefaultConnectionSpec.newSpec(SettingsUtils.getHost(), 3333, 0, + null)); + pool.getResource(); + } + + @Test + public void testReturnResource() throws RedisException { + Config poolConfig = new Config(); + poolConfig.maxActive = 1; + poolConfig.maxWait = 1; + this.pool = new DefaultJredisPool(connectionSpec); + JRedis client = pool.getResource(); + assertNotNull(client); + pool.returnResource(client); + assertNotNull(pool.getResource()); + } + + @Test + public void testReturnBrokenResource() throws RedisException { + Config poolConfig = new Config(); + poolConfig.maxActive = 1; + poolConfig.maxWait = 1; + this.pool = new DefaultJredisPool(connectionSpec, poolConfig); + JRedis client = pool.getResource(); + assertNotNull(client); + pool.returnBrokenResource(client); + JRedis client2 = pool.getResource(); + assertNotSame(client, client2); + try { + client.ping(); + fail("Broken resouce connection should be closed"); + } catch (NotConnectedException e) { + } + } + + @Test + public void testCreateWithHostAndPort() { + this.pool = new DefaultJredisPool(SettingsUtils.getHost(), SettingsUtils.getPort()); + assertNotNull(pool.getResource()); + } + + @Test + public void testCreateWithHostPortAndDbIndex() { + this.pool = new DefaultJredisPool(SettingsUtils.getHost(), SettingsUtils.getPort(), 1, null, 0); + assertNotNull(pool.getResource()); + } + +} diff --git a/src/test/java/org/springframework/data/redis/connection/jredis/JRedisConnectionIntegrationTests.java b/src/test/java/org/springframework/data/redis/connection/jredis/JRedisConnectionIntegrationTests.java index cd56b6d24..7f4b75811 100644 --- a/src/test/java/org/springframework/data/redis/connection/jredis/JRedisConnectionIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/connection/jredis/JRedisConnectionIntegrationTests.java @@ -21,6 +21,7 @@ import static org.junit.Assert.fail; import java.util.Arrays; +import org.jredis.JRedis; import org.jredis.protocol.BulkResponse; import org.junit.After; import org.junit.Ignore; @@ -28,11 +29,14 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.dao.DataAccessException; import org.springframework.data.redis.RedisConnectionFailureException; +import org.springframework.data.redis.SettingsUtils; import org.springframework.data.redis.connection.AbstractConnectionIntegrationTests; import org.springframework.data.redis.connection.DefaultSortParameters; +import org.springframework.data.redis.connection.RedisConnection; import org.springframework.data.redis.connection.SortParameters.Order; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.apache.commons.pool.impl.GenericObjectPool.Config; /** * Integration test of {@link JredisConnection} @@ -85,6 +89,35 @@ public class JRedisConnectionIntegrationTests extends AbstractConnectionIntegrat } } + @Test + public void testConnectionStaysOpenWhenPooled() { + JredisConnectionFactory factory2 = new JredisConnectionFactory(new DefaultJredisPool( + SettingsUtils.getHost(), SettingsUtils.getPort())); + RedisConnection conn2 = factory2.getConnection(); + conn2.close(); + conn2.ping(); + } + + @Test + public void testConnectionNotReturnedOnException() { + Config config = new Config(); + config.maxActive = 1; + config.maxWait = 1; + JredisConnectionFactory factory2 = new JredisConnectionFactory(new DefaultJredisPool( + SettingsUtils.getHost(), SettingsUtils.getPort(), config)); + RedisConnection conn2 = factory2.getConnection(); + ((JRedis) conn2.getNativeConnection()).quit(); + try { + conn2.ping(); + fail("Expected RedisConnectionFailureException trying to use a closed connection"); + } catch (RedisConnectionFailureException e) { + } + conn2.close(); + // Verify we get a new connection from the pool and not the broken one + RedisConnection conn3 = factory2.getConnection(); + conn3.ping(); + } + @Test(expected = UnsupportedOperationException.class) public void testMultiExec() throws Exception { super.testMultiExec(); diff --git a/src/test/java/org/springframework/data/redis/support/atomic/AtomicCountersParam.java b/src/test/java/org/springframework/data/redis/support/atomic/AtomicCountersParam.java index cce786acf..493f5abb0 100644 --- a/src/test/java/org/springframework/data/redis/support/atomic/AtomicCountersParam.java +++ b/src/test/java/org/springframework/data/redis/support/atomic/AtomicCountersParam.java @@ -20,6 +20,7 @@ import java.util.Collection; import org.springframework.data.redis.SettingsUtils; import org.springframework.data.redis.connection.jedis.JedisConnectionFactory; +import org.springframework.data.redis.connection.jredis.DefaultJredisPool; import org.springframework.data.redis.connection.jredis.JredisConnectionFactory; import org.springframework.data.redis.connection.lettuce.LettuceConnectionFactory; import org.springframework.data.redis.connection.srp.SrpConnectionFactory; @@ -39,9 +40,8 @@ public abstract class AtomicCountersParam { jedisConnFactory.afterPropertiesSet(); // JRedis - JredisConnectionFactory jredisConnFactory = new JredisConnectionFactory(); - jredisConnFactory.setPort(SettingsUtils.getPort()); - jredisConnFactory.setHostName(SettingsUtils.getHost()); + JredisConnectionFactory jredisConnFactory = new JredisConnectionFactory(new DefaultJredisPool( + SettingsUtils.getHost(), SettingsUtils.getPort())); jredisConnFactory.afterPropertiesSet(); // Lettuce diff --git a/src/test/java/org/springframework/data/redis/support/collections/CollectionTestParams.java b/src/test/java/org/springframework/data/redis/support/collections/CollectionTestParams.java index d47b70812..9eba8da5b 100644 --- a/src/test/java/org/springframework/data/redis/support/collections/CollectionTestParams.java +++ b/src/test/java/org/springframework/data/redis/support/collections/CollectionTestParams.java @@ -21,6 +21,7 @@ import java.util.Collection; import org.springframework.data.redis.Person; import org.springframework.data.redis.SettingsUtils; import org.springframework.data.redis.connection.jedis.JedisConnectionFactory; +import org.springframework.data.redis.connection.jredis.DefaultJredisPool; import org.springframework.data.redis.connection.jredis.JredisConnectionFactory; import org.springframework.data.redis.connection.lettuce.LettuceConnectionFactory; import org.springframework.data.redis.connection.srp.SrpConnectionFactory; @@ -80,11 +81,8 @@ public abstract class CollectionTestParams { jsonPersonTemplate.afterPropertiesSet(); // jredis - JredisConnectionFactory jredisConnFactory = new JredisConnectionFactory(); - - jredisConnFactory.setPort(SettingsUtils.getPort()); - jredisConnFactory.setHostName(SettingsUtils.getHost()); - + JredisConnectionFactory jredisConnFactory = new JredisConnectionFactory(new DefaultJredisPool( + SettingsUtils.getHost(), SettingsUtils.getPort())); jredisConnFactory.afterPropertiesSet(); RedisTemplate stringTemplateJR = new StringRedisTemplate(jredisConnFactory); diff --git a/src/test/java/org/springframework/data/redis/support/collections/RedisMapTests.java b/src/test/java/org/springframework/data/redis/support/collections/RedisMapTests.java index b0eb2973b..f16a89cd3 100644 --- a/src/test/java/org/springframework/data/redis/support/collections/RedisMapTests.java +++ b/src/test/java/org/springframework/data/redis/support/collections/RedisMapTests.java @@ -22,6 +22,7 @@ import org.junit.runners.Parameterized.Parameters; import org.springframework.data.redis.Person; import org.springframework.data.redis.SettingsUtils; import org.springframework.data.redis.connection.jedis.JedisConnectionFactory; +import org.springframework.data.redis.connection.jredis.DefaultJredisPool; import org.springframework.data.redis.connection.jredis.JredisConnectionFactory; import org.springframework.data.redis.connection.lettuce.LettuceConnectionFactory; import org.springframework.data.redis.connection.srp.SrpConnectionFactory; @@ -88,9 +89,8 @@ public class RedisMapTests extends AbstractRedisMapTests { jsonPersonTemplate.afterPropertiesSet(); // JRedis - JredisConnectionFactory jredisConnFactory = new JredisConnectionFactory(); - jredisConnFactory.setPort(SettingsUtils.getPort()); - jredisConnFactory.setHostName(SettingsUtils.getHost()); + JredisConnectionFactory jredisConnFactory = new JredisConnectionFactory(new DefaultJredisPool( + SettingsUtils.getHost(), SettingsUtils.getPort())); jredisConnFactory.afterPropertiesSet(); RedisTemplate genericTemplateJR = new RedisTemplate(); diff --git a/src/test/java/org/springframework/data/redis/support/collections/RedisPropertiesTests.java b/src/test/java/org/springframework/data/redis/support/collections/RedisPropertiesTests.java index d860b93ff..2455f1841 100644 --- a/src/test/java/org/springframework/data/redis/support/collections/RedisPropertiesTests.java +++ b/src/test/java/org/springframework/data/redis/support/collections/RedisPropertiesTests.java @@ -38,6 +38,7 @@ import org.junit.runners.Parameterized.Parameters; import org.springframework.data.redis.Person; import org.springframework.data.redis.SettingsUtils; import org.springframework.data.redis.connection.jedis.JedisConnectionFactory; +import org.springframework.data.redis.connection.jredis.DefaultJredisPool; import org.springframework.data.redis.connection.jredis.JredisConnectionFactory; import org.springframework.data.redis.connection.lettuce.LettuceConnectionFactory; import org.springframework.data.redis.core.RedisTemplate; @@ -258,9 +259,8 @@ public class RedisPropertiesTests extends RedisMapTests { jsonPersonTemplate.afterPropertiesSet(); // JRedis - JredisConnectionFactory jredisConnFactory = new JredisConnectionFactory(); - jredisConnFactory.setPort(SettingsUtils.getPort()); - jredisConnFactory.setHostName(SettingsUtils.getHost()); + JredisConnectionFactory jredisConnFactory = new JredisConnectionFactory(new DefaultJredisPool( + SettingsUtils.getHost(), SettingsUtils.getPort())); jredisConnFactory.afterPropertiesSet(); RedisTemplate genericTemplateJR = new StringRedisTemplate(jredisConnFactory); diff --git a/template.mf b/template.mf index aa4a6be89..cb7b75119 100644 --- a/template.mf +++ b/template.mf @@ -19,10 +19,8 @@ Import-Template: org.w3c.dom.*;version="0", javax.xml.transform.*;resolution:="optional";version="0", org.jredis.*;resolution:="optional";version="[1.0.0, 2.0.0)", - org.jredis.ri.alphazero.*;resolution:="optional";version="[1.0.0, 2.0.0)", redis.clients.*;resolution:="optional";version="[2.1.0, 2.1.0]", - org.idevlab.rjc.*;resolution:="optional";version="[0.6.4, 0.6.4]", - org.apache.commons.pool.impl.*;resolution:="optional";version="[1.0.0, 3.0.0)", + org.apache.commons.pool.*;resolution:="optional";version="[1.0.0, 3.0.0)", org.codehaus.jackson.*;resolution:="optional";version="[1.6, 2.0.0)", org.apache.commons.beanutils.*;resolution:="optional";version=1.8.5, redis.*;resolution:="optional";version="[0.2, 1.0)",