From 71eb4c5b4ae74a18a8db6de91e7f71a2a2797c3c Mon Sep 17 00:00:00 2001 From: Jonathan Leibiusky Date: Sun, 21 Nov 2010 18:16:31 -0300 Subject: [PATCH] replace custom pool implementation with apache's --- .../java/redis/clients/jedis/JedisPool.java | 173 ++++++++---------- .../redis/clients/jedis/ShardedJedisPool.java | 125 +++++++------ src/main/java/redis/clients/util/Pool.java | 30 +++ .../clients/jedis/tests/JedisPoolTest.java | 62 +++---- .../jedis/tests/ShardedJedisPoolTest.java | 61 +++--- .../jedis/tests/benchmark/PoolBenchmark.java | 23 +-- 6 files changed, 236 insertions(+), 238 deletions(-) create mode 100644 src/main/java/redis/clients/util/Pool.java diff --git a/src/main/java/redis/clients/jedis/JedisPool.java b/src/main/java/redis/clients/jedis/JedisPool.java index d0d4c20..d779a51 100644 --- a/src/main/java/redis/clients/jedis/JedisPool.java +++ b/src/main/java/redis/clients/jedis/JedisPool.java @@ -2,110 +2,95 @@ package redis.clients.jedis; import org.apache.commons.pool.BasePoolableObjectFactory; import org.apache.commons.pool.impl.GenericObjectPool; +import org.apache.commons.pool.impl.GenericObjectPool.Config; -public class JedisPool { - private final GenericObjectPool internalPool; +import redis.clients.util.Pool; - public JedisPool(final GenericObjectPool.Config poolConfig, final String host) { - this(poolConfig, host, Protocol.DEFAULT_PORT, Protocol.DEFAULT_TIMEOUT, null); - } +public class JedisPool extends Pool { - public JedisPool(final GenericObjectPool.Config poolConfig,final String host, final int port) { - this(poolConfig, host, port, Protocol.DEFAULT_TIMEOUT, null); - } + public JedisPool(final GenericObjectPool.Config poolConfig, + final String host) { + this(poolConfig, host, Protocol.DEFAULT_PORT, Protocol.DEFAULT_TIMEOUT, + null); + } - public JedisPool(final GenericObjectPool.Config poolConfig,final String host, final int port, final int timeout) { - this(poolConfig, host, port, timeout, null); - } + public JedisPool(final Config poolConfig, final String host, int port, + int timeout, final String password) { + super(poolConfig, new JedisFactory(host, port, timeout, password)); + } - public JedisPool(final GenericObjectPool.Config poolConfig,final String host, final int port, final int timeout, final String password) { - final String lhost; - final int lport; - final int ltimeout; - final String lpassword; - lhost = host; - lport = port; - ltimeout = (timeout > 0) ? timeout : Protocol.DEFAULT_TIMEOUT; - lpassword = password; - - final JedisFactory factory = new JedisFactory(lhost, lport, ltimeout, lpassword); - this.internalPool = new GenericObjectPool(factory, poolConfig); - } + public JedisPool(final GenericObjectPool.Config poolConfig, + final String host, final int port) { + this(poolConfig, host, port, Protocol.DEFAULT_TIMEOUT, null); + } - public JedisPool(final GenericObjectPool.Config poolConfig, final JedisShardInfo shardInfo) { - this(poolConfig, shardInfo.getHost(), shardInfo.getPort(), shardInfo.getTimeout(), shardInfo.getPassword()); - } - - public Jedis getResource() throws Exception { - return (Jedis) internalPool.borrowObject(); - } - - public void returnResource(final Jedis jedis) throws Exception { - internalPool.returnObject(jedis); - } + public JedisPool(final GenericObjectPool.Config poolConfig, + final String host, final int port, final int timeout) { + this(poolConfig, host, port, timeout, null); + } - /** - * PoolableObjectFactory custom impl. - */ - private static class JedisFactory extends BasePoolableObjectFactory { - private final String host; - private final int port; - private final int timeout; - private final String password; + /** + * PoolableObjectFactory custom impl. + */ + private static class JedisFactory extends BasePoolableObjectFactory { + private final String host; + private final int port; + private final int timeout; + private final String password; - public JedisFactory(final String host, final int port, final int timeout, final String password) { - super(); - this.host = host; - this.port = port; - this.timeout = (timeout > 0) ? timeout : -1; - this.password = password; - } + public JedisFactory(final String host, final int port, + final int timeout, final String password) { + super(); + this.host = host; + this.port = port; + this.timeout = (timeout > 0) ? timeout : -1; + this.password = password; + } - @Override - public Object makeObject() throws Exception { - final Jedis jedis; - if (timeout > 0) { - jedis = new Jedis(this.host, this.port, this.timeout); - } else { - jedis = new Jedis(this.host, this.port); - } + @Override + public Object makeObject() throws Exception { + final Jedis jedis; + if (timeout > 0) { + jedis = new Jedis(this.host, this.port, this.timeout); + } else { + jedis = new Jedis(this.host, this.port); + } - jedis.connect(); - if (null != this.password) { - jedis.auth(this.password); - } - return jedis; - } + jedis.connect(); + if (null != this.password) { + jedis.auth(this.password); + } + return jedis; + } - @Override - public void destroyObject(final Object obj) throws Exception { - if(obj instanceof Jedis) { - final Jedis jedis = (Jedis) obj; - if (jedis.isConnected()) { - try { - jedis.quit(); - jedis.disconnect(); - } catch (Exception e) { - - } - } - } - } + @Override + public void destroyObject(final Object obj) throws Exception { + if (obj instanceof Jedis) { + final Jedis jedis = (Jedis) obj; + if (jedis.isConnected()) { + try { + jedis.quit(); + jedis.disconnect(); + } catch (Exception e) { - @Override - public boolean validateObject(final Object obj) { - if(obj instanceof Jedis) { - final Jedis jedis = (Jedis) obj; - try { - return jedis.isConnected() && jedis.ping().equals("PONG"); - } catch (final Exception e) { - return false; - } - } else { - return false; - } - } + } + } + } + } - - } -} + @Override + public boolean validateObject(final Object obj) { + if (obj instanceof Jedis) { + final Jedis jedis = (Jedis) obj; + try { + return jedis.isConnected() && jedis.ping().equals("PONG"); + } catch (final Exception e) { + return false; + } + } else { + return false; + } + } + + } +} \ No newline at end of file diff --git a/src/main/java/redis/clients/jedis/ShardedJedisPool.java b/src/main/java/redis/clients/jedis/ShardedJedisPool.java index 20045ef..f80fd54 100644 --- a/src/main/java/redis/clients/jedis/ShardedJedisPool.java +++ b/src/main/java/redis/clients/jedis/ShardedJedisPool.java @@ -3,80 +3,95 @@ package redis.clients.jedis; import java.util.List; import java.util.regex.Pattern; -import redis.clients.util.FixedResourcePool; +import org.apache.commons.pool.BasePoolableObjectFactory; +import org.apache.commons.pool.impl.GenericObjectPool; + import redis.clients.util.Hashing; +import redis.clients.util.Pool; -public class ShardedJedisPool extends FixedResourcePool { - private List shards; - private Hashing algo = Hashing.MD5; - private Pattern keyTagPattern; - - public ShardedJedisPool(List shards) { - this.shards = shards; +public class ShardedJedisPool extends Pool { + public ShardedJedisPool(final GenericObjectPool.Config poolConfig, + List shards) { + this(poolConfig, shards, Hashing.MD5); } - public ShardedJedisPool(List shards, Hashing algo) { - this.shards = shards; - this.algo = algo; + public ShardedJedisPool(final GenericObjectPool.Config poolConfig, + List shards, Hashing algo) { + this(poolConfig, shards, algo, null); } - public ShardedJedisPool(List shards, Pattern keyTagPattern) { - this.shards = shards; - this.keyTagPattern = keyTagPattern; + public ShardedJedisPool(final GenericObjectPool.Config poolConfig, + List shards, Pattern keyTagPattern) { + this(poolConfig, shards, Hashing.MD5, keyTagPattern); } - public ShardedJedisPool(List shards, Hashing algo, - Pattern keyTagPattern) { - this.shards = shards; - this.algo = algo; - this.keyTagPattern = keyTagPattern; + public ShardedJedisPool(final GenericObjectPool.Config poolConfig, + List shards, Hashing algo, Pattern keyTagPattern) { + super(poolConfig, new ShardedJedisFactory(shards, algo, keyTagPattern)); } - @Override - protected ShardedJedis createResource() { - ShardedJedis jedis = new ShardedJedis(shards, algo, keyTagPattern); - boolean done = false; - while (!done) { - try { - for (JedisShardInfo shard : jedis.getAllShards()) { - if (!shard.getResource().isConnected()) { - shard.getResource().connect(); + /** + * PoolableObjectFactory custom impl. + */ + private static class ShardedJedisFactory extends BasePoolableObjectFactory { + private List shards; + private Hashing algo; + private Pattern keyTagPattern; + + public ShardedJedisFactory(List shards, Hashing algo, + Pattern keyTagPattern) { + this.shards = shards; + this.algo = algo; + this.keyTagPattern = keyTagPattern; + } + + @Override + public Object makeObject() throws Exception { + ShardedJedis jedis = new ShardedJedis(shards, algo, keyTagPattern); + boolean done = false; + while (!done) { + try { + for (JedisShardInfo shard : jedis.getAllShards()) { + if (!shard.getResource().isConnected()) { + shard.getResource().connect(); + } + } + done = true; + } catch (Exception e) { + try { + Thread.sleep(100); + } catch (InterruptedException e1) { } } - done = true; - } catch (Exception e) { + } + return jedis; + } + + @Override + public void destroyObject(final Object obj) throws Exception { + if (obj != null) { try { - Thread.sleep(100); - } catch (InterruptedException e1) { + ((ShardedJedis) obj).disconnect(); + } catch (Exception e) { + } } } - return jedis; - } - @Override - protected void destroyResource(ShardedJedis jedis) { - if (jedis != null) { + @Override + public boolean validateObject(final Object obj) { try { - jedis.disconnect(); - } catch (Exception e) { - - } - } - } - - @Override - protected boolean isResourceValid(ShardedJedis jedis) { - try { - for (JedisShardInfo shard : jedis.getAllShards()) { - if (!shard.getResource().isConnected() - || !shard.getResource().ping().equals("PONG")) { - return false; + ShardedJedis jedis = (ShardedJedis) obj; + for (JedisShardInfo shard : jedis.getAllShards()) { + if (!shard.getResource().isConnected() + || !shard.getResource().ping().equals("PONG")) { + return false; + } } + return true; + } catch (Exception ex) { + return false; } - return true; - } catch (Exception ex) { - return false; } } -} +} \ No newline at end of file diff --git a/src/main/java/redis/clients/util/Pool.java b/src/main/java/redis/clients/util/Pool.java new file mode 100644 index 0000000..3dc2020 --- /dev/null +++ b/src/main/java/redis/clients/util/Pool.java @@ -0,0 +1,30 @@ +package redis.clients.util; + +import org.apache.commons.pool.PoolableObjectFactory; +import org.apache.commons.pool.impl.GenericObjectPool; + +public abstract class Pool { + private final GenericObjectPool internalPool; + + public Pool(final GenericObjectPool.Config poolConfig, + PoolableObjectFactory factory) { + this.internalPool = new GenericObjectPool(factory, poolConfig); + } + + @SuppressWarnings("unchecked") + public T getResource() throws Exception { + return (T) internalPool.borrowObject(); + } + + public void returnResource(final T resource) throws Exception { + internalPool.returnObject(resource); + } + + public void returnBrokenResource(final T resource) throws Exception { + internalPool.invalidateObject(resource); + } + + public void destroy() throws Exception { + internalPool.close(); + } +} \ No newline at end of file diff --git a/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java b/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java index e106499..5af78c4 100644 --- a/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java +++ b/src/test/java/redis/clients/jedis/tests/JedisPoolTest.java @@ -1,8 +1,9 @@ package redis.clients.jedis.tests; -import java.io.IOException; -import java.util.concurrent.TimeoutException; +import java.util.NoSuchElementException; +import org.apache.commons.pool.impl.GenericObjectPool; +import org.apache.commons.pool.impl.GenericObjectPool.Config; import org.junit.Assert; import org.junit.Test; @@ -14,12 +15,9 @@ public class JedisPoolTest extends Assert { private static HostAndPort hnp = HostAndPortUtil.getRedisServers().get(0); @Test - public void checkConnections() throws TimeoutException { - JedisPool pool = new JedisPool(hnp.host, hnp.port, 2000); - pool.setResourcesNumber(10); - pool.init(); - - Jedis jedis = pool.getResource(200); + public void checkConnections() throws Exception { + JedisPool pool = new JedisPool(new Config(), hnp.host, hnp.port, 2000); + Jedis jedis = pool.getResource(); jedis.auth("foobared"); jedis.set("foo", "bar"); assertEquals("bar", jedis.get("foo")); @@ -28,12 +26,9 @@ public class JedisPoolTest extends Assert { } @Test - public void checkConnectionWithDefaultPort() throws TimeoutException { - JedisPool pool = new JedisPool(hnp.host, hnp.port); - pool.setResourcesNumber(10); - pool.init(); - - Jedis jedis = pool.getResource(200); + public void checkConnectionWithDefaultPort() throws Exception { + JedisPool pool = new JedisPool(new Config(), hnp.host, hnp.port); + Jedis jedis = pool.getResource(); jedis.auth("foobared"); jedis.set("foo", "bar"); assertEquals("bar", jedis.get("foo")); @@ -42,17 +37,14 @@ public class JedisPoolTest extends Assert { } @Test - public void checkJedisIsReusedWhenReturned() throws TimeoutException { - JedisPool pool = new JedisPool(hnp.host, hnp.port); - pool.setResourcesNumber(1); - pool.init(); - - Jedis jedis = pool.getResource(200); + public void checkJedisIsReusedWhenReturned() throws Exception { + JedisPool pool = new JedisPool(new Config(), hnp.host, hnp.port); + Jedis jedis = pool.getResource(); jedis.auth("foobared"); jedis.set("foo", "0"); pool.returnResource(jedis); - jedis = pool.getResource(200); + jedis = pool.getResource(); jedis.auth("foobared"); jedis.incr("foo"); pool.returnResource(jedis); @@ -60,35 +52,31 @@ public class JedisPoolTest extends Assert { } @Test - public void checkPoolRepairedWhenJedisIsBroken() throws TimeoutException, - IOException { - JedisPool pool = new JedisPool(hnp.host, hnp.port); - pool.setResourcesNumber(1); - pool.init(); - - Jedis jedis = pool.getResource(200); + public void checkPoolRepairedWhenJedisIsBroken() throws Exception { + JedisPool pool = new JedisPool(new Config(), hnp.host, hnp.port); + Jedis jedis = pool.getResource(); jedis.auth("foobared"); jedis.quit(); pool.returnBrokenResource(jedis); - jedis = pool.getResource(200); + jedis = pool.getResource(); jedis.auth("foobared"); jedis.incr("foo"); pool.returnResource(jedis); pool.destroy(); } - @Test(expected = TimeoutException.class) - public void checkPoolOverflow() throws TimeoutException { - JedisPool pool = new JedisPool(hnp.host, hnp.port); - pool.setResourcesNumber(1); - pool.init(); - - Jedis jedis = pool.getResource(200); + @Test(expected = NoSuchElementException.class) + public void checkPoolOverflow() throws Exception { + Config config = new Config(); + config.maxActive = 1; + config.whenExhaustedAction = GenericObjectPool.WHEN_EXHAUSTED_FAIL; + JedisPool pool = new JedisPool(config, hnp.host, hnp.port); + Jedis jedis = pool.getResource(); jedis.auth("foobared"); jedis.set("foo", "0"); - Jedis newJedis = pool.getResource(200); + Jedis newJedis = pool.getResource(); newJedis.auth("foobared"); newJedis.incr("foo"); } diff --git a/src/test/java/redis/clients/jedis/tests/ShardedJedisPoolTest.java b/src/test/java/redis/clients/jedis/tests/ShardedJedisPoolTest.java index b63eab1..6523591 100644 --- a/src/test/java/redis/clients/jedis/tests/ShardedJedisPoolTest.java +++ b/src/test/java/redis/clients/jedis/tests/ShardedJedisPoolTest.java @@ -4,8 +4,10 @@ import java.io.IOException; import java.net.UnknownHostException; import java.util.ArrayList; import java.util.List; -import java.util.concurrent.TimeoutException; +import java.util.NoSuchElementException; +import org.apache.commons.pool.impl.GenericObjectPool; +import org.apache.commons.pool.impl.GenericObjectPool.Config; import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -42,12 +44,9 @@ public class ShardedJedisPoolTest extends Assert { } @Test - public void checkConnections() throws TimeoutException { - ShardedJedisPool pool = new ShardedJedisPool(shards); - pool.setResourcesNumber(10); - pool.init(); - - ShardedJedis jedis = pool.getResource(200); + public void checkConnections() throws Exception { + ShardedJedisPool pool = new ShardedJedisPool(new Config(), shards); + ShardedJedis jedis = pool.getResource(); jedis.set("foo", "bar"); assertEquals("bar", jedis.get("foo")); pool.returnResource(jedis); @@ -55,12 +54,9 @@ public class ShardedJedisPoolTest extends Assert { } @Test - public void checkConnectionWithDefaultPort() throws TimeoutException { - ShardedJedisPool pool = new ShardedJedisPool(shards); - pool.setResourcesNumber(1); - pool.init(); - - ShardedJedis jedis = pool.getResource(200); + public void checkConnectionWithDefaultPort() throws Exception { + ShardedJedisPool pool = new ShardedJedisPool(new Config(), shards); + ShardedJedis jedis = pool.getResource(); jedis.set("foo", "bar"); assertEquals("bar", jedis.get("foo")); pool.returnResource(jedis); @@ -68,48 +64,43 @@ public class ShardedJedisPoolTest extends Assert { } @Test - public void checkJedisIsReusedWhenReturned() throws TimeoutException { - ShardedJedisPool pool = new ShardedJedisPool(shards); - pool.setResourcesNumber(1); - pool.init(); - - ShardedJedis jedis = pool.getResource(200); + public void checkJedisIsReusedWhenReturned() throws Exception { + ShardedJedisPool pool = new ShardedJedisPool(new Config(), shards); + ShardedJedis jedis = pool.getResource(); jedis.set("foo", "0"); pool.returnResource(jedis); - jedis = pool.getResource(200); + jedis = pool.getResource(); jedis.incr("foo"); pool.returnResource(jedis); pool.destroy(); } @Test - public void checkPoolRepairedWhenJedisIsBroken() throws TimeoutException, - IOException { - ShardedJedisPool pool = new ShardedJedisPool(shards); - pool.setResourcesNumber(1); - pool.init(); - - ShardedJedis jedis = pool.getResource(200); + public void checkPoolRepairedWhenJedisIsBroken() throws Exception { + ShardedJedisPool pool = new ShardedJedisPool(new Config(), shards); + ShardedJedis jedis = pool.getResource(); jedis.disconnect(); pool.returnBrokenResource(jedis); - jedis = pool.getResource(200); + jedis = pool.getResource(); jedis.incr("foo"); pool.returnResource(jedis); pool.destroy(); } - @Test(expected = TimeoutException.class) - public void checkPoolOverflow() throws TimeoutException { - ShardedJedisPool pool = new ShardedJedisPool(shards); - pool.setResourcesNumber(1); - pool.init(); + @Test(expected = NoSuchElementException.class) + public void checkPoolOverflow() throws Exception { + Config config = new Config(); + config.maxActive = 1; + config.whenExhaustedAction = GenericObjectPool.WHEN_EXHAUSTED_FAIL; - ShardedJedis jedis = pool.getResource(200); + ShardedJedisPool pool = new ShardedJedisPool(config, shards); + + ShardedJedis jedis = pool.getResource(); jedis.set("foo", "0"); - ShardedJedis newJedis = pool.getResource(200); + ShardedJedis newJedis = pool.getResource(); newJedis.incr("foo"); } } \ No newline at end of file diff --git a/src/test/java/redis/clients/jedis/tests/benchmark/PoolBenchmark.java b/src/test/java/redis/clients/jedis/tests/benchmark/PoolBenchmark.java index e02742f..ae1ffc3 100644 --- a/src/test/java/redis/clients/jedis/tests/benchmark/PoolBenchmark.java +++ b/src/test/java/redis/clients/jedis/tests/benchmark/PoolBenchmark.java @@ -1,29 +1,21 @@ package redis.clients.jedis.tests.benchmark; -import java.io.IOException; -import java.net.UnknownHostException; import java.util.ArrayList; import java.util.List; -import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicInteger; -import java.util.logging.Level; -import java.util.logging.Logger; + +import org.apache.commons.pool.impl.GenericObjectPool.Config; import redis.clients.jedis.Jedis; import redis.clients.jedis.JedisPool; import redis.clients.jedis.tests.HostAndPortUtil; import redis.clients.jedis.tests.HostAndPortUtil.HostAndPort; -import redis.clients.util.FixedResourcePool; public class PoolBenchmark { private static HostAndPort hnp = HostAndPortUtil.getRedisServers().get(0); private static final int TOTAL_OPERATIONS = 100000; - public static void main(String[] args) throws UnknownHostException, - IOException, TimeoutException, InterruptedException { - Logger logger = Logger.getLogger(FixedResourcePool.class.getName()); - logger.setLevel(Level.OFF); - + public static void main(String[] args) throws Exception { Jedis j = new Jedis(hnp.host, hnp.port); j.connect(); j.auth("foobared"); @@ -37,12 +29,9 @@ public class PoolBenchmark { System.out.println(((1000 * 2 * TOTAL_OPERATIONS) / elapsed) + " ops"); } - private static void withPool() throws InterruptedException { - final JedisPool pool = new JedisPool(hnp.host, hnp.port, 2000, - "foobared"); - pool.setResourcesNumber(50); - pool.setDefaultPoolWait(1000000); - pool.init(); + private static void withPool() throws Exception { + final JedisPool pool = new JedisPool(new Config(), hnp.host, hnp.port, + 2000, "foobared"); List tds = new ArrayList(); final AtomicInteger ind = new AtomicInteger();