You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Phil Steitz <ph...@gmail.com> on 2011/05/18 19:48:48 UTC
Re: svn commit: r1104601 - /commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
On 5/17/11 3:21 PM, markt@apache.org wrote:
> Author: markt
> Date: Tue May 17 22:21:43 2011
> New Revision: 1104601
>
> URL: http://svn.apache.org/viewvc?rev=1104601&view=rev
> Log:
> Make Phil's suggested changes to (hopefully) fix issues when testing with commons-performance
I have not rerun all of the commons-performance tests yet with the
new code, but the test case that I just committed failed before this
change and now succeeds.
Phil
> Modified:
> commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
>
> Modified: commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
> URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java?rev=1104601&r1=1104600&r2=1104601&view=diff
> ==============================================================================
> --- commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java (original)
> +++ commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java Tue May 17 22:21:43 2011
> @@ -1078,7 +1078,7 @@ public class GenericKeyedObjectPool<K,T>
> _factory.activateObject(key, p.getObject());
> } catch (Exception e) {
> try {
> - destroy(key, p);
> + destroy(key, p, true);
> } catch (Exception e1) {
> // Ignore - activation failure is more important
> }
> @@ -1100,7 +1100,7 @@ public class GenericKeyedObjectPool<K,T>
> }
> if (!validate) {
> try {
> - destroy(key, p);
> + destroy(key, p, true);
> } catch (Exception e) {
> // Ignore - validation failure is more important
> }
> @@ -1155,7 +1155,7 @@ public class GenericKeyedObjectPool<K,T>
> if (getTestOnReturn()) {
> if (!_factory.validateObject(key, t)) {
> try {
> - destroy(key, p);
> + destroy(key, p, true);
> } catch (Exception e) {
> // TODO - Ignore?
> }
> @@ -1167,7 +1167,7 @@ public class GenericKeyedObjectPool<K,T>
> _factory.passivateObject(key, t);
> } catch (Exception e1) {
> try {
> - destroy(key, p);
> + destroy(key, p, true);
> } catch (Exception e) {
> // TODO - Ignore?
> }
> @@ -1184,7 +1184,7 @@ public class GenericKeyedObjectPool<K,T>
>
> if (isClosed() || maxIdle > -1 && maxIdle <= idleObjects.size()) {
> try {
> - destroy(key, p);
> + destroy(key, p, true);
> } catch (Exception e) {
> // TODO - Ignore?
> }
> @@ -1200,8 +1200,8 @@ public class GenericKeyedObjectPool<K,T>
>
> /**
> * {@inheritDoc}
> - * <p>Activation of this method decrements the active count associated with the given keyed pool
> - * and attempts to destroy <code>obj.</code></p>
> + * <p>Activation of this method decrements the active count associated with
> + * the given keyed pool and attempts to destroy <code>obj.</code></p>
> *
> * @param key pool key
> * @param obj instance to invalidate
> @@ -1217,7 +1217,7 @@ public class GenericKeyedObjectPool<K,T>
> throw new IllegalStateException(
> "Object not currently part of this pool");
> }
> - destroy(key, p);
> + destroy(key, p, true);
> }
>
>
> @@ -1247,7 +1247,8 @@ public class GenericKeyedObjectPool<K,T>
>
>
> /**
> - * Clears the specified pool, removing all pooled instances corresponding to the given <code>key</code>.
> + * Clears the specified pool, removing all pooled instances corresponding
> + * to the given <code>key</code>.
> *
> * @param key the key to clear
> */
> @@ -1268,7 +1269,7 @@ public class GenericKeyedObjectPool<K,T>
>
> while (p != null) {
> try {
> - destroy(key, p);
> + destroy(key, p, true);
> } catch (Exception e) {
> // TODO - Ignore?
> }
> @@ -1417,7 +1418,7 @@ public class GenericKeyedObjectPool<K,T>
> K key = entry.getValue();
> PooledObject<T> p = entry.getKey();
> try {
> - destroy(key, p);
> + destroy(key, p, false);
> } catch (Exception e) {
> // TODO - Ignore?
> }
> @@ -1506,7 +1507,7 @@ public class GenericKeyedObjectPool<K,T>
> }
>
> if (idleEvictTime < underTest.getIdleTimeMillis()) {
> - destroy(evictionKey, underTest);
> + destroy(evictionKey, underTest, true);
> } else {
> if (testWhileIdle) {
> boolean active = false;
> @@ -1515,18 +1516,18 @@ public class GenericKeyedObjectPool<K,T>
> underTest.getObject());
> active = true;
> } catch (Exception e) {
> - destroy(evictionKey, underTest);
> + destroy(evictionKey, underTest, true);
> }
> if (active) {
> if (!_factory.validateObject(evictionKey,
> underTest.getObject())) {
> - destroy(evictionKey, underTest);
> + destroy(evictionKey, underTest, true);
> } else {
> try {
> _factory.passivateObject(evictionKey,
> underTest.getObject());
> } catch (Exception e) {
> - destroy(evictionKey, underTest);
> + destroy(evictionKey, underTest, true);
> }
> }
> }
> @@ -1585,20 +1586,24 @@ public class GenericKeyedObjectPool<K,T>
> return p;
> }
>
> - private void destroy(K key, PooledObject<T> toDestory) throws Exception {
> + private void destroy(K key, PooledObject<T> toDestory, boolean always)
> + throws Exception {
>
> register(key);
>
> try {
> ObjectDeque<T> objectDeque = poolMap.get(key);
> - objectDeque.getIdleObjects().remove(toDestory);
> - objectDeque.getAllObjects().remove(toDestory.getObject());
> -
> - try {
> - _factory.destroyObject(key, toDestory.getObject());
> - } finally {
> - objectDeque.getCreateCount().decrementAndGet();
> - numTotal.decrementAndGet();
> + boolean isIdle = objectDeque.getIdleObjects().remove(toDestory);
> +
> + if (isIdle || always) {
> + objectDeque.getAllObjects().remove(toDestory.getObject());
> +
> + try {
> + _factory.destroyObject(key, toDestory.getObject());
> + } finally {
> + objectDeque.getCreateCount().decrementAndGet();
> + numTotal.decrementAndGet();
> + }
> }
> } finally {
> deregister(key);
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1104601 - /commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
Posted by Phil Steitz <ph...@gmail.com>.
On 5/18/11 10:48 AM, Phil Steitz wrote:
> On 5/17/11 3:21 PM, markt@apache.org wrote:
>> Author: markt
>> Date: Tue May 17 22:21:43 2011
>> New Revision: 1104601
>>
>> URL: http://svn.apache.org/viewvc?rev=1104601&view=rev
>> Log:
>> Make Phil's suggested changes to (hopefully) fix issues when testing with commons-performance
> I have not rerun all of the commons-performance tests yet with the
> new code, but the test case that I just committed failed before this
> change and now succeeds.
One more comment on this. The itemsToRemove counter in clearOldest
should probably not be decremented when a checked out instance is
encountered. I am personally OK leaving as is, though, because I
don't think clearOldest (actually borrowObject) makes a promise of
precision at that level and unless destroy has a lot of latency,
occurrence should be rare.
Phil
> Phil
>> Modified:
>> commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
>>
>> Modified: commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
>> URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java?rev=1104601&r1=1104600&r2=1104601&view=diff
>> ==============================================================================
>> --- commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java (original)
>> +++ commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java Tue May 17 22:21:43 2011
>> @@ -1078,7 +1078,7 @@ public class GenericKeyedObjectPool<K,T>
>> _factory.activateObject(key, p.getObject());
>> } catch (Exception e) {
>> try {
>> - destroy(key, p);
>> + destroy(key, p, true);
>> } catch (Exception e1) {
>> // Ignore - activation failure is more important
>> }
>> @@ -1100,7 +1100,7 @@ public class GenericKeyedObjectPool<K,T>
>> }
>> if (!validate) {
>> try {
>> - destroy(key, p);
>> + destroy(key, p, true);
>> } catch (Exception e) {
>> // Ignore - validation failure is more important
>> }
>> @@ -1155,7 +1155,7 @@ public class GenericKeyedObjectPool<K,T>
>> if (getTestOnReturn()) {
>> if (!_factory.validateObject(key, t)) {
>> try {
>> - destroy(key, p);
>> + destroy(key, p, true);
>> } catch (Exception e) {
>> // TODO - Ignore?
>> }
>> @@ -1167,7 +1167,7 @@ public class GenericKeyedObjectPool<K,T>
>> _factory.passivateObject(key, t);
>> } catch (Exception e1) {
>> try {
>> - destroy(key, p);
>> + destroy(key, p, true);
>> } catch (Exception e) {
>> // TODO - Ignore?
>> }
>> @@ -1184,7 +1184,7 @@ public class GenericKeyedObjectPool<K,T>
>>
>> if (isClosed() || maxIdle > -1 && maxIdle <= idleObjects.size()) {
>> try {
>> - destroy(key, p);
>> + destroy(key, p, true);
>> } catch (Exception e) {
>> // TODO - Ignore?
>> }
>> @@ -1200,8 +1200,8 @@ public class GenericKeyedObjectPool<K,T>
>>
>> /**
>> * {@inheritDoc}
>> - * <p>Activation of this method decrements the active count associated with the given keyed pool
>> - * and attempts to destroy <code>obj.</code></p>
>> + * <p>Activation of this method decrements the active count associated with
>> + * the given keyed pool and attempts to destroy <code>obj.</code></p>
>> *
>> * @param key pool key
>> * @param obj instance to invalidate
>> @@ -1217,7 +1217,7 @@ public class GenericKeyedObjectPool<K,T>
>> throw new IllegalStateException(
>> "Object not currently part of this pool");
>> }
>> - destroy(key, p);
>> + destroy(key, p, true);
>> }
>>
>>
>> @@ -1247,7 +1247,8 @@ public class GenericKeyedObjectPool<K,T>
>>
>>
>> /**
>> - * Clears the specified pool, removing all pooled instances corresponding to the given <code>key</code>.
>> + * Clears the specified pool, removing all pooled instances corresponding
>> + * to the given <code>key</code>.
>> *
>> * @param key the key to clear
>> */
>> @@ -1268,7 +1269,7 @@ public class GenericKeyedObjectPool<K,T>
>>
>> while (p != null) {
>> try {
>> - destroy(key, p);
>> + destroy(key, p, true);
>> } catch (Exception e) {
>> // TODO - Ignore?
>> }
>> @@ -1417,7 +1418,7 @@ public class GenericKeyedObjectPool<K,T>
>> K key = entry.getValue();
>> PooledObject<T> p = entry.getKey();
>> try {
>> - destroy(key, p);
>> + destroy(key, p, false);
>> } catch (Exception e) {
>> // TODO - Ignore?
>> }
>> @@ -1506,7 +1507,7 @@ public class GenericKeyedObjectPool<K,T>
>> }
>>
>> if (idleEvictTime < underTest.getIdleTimeMillis()) {
>> - destroy(evictionKey, underTest);
>> + destroy(evictionKey, underTest, true);
>> } else {
>> if (testWhileIdle) {
>> boolean active = false;
>> @@ -1515,18 +1516,18 @@ public class GenericKeyedObjectPool<K,T>
>> underTest.getObject());
>> active = true;
>> } catch (Exception e) {
>> - destroy(evictionKey, underTest);
>> + destroy(evictionKey, underTest, true);
>> }
>> if (active) {
>> if (!_factory.validateObject(evictionKey,
>> underTest.getObject())) {
>> - destroy(evictionKey, underTest);
>> + destroy(evictionKey, underTest, true);
>> } else {
>> try {
>> _factory.passivateObject(evictionKey,
>> underTest.getObject());
>> } catch (Exception e) {
>> - destroy(evictionKey, underTest);
>> + destroy(evictionKey, underTest, true);
>> }
>> }
>> }
>> @@ -1585,20 +1586,24 @@ public class GenericKeyedObjectPool<K,T>
>> return p;
>> }
>>
>> - private void destroy(K key, PooledObject<T> toDestory) throws Exception {
>> + private void destroy(K key, PooledObject<T> toDestory, boolean always)
>> + throws Exception {
>>
>> register(key);
>>
>> try {
>> ObjectDeque<T> objectDeque = poolMap.get(key);
>> - objectDeque.getIdleObjects().remove(toDestory);
>> - objectDeque.getAllObjects().remove(toDestory.getObject());
>> -
>> - try {
>> - _factory.destroyObject(key, toDestory.getObject());
>> - } finally {
>> - objectDeque.getCreateCount().decrementAndGet();
>> - numTotal.decrementAndGet();
>> + boolean isIdle = objectDeque.getIdleObjects().remove(toDestory);
>> +
>> + if (isIdle || always) {
>> + objectDeque.getAllObjects().remove(toDestory.getObject());
>> +
>> + try {
>> + _factory.destroyObject(key, toDestory.getObject());
>> + } finally {
>> + objectDeque.getCreateCount().decrementAndGet();
>> + numTotal.decrementAndGet();
>> + }
>> }
>> } finally {
>> deregister(key);
>>
>>
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org