replace custom pool implementation with apache's

2010-11-21 18:16:31 -03:00
parent e7582644b1
commit 71eb4c5b4a
6 changed files with 236 additions and 238 deletions
--- 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<Jedis> {

-	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;
+            }
+        }
+
+    }
+}
--- 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<ShardedJedis> {
-    private List<JedisShardInfo> shards;
-    private Hashing algo = Hashing.MD5;
-    private Pattern keyTagPattern;
-
-    public ShardedJedisPool(List<JedisShardInfo> shards) {
-        this.shards = shards;
+public class ShardedJedisPool extends Pool<ShardedJedis> {
+    public ShardedJedisPool(final GenericObjectPool.Config poolConfig,
+            List<JedisShardInfo> shards) {
+        this(poolConfig, shards, Hashing.MD5);
    }

-    public ShardedJedisPool(List<JedisShardInfo> shards, Hashing algo) {
-        this.shards = shards;
-        this.algo = algo;
+    public ShardedJedisPool(final GenericObjectPool.Config poolConfig,
+            List<JedisShardInfo> shards, Hashing algo) {
+        this(poolConfig, shards, algo, null);
    }

-    public ShardedJedisPool(List<JedisShardInfo> shards, Pattern keyTagPattern) {
-        this.shards = shards;
-        this.keyTagPattern = keyTagPattern;
+    public ShardedJedisPool(final GenericObjectPool.Config poolConfig,
+            List<JedisShardInfo> shards, Pattern keyTagPattern) {
+        this(poolConfig, shards, Hashing.MD5, keyTagPattern);
    }

-    public ShardedJedisPool(List<JedisShardInfo> shards, Hashing algo,
-            Pattern keyTagPattern) {
-        this.shards = shards;
-        this.algo = algo;
-        this.keyTagPattern = keyTagPattern;
+    public ShardedJedisPool(final GenericObjectPool.Config poolConfig,
+            List<JedisShardInfo> 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<JedisShardInfo> shards;
+        private Hashing algo;
+        private Pattern keyTagPattern;
+
+        public ShardedJedisFactory(List<JedisShardInfo> 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;
        }
    }
-}
+}
--- a/src/main/java/redis/clients/util/Pool.java
+++ 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<T> {
+    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();
+    }
+}
--- 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");
    }
--- 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");
    }
 }
--- 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<Thread> tds = new ArrayList<Thread>();

        final AtomicInteger ind = new AtomicInteger();