commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Phil Steitz <phil.ste...@gmail.com>
Subject Re: svn commit: r1104601 - /commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
Date Wed, 18 May 2011 18:04:17 GMT
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


Mime
View raw message