Return-Path: X-Original-To: apmail-commons-commits-archive@minotaur.apache.org Delivered-To: apmail-commons-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id C939B10336 for ; Sat, 31 May 2014 00:54:55 +0000 (UTC) Received: (qmail 86603 invoked by uid 500); 31 May 2014 00:54:55 -0000 Delivered-To: apmail-commons-commits-archive@commons.apache.org Received: (qmail 86530 invoked by uid 500); 31 May 2014 00:54:55 -0000 Mailing-List: contact commits-help@commons.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@commons.apache.org Delivered-To: mailing list commits@commons.apache.org Received: (qmail 86521 invoked by uid 99); 31 May 2014 00:54:55 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Sat, 31 May 2014 00:54:55 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=5.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Sat, 31 May 2014 00:54:53 +0000 Received: from eris.apache.org (localhost [127.0.0.1]) by eris.apache.org (Postfix) with ESMTP id 5C6B023889B9; Sat, 31 May 2014 00:54:28 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1598794 - in /commons/proper/pool/trunk/src: changes/ main/java/org/apache/commons/pool2/impl/ test/java/org/apache/commons/pool2/impl/ Date: Sat, 31 May 2014 00:54:27 -0000 To: commits@commons.apache.org From: psteitz@apache.org X-Mailer: svnmailer-1.0.9 Message-Id: <20140531005428.5C6B023889B9@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org Author: psteitz Date: Sat May 31 00:54:27 2014 New Revision: 1598794 URL: http://svn.apache.org/r1598794 Log: Made fairness configurable for GOP, GKOP. Also made 'testBorrowObjectFairness' more rigorous (was succeeding despite 2.x pools not being fair). JIRA: POOL-262. Modified: commons/proper/pool/trunk/src/changes/changes.xml commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/BaseObjectPoolConfig.java commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPoolMXBean.java commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPoolMXBean.java commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/InterruptibleReentrantLock.java commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/LinkedBlockingDeque.java commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java Modified: commons/proper/pool/trunk/src/changes/changes.xml URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/changes/changes.xml?rev=1598794&r1=1598793&r2=1598794&view=diff ============================================================================== --- commons/proper/pool/trunk/src/changes/changes.xml (original) +++ commons/proper/pool/trunk/src/changes/changes.xml Sat May 31 00:54:27 2014 @@ -45,6 +45,9 @@ The type attribute can be add,u + + Made fairness configurable for GenericObjectPool, GenericKeyedObjectPool. + Improve performance of statistics collection for pools that extend BaseGenericObjectPool. Modified: commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java?rev=1598794&r1=1598793&r2=1598794&view=diff ============================================================================== --- commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java (original) +++ commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java Sat May 31 00:54:27 2014 @@ -65,6 +65,7 @@ public abstract class BaseGenericObjectP private volatile long maxWaitMillis = BaseObjectPoolConfig.DEFAULT_MAX_WAIT_MILLIS; private volatile boolean lifo = BaseObjectPoolConfig.DEFAULT_LIFO; + private final boolean fairness; private volatile boolean testOnCreate = BaseObjectPoolConfig.DEFAULT_TEST_ON_CREATE; private volatile boolean testOnBorrow = @@ -137,6 +138,7 @@ public abstract class BaseGenericObjectP // save the current CCL to be used later by the evictor Thread factoryClassLoader = Thread.currentThread().getContextClassLoader(); + fairness = config.getFairness(); } @@ -249,6 +251,17 @@ public abstract class BaseGenericObjectP public final boolean getLifo() { return lifo; } + + /** + * Returns whether or not the pool serves threads waiting to borrow objects fairly. + * True means that waiting threads are served as if waiting in a FIFO queue. + * + * @return true if waiting threads are to be served + * by the pool in arrival order + */ + public final boolean getFairness() { + return fairness; + } /** * Sets whether the pool has LIFO (last in, first out) behaviour with Modified: commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/BaseObjectPoolConfig.java URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/BaseObjectPoolConfig.java?rev=1598794&r1=1598793&r2=1598794&view=diff ============================================================================== --- commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/BaseObjectPoolConfig.java (original) +++ commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/BaseObjectPoolConfig.java Sat May 31 00:54:27 2014 @@ -35,6 +35,13 @@ public abstract class BaseObjectPoolConf * @see GenericKeyedObjectPool#getLifo() */ public static final boolean DEFAULT_LIFO = true; + + /** + * The default value for the {@code fairness} configuration attribute. + * @see GenericObjectPool#getFairness() + * @see GenericKeyedObjectPool#getFairness() + */ + public static final boolean DEFAULT_FAIRNESS = false; /** * The default value for the {@code maxWait} configuration attribute. @@ -148,6 +155,8 @@ public abstract class BaseObjectPoolConf private boolean lifo = DEFAULT_LIFO; + + private boolean fairness = DEFAULT_FAIRNESS; private long maxWaitMillis = DEFAULT_MAX_WAIT_MILLIS; @@ -196,6 +205,20 @@ public abstract class BaseObjectPoolConf public boolean getLifo() { return lifo; } + + /** + * Get the value for the {@code fairness} configuration attribute for pools + * created with this configuration instance. + * + * @return The current setting of {@code fairness} for this configuration + * instance + * + * @see GenericObjectPool#getFairness() + * @see GenericKeyedObjectPool#getFairness() + */ + public boolean getFairness() { + return fairness; + } /** * Set the value for the {@code lifo} configuration attribute for pools @@ -210,6 +233,20 @@ public abstract class BaseObjectPoolConf public void setLifo(boolean lifo) { this.lifo = lifo; } + + /** + * Set the value for the {@code fairness} configuration attribute for pools + * created with this configuration instance. + * + * @param fairness The new setting of {@code fairness} + * for this configuration instance + * + * @see GenericObjectPool#getFairness() + * @see GenericKeyedObjectPool#getFairness() + */ + public void setFairness(boolean fairness) { + this.fairness = fairness; + } /** * Get the value for the {@code maxWait} configuration attribute for pools Modified: commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java?rev=1598794&r1=1598793&r2=1598794&view=diff ============================================================================== --- commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java (original) +++ commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java Sat May 31 00:54:27 2014 @@ -106,6 +106,7 @@ public class GenericKeyedObjectPool throw new IllegalArgumentException("factory may not be null"); } this.factory = factory; + this.fairness = config.getFairness(); setConfig(config); @@ -1082,7 +1083,7 @@ public class GenericKeyedObjectPool lock.lock(); objectDeque = poolMap.get(k); if (objectDeque == null) { - objectDeque = new ObjectDeque(); + objectDeque = new ObjectDeque(fairness); objectDeque.getNumInterested().incrementAndGet(); // NOTE: Keys must always be added to both poolMap and // poolKeyList at the same time while protected by @@ -1398,8 +1399,7 @@ public class GenericKeyedObjectPool */ private class ObjectDeque { - private final LinkedBlockingDeque> idleObjects = - new LinkedBlockingDeque>(); + private final LinkedBlockingDeque> idleObjects; /* * Number of instances created - number destroyed. @@ -1424,6 +1424,16 @@ public class GenericKeyedObjectPool private final AtomicLong numInterested = new AtomicLong(0); /** + * Create a new ObjecDeque with the given fairness policy. + * @param fairness true means client threads waiting to borrow / return instances + * will be served as if waiting in a FIFO queue. + */ + public ObjectDeque(boolean fairness) { + idleObjects = new LinkedBlockingDeque>(fairness); + + } + + /** * Obtain the idle objects for the current key. * * @return The idle objects @@ -1469,6 +1479,7 @@ public class GenericKeyedObjectPool private volatile int maxTotalPerKey = GenericKeyedObjectPoolConfig.DEFAULT_MAX_TOTAL_PER_KEY; private final KeyedPooledObjectFactory factory; + private final boolean fairness; //--- internal attributes -------------------------------------------------- Modified: commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPoolMXBean.java URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPoolMXBean.java?rev=1598794&r1=1598793&r2=1598794&view=diff ============================================================================== --- commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPoolMXBean.java (original) +++ commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPoolMXBean.java Sat May 31 00:54:27 2014 @@ -43,6 +43,11 @@ public interface GenericKeyedObjectPoolM */ boolean getBlockWhenExhausted(); /** + * See {@link GenericKeyedObjectPool#getFairness()} + * @return See {@link GenericKeyedObjectPool#getFairness()} + */ + boolean getFairness(); + /** * See {@link GenericKeyedObjectPool#getLifo()} * @return See {@link GenericKeyedObjectPool#getLifo()} */ Modified: commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java?rev=1598794&r1=1598793&r2=1598794&view=diff ============================================================================== --- commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java (original) +++ commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java Sat May 31 00:54:27 2014 @@ -111,6 +111,8 @@ public class GenericObjectPool extend throw new IllegalArgumentException("factory may not be null"); } this.factory = factory; + + idleObjects = new LinkedBlockingDeque>(config.getFairness()); setConfig(config); @@ -1096,8 +1098,7 @@ public class GenericObjectPool extend * {@link #_maxActive} objects created at any one time. */ private final AtomicLong createCount = new AtomicLong(0); - private final LinkedBlockingDeque> idleObjects = - new LinkedBlockingDeque>(); + private final LinkedBlockingDeque> idleObjects; // JMX specific attributes private static final String ONAME_BASE = Modified: commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPoolMXBean.java URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPoolMXBean.java?rev=1598794&r1=1598793&r2=1598794&view=diff ============================================================================== --- commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPoolMXBean.java (original) +++ commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPoolMXBean.java Sat May 31 00:54:27 2014 @@ -43,6 +43,11 @@ public interface GenericObjectPoolMXBean * See {@link GenericObjectPool#getLifo()} * @return See {@link GenericObjectPool#getLifo()} */ + boolean getFairness(); + /** + * See {@link GenericObjectPool#getFairness()} + * @return See {@link GenericObjectPool#getFairness()} + */ boolean getLifo(); /** * See {@link GenericObjectPool#getMaxIdle()} Modified: commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/InterruptibleReentrantLock.java URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/InterruptibleReentrantLock.java?rev=1598794&r1=1598793&r2=1598794&view=diff ============================================================================== --- commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/InterruptibleReentrantLock.java (original) +++ commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/InterruptibleReentrantLock.java Sat May 31 00:54:27 2014 @@ -34,6 +34,16 @@ class InterruptibleReentrantLock extends private static final long serialVersionUID = 1L; /** + * Create a new InterruptibleReentrantLock with the given fairness policy. + * + * @param fairness true means threads should acquire contended locks as if + * waiting in a FIFO queue + */ + public InterruptibleReentrantLock(boolean fairness) { + super(fairness); + } + + /** * Interrupt the threads that are waiting on a specific condition * * @param condition the condition on which the threads are waiting. Modified: commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/LinkedBlockingDeque.java URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/LinkedBlockingDeque.java?rev=1598794&r1=1598793&r2=1598794&view=diff ============================================================================== --- commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/LinkedBlockingDeque.java (original) +++ commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/LinkedBlockingDeque.java Sat May 31 00:54:27 2014 @@ -147,14 +147,13 @@ class LinkedBlockingDeque extends Abs private final int capacity; /** Main lock guarding all access */ - private final InterruptibleReentrantLock lock = - new InterruptibleReentrantLock(); + private final InterruptibleReentrantLock lock; /** Condition for waiting takes */ - private final Condition notEmpty = lock.newCondition(); + private final Condition notEmpty; /** Condition for waiting puts */ - private final Condition notFull = lock.newCondition(); + private final Condition notFull; /** * Creates a {@code LinkedBlockingDeque} with a capacity of @@ -163,6 +162,16 @@ class LinkedBlockingDeque extends Abs public LinkedBlockingDeque() { this(Integer.MAX_VALUE); } + + /** + * Creates a {@code LinkedBlockingDeque} with a capacity of + * {@link Integer#MAX_VALUE} and the given fairness policy. + * @param fairness true means threads waiting on the deque should be served + * as if waiting in a FIFO request queue + */ + public LinkedBlockingDeque(boolean fairness) { + this(Integer.MAX_VALUE, fairness); + } /** * Creates a {@code LinkedBlockingDeque} with the given (fixed) capacity. @@ -171,8 +180,24 @@ class LinkedBlockingDeque extends Abs * @throws IllegalArgumentException if {@code capacity} is less than 1 */ public LinkedBlockingDeque(int capacity) { + this(capacity, false); + } + + /** + * Creates a {@code LinkedBlockingDeque} with the given (fixed) capacity + * and fairness policy. + * + * @param capacity the capacity of this deque + * @param fairness true means threads waiting on the deque should be served + * as if waiting in a FIFO request queue + * @throws IllegalArgumentException if {@code capacity} is less than 1 + */ + public LinkedBlockingDeque(int capacity, boolean fairness) { if (capacity <= 0) throw new IllegalArgumentException(); this.capacity = capacity; + lock = new InterruptibleReentrantLock(fairness); + notEmpty = lock.newCondition(); + notFull = lock.newCondition(); } /** Modified: commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java?rev=1598794&r1=1598793&r2=1598794&view=diff ============================================================================== --- commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java (original) +++ commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java Sat May 31 00:54:27 2014 @@ -1029,6 +1029,68 @@ public class TestGenericKeyedObjectPool } } } + + /* + * Note: This test relies on timing for correct execution. There *should* be + * enough margin for this to work correctly on most (all?) systems but be + * aware of this if you see a failure of this test. + */ + @SuppressWarnings({ + "rawtypes", "unchecked" + }) + @Test(timeout=60000) + public void testBorrowObjectFairness() throws Exception { + + int numThreads = 40; + int maxTotal = 40; + + GenericKeyedObjectPoolConfig config = new GenericKeyedObjectPoolConfig(); + config.setMaxTotalPerKey(maxTotal); + config.setFairness(true); + config.setLifo(false); + config.setMaxIdlePerKey(maxTotal); + + pool = new GenericKeyedObjectPool(factory, config); + + // Exhaust the pool + String[] objects = new String[maxTotal]; + for (int i = 0; i < maxTotal; i++) { + objects[i] = pool.borrowObject("0"); + } + + // Start and park threads waiting to borrow objects + TestThread[] threads = new TestThread[numThreads]; + for(int i=0;i implements Runnable { private final java.util.Random _random = new java.util.Random(); - // Thread config items + /** GKOP to hit */ private final KeyedObjectPool _pool; + /** number of borrow/return iterations */ private final int _iter; - private final int _delay; + /** delay before borrow */ + private final int _startDelay; + /** delay before return */ + private final int _holdTime; + /** whether or not delays are random (with max = configured values) */ + private final boolean _randomDelay; + /** expected object */ + private final T _expectedObject; + /** key used in borrow / return sequence - null means random */ + private final String _key; private volatile boolean _complete = false; private volatile boolean _failed = false; private volatile Exception _exception; public TestThread(KeyedObjectPool pool) { - this(pool, 100, 50); + this(pool, 100, 50, 50, true, null, null); } public TestThread(KeyedObjectPool pool, int iter) { - this(pool, iter, 50); + this(pool, iter, 50, 50, true, null, null); } - + public TestThread(KeyedObjectPool pool, int iter, int delay) { + this(pool, iter, delay, delay, true, null, null); + } + + public TestThread(KeyedObjectPool pool, int iter, int startDelay, + int holdTime, boolean randomDelay, T expectedObject, String key) { _pool = pool; _iter = iter; - _delay = delay; + _startDelay = startDelay; + _holdTime = holdTime; + _randomDelay = randomDelay; + _expectedObject = expectedObject; + _key = key; + } public boolean complete() { @@ -1637,9 +1719,9 @@ public class TestGenericKeyedObjectPool @Override public void run() { for(int i=0;i<_iter;i++) { - String key = String.valueOf(_random.nextInt(3)); + String key = _key == null ? String.valueOf(_random.nextInt(3)) : _key; try { - Thread.sleep(_random.nextInt(_delay)); + Thread.sleep(_randomDelay ? _random.nextInt(_startDelay) : _startDelay); } catch(InterruptedException e) { // ignored } @@ -1652,9 +1734,16 @@ public class TestGenericKeyedObjectPool _complete = true; break; } + + if (_expectedObject != null && !_expectedObject.equals(obj)) { + _exception = new Exception("Expected: "+_expectedObject+ " found: "+obj); + _failed = true; + _complete = true; + break; + } try { - Thread.sleep(_random.nextInt(_delay)); + Thread.sleep(_randomDelay ? _random.nextInt(_holdTime) : _holdTime); } catch(InterruptedException e) { // ignored } Modified: commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java?rev=1598794&r1=1598793&r2=1598794&view=diff ============================================================================== --- commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java (original) +++ commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java Sat May 31 00:54:27 2014 @@ -1917,8 +1917,8 @@ public class TestGenericObjectPool exten /** * @param validateLatency the validateLatency to set */ - public void setDelayLatency(long delayLatency) { - this.validateLatency = delayLatency; + public void setValidateLatency(long validateLatency) { + this.validateLatency = validateLatency; } } @@ -1941,29 +1941,43 @@ public class TestGenericObjectPool exten "rawtypes", "unchecked" }) @Test(timeout=60000) - public void testBorrowObjectFairness() { + public void testBorrowObjectFairness() throws Exception { + + int numThreads = 40; + int maxTotal = 40; - // Config - int numThreads = 30; - int maxTotal = 10; - - pool.setMaxTotal(maxTotal); - pool.setBlockWhenExhausted(true); - pool.setTimeBetweenEvictionRunsMillis(-1); + GenericObjectPoolConfig config = new GenericObjectPoolConfig(); + config.setMaxTotal(maxTotal); + config.setMaxIdle(maxTotal); + config.setFairness(true); + config.setLifo(false); + + pool = new GenericObjectPool(factory, config); + + // Exhaust the pool + String[] objects = new String[maxTotal]; + for (int i = 0; i < maxTotal; i++) { + objects[i] = pool.borrowObject(); + } - // Start threads to borrow objects + // Start and park threads waiting to borrow objects TestThread[] threads = new TestThread[numThreads]; for(int i=0;i