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 2021/06/27 15:49:35 UTC

Re: [commons-pool] branch master updated: [POOL-395] Improve exception thrown in GenericObjectPool.borrowObject when pool is exhausted. Added BaseGenericObjectPool.setMessagesStatistics(boolean).

It's hard to tell what the actual change is below with all of the 
formatting / cosmetic changes mixed it, but AFAICT there is no sync 
control to ensure consistency or currency of the stats reported. Some 
note in javadoc or somewhere should be added to make it clear that stats 
may not accurately reflect snapshot state at the time of the exception.  
If you want to get a guaranteed accurate snapshot, you would have to 
lock the pool when you take it, which we don't want to do.

On 6/27/21 7:47 AM, ggregory@apache.org wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> ggregory pushed a commit to branch master
> in repository https://gitbox.apache.org/repos/asf/commons-pool.git
>
>
> The following commit(s) were added to refs/heads/master by this push:
>       new 26b0829  [POOL-395] Improve exception thrown in GenericObjectPool.borrowObject when pool is exhausted. Added BaseGenericObjectPool.setMessagesStatistics(boolean).
> 26b0829 is described below
>
> commit 26b0829fef39729df84adb1c01eb55c944aff6d7
> Author: Gary Gregory <ga...@gmail.com>
> AuthorDate: Sun Jun 27 10:47:03 2021 -0400
>
>      [POOL-395] Improve exception thrown in GenericObjectPool.borrowObject
>      when pool is exhausted. Added
>      BaseGenericObjectPool.setMessagesStatistics(boolean).
>      
>      - Pick up Checkstyle LineLength module from Commons IO where there was
>      none before.
>      - Message sentences start with a capital letter.
>      - Better parameter names.
>      - Javadoc.
>      - Inline comments.
> ---
>   checkstyle.xml                                     |   4 +
>   src/changes/changes.xml                            |   3 +
>   .../org/apache/commons/pool2/KeyedObjectPool.java  |   4 +-
>   .../commons/pool2/KeyedPooledObjectFactory.java    |   4 +-
>   .../java/org/apache/commons/pool2/ObjectPool.java  |   4 +-
>   .../apache/commons/pool2/PooledObjectFactory.java  |   4 +-
>   .../commons/pool2/impl/BaseGenericObjectPool.java  | 114 ++++++++++++++++-----
>   .../commons/pool2/impl/GenericKeyedObjectPool.java |  48 +++++----
>   .../commons/pool2/impl/GenericObjectPool.java      |  46 +++++----
>   .../pool2/impl/SoftReferenceObjectPool.java        |   4 +-
>   .../pool2/TestBasePoolableObjectFactory.java       |   4 +-
>   .../pool2/impl/TestAbandonedKeyedObjectPool.java   |   4 +-
>   .../pool2/impl/TestAbandonedObjectPool.java        |   4 +-
>   .../pool2/impl/TestGenericKeyedObjectPool.java     |  64 +++++++-----
>   .../commons/pool2/impl/TestGenericObjectPool.java  |  28 ++++-
>   15 files changed, 230 insertions(+), 109 deletions(-)
>
> diff --git a/checkstyle.xml b/checkstyle.xml
> index dca770b..3f884e1 100644
> --- a/checkstyle.xml
> +++ b/checkstyle.xml
> @@ -74,4 +74,8 @@
>       <property name="allowLegacy" value="true"/>
>     </module>
>   
> +  <module name="LineLength">
> +    <property name="max" value="160"/>
> +  </module>
> +
>   </module>
> diff --git a/src/changes/changes.xml b/src/changes/changes.xml
> index 40ef624..2368098 100644
> --- a/src/changes/changes.xml
> +++ b/src/changes/changes.xml
> @@ -69,6 +69,9 @@ The <action> type attribute can be add,update,fix,remove.
>       <action issue="POOL-396" dev="ggregory" type="add" due-to="Jeremy Kong, Phil Steitz">
>         Handle validation exceptions during eviction. #85.
>       </action>
> +    <action issue="POOL-395" dev="ggregory" type="add" due-to="Gary Gregory, Arash Nikoo">
> +      Improve exception thrown in GenericObjectPool.borrowObject when pool is exhausted. Added BaseGenericObjectPool.setMessagesStatistics(boolean).
> +    </action>
>       <!-- FIXES -->
>       <action dev="ggregory" type="fix" due-to="Gary Gregory">
>         Fix [WARNING] Old version of checkstyle detected. Consider updating to >= v8.30. Update Checktyle to 8.43.
> diff --git a/src/main/java/org/apache/commons/pool2/KeyedObjectPool.java b/src/main/java/org/apache/commons/pool2/KeyedObjectPool.java
> index ab52908..cac55ac 100644
> --- a/src/main/java/org/apache/commons/pool2/KeyedObjectPool.java
> +++ b/src/main/java/org/apache/commons/pool2/KeyedObjectPool.java
> @@ -289,12 +289,12 @@ public interface KeyedObjectPool<K, V> extends Closeable {
>        *
>        * @param key the key used to obtain the object
>        * @param obj a {@link #borrowObject borrowed} instance to be returned.
> -     * @param mode destroy activation context provided to the factory
> +     * @param destroyMode destroy activation context provided to the factory
>        *
>        * @throws Exception if the instance cannot be invalidated
>        * @since 2.9.0
>        */
> -    default void invalidateObject(final K key, final V obj, final DestroyMode mode) throws Exception {
> +    default void invalidateObject(final K key, final V obj, final DestroyMode destroyMode) throws Exception {
>           invalidateObject(key, obj);
>       }
>   
> diff --git a/src/main/java/org/apache/commons/pool2/KeyedPooledObjectFactory.java b/src/main/java/org/apache/commons/pool2/KeyedPooledObjectFactory.java
> index ac93114..a7f75a3 100644
> --- a/src/main/java/org/apache/commons/pool2/KeyedPooledObjectFactory.java
> +++ b/src/main/java/org/apache/commons/pool2/KeyedPooledObjectFactory.java
> @@ -117,7 +117,7 @@ public interface KeyedPooledObjectFactory<K, V> {
>        *
>        * @param key the key used when selecting the instance
>        * @param p a {@code PooledObject} wrapping the instance to be destroyed
> -     * @param mode DestroyMode providing context to the factory
> +     * @param destroyMode DestroyMode providing context to the factory
>        *
>        * @throws Exception should be avoided as it may be swallowed by
>        *    the pool implementation.
> @@ -128,7 +128,7 @@ public interface KeyedPooledObjectFactory<K, V> {
>        * @see DestroyMode
>        * @since 2.9.0
>        */
> -    default void destroyObject(final K key, final PooledObject<V> p, final DestroyMode mode) throws Exception {
> +    default void destroyObject(final K key, final PooledObject<V> p, final DestroyMode destroyMode) throws Exception {
>           destroyObject(key, p);
>       }
>   
> diff --git a/src/main/java/org/apache/commons/pool2/ObjectPool.java b/src/main/java/org/apache/commons/pool2/ObjectPool.java
> index 8225d27..8aa2d58 100644
> --- a/src/main/java/org/apache/commons/pool2/ObjectPool.java
> +++ b/src/main/java/org/apache/commons/pool2/ObjectPool.java
> @@ -198,12 +198,12 @@ public interface ObjectPool<T> extends Closeable {
>        * </p>
>        *
>        * @param obj a {@link #borrowObject borrowed} instance to be disposed.
> -     * @param mode destroy activation context provided to the factory
> +     * @param destroyMode destroy activation context provided to the factory
>        *
>        * @throws Exception if the instance cannot be invalidated
>        * @since 2.9.0
>        */
> -    default void invalidateObject(final T obj, final DestroyMode mode) throws Exception {
> +    default void invalidateObject(final T obj, final DestroyMode destroyMode) throws Exception {
>           invalidateObject(obj);
>       }
>   
> diff --git a/src/main/java/org/apache/commons/pool2/PooledObjectFactory.java b/src/main/java/org/apache/commons/pool2/PooledObjectFactory.java
> index 009f341..839e866 100644
> --- a/src/main/java/org/apache/commons/pool2/PooledObjectFactory.java
> +++ b/src/main/java/org/apache/commons/pool2/PooledObjectFactory.java
> @@ -112,7 +112,7 @@ public interface PooledObjectFactory<T> {
>      * DestroyMode.
>      *
>      * @param p a {@code PooledObject} wrapping the instance to be destroyed
> -   * @param mode DestroyMode providing context to the factory
> +   * @param destroyMode DestroyMode providing context to the factory
>      *
>      * @throws Exception should be avoided as it may be swallowed by
>      *    the pool implementation.
> @@ -123,7 +123,7 @@ public interface PooledObjectFactory<T> {
>      * @see DestroyMode
>      * @since 2.9.0
>      */
> -  default void destroyObject(final PooledObject<T> p, final DestroyMode mode) throws Exception {
> +  default void destroyObject(final PooledObject<T> p, final DestroyMode destroyMode) throws Exception {
>         destroyObject(p);
>     }
>   
> diff --git a/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java b/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java
> index 000a6eb..83f37e1 100644
> --- a/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java
> +++ b/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java
> @@ -26,9 +26,11 @@ import java.time.Duration;
>   import java.util.Arrays;
>   import java.util.Deque;
>   import java.util.Iterator;
> +import java.util.List;
>   import java.util.TimerTask;
>   import java.util.concurrent.ScheduledFuture;
>   import java.util.concurrent.atomic.AtomicLong;
> +import java.util.stream.Collectors;
>   
>   import javax.management.InstanceAlreadyExistsException;
>   import javax.management.InstanceNotFoundException;
> @@ -233,12 +235,14 @@ public abstract class BaseGenericObjectPool<T> extends BaseObject {
>               return builder.toString();
>           }
>       }
> +
>       /**
>        * Maintains a cache of values for a single metric and reports
>        * statistics on the cached values.
>        */
>       private class StatsStore {
>   
> +        private static final int NULL = -1;
>           private final AtomicLong[] values;
>           private final int size;
>           private int index;
> @@ -252,7 +256,7 @@ public abstract class BaseGenericObjectPool<T> extends BaseObject {
>               this.size = size;
>               values = new AtomicLong[size];
>               for (int i = 0; i < size; i++) {
> -                values[i] = new AtomicLong(-1);
> +                values[i] = new AtomicLong(NULL);
>               }
>           }
>   
> @@ -275,6 +279,15 @@ public abstract class BaseGenericObjectPool<T> extends BaseObject {
>           }
>   
>           /**
> +         * Gets the current values as a List.
> +         *
> +         * @return the current values as a List.
> +         */
> +        synchronized List<AtomicLong> getCurrentValues() {
> +            return Arrays.stream(values, 0, index).collect(Collectors.toList());
> +        }
> +
> +        /**
>            * Gets the mean of the cached values.
>            *
>            * @return the mean of the cache, truncated to long
> @@ -284,10 +297,9 @@ public abstract class BaseGenericObjectPool<T> extends BaseObject {
>               int counter = 0;
>               for (int i = 0; i < size; i++) {
>                   final long value = values[i].get();
> -                if (value != -1) {
> +                if (value != NULL) {
>                       counter++;
> -                    result = result * ((counter - 1) / (double) counter) +
> -                            value/(double) counter;
> +                    result = result * ((counter - 1) / (double) counter) + value / (double) counter;
>                   }
>               }
>               return (long) result;
> @@ -296,16 +308,19 @@ public abstract class BaseGenericObjectPool<T> extends BaseObject {
>           @Override
>           public String toString() {
>               final StringBuilder builder = new StringBuilder();
> -            builder.append("StatsStore [values=");
> -            builder.append(Arrays.toString(values));
> -            builder.append(", size=");
> +            builder.append("StatsStore [");
> +            // Only append what's been filled in.
> +            builder.append(getCurrentValues());
> +            builder.append("], size=");
>               builder.append(size);
>               builder.append(", index=");
>               builder.append(index);
>               builder.append("]");
>               return builder.toString();
>           }
> +
>       }
> +
>       // Constants
>       /**
>        * The size of the caches used to store historical data for some attributes
> @@ -334,7 +349,6 @@ public abstract class BaseGenericObjectPool<T> extends BaseObject {
>       final Object closeLock = new Object();
>       volatile boolean closed;
>   
> -
>       final Object evictionLock = new Object();
>       private Evictor evictor = null; // @GuardedBy("evictionLock")
>       EvictionIterator evictionIterator = null; // @GuardedBy("evictionLock")
> @@ -348,21 +362,21 @@ public abstract class BaseGenericObjectPool<T> extends BaseObject {
>       // Monitoring (primarily JMX) attributes
>       private final ObjectName objectName;
>       private final String creationStackTrace;
> -    private final AtomicLong borrowedCount = new AtomicLong(0);
> -    private final AtomicLong returnedCount = new AtomicLong(0);
> -    final AtomicLong createdCount = new AtomicLong(0);
> -    final AtomicLong destroyedCount = new AtomicLong(0);
> -    final AtomicLong destroyedByEvictorCount = new AtomicLong(0);
> -    final AtomicLong destroyedByBorrowValidationCount = new AtomicLong(0);
> +    private final AtomicLong borrowedCount = new AtomicLong();
> +    private final AtomicLong returnedCount = new AtomicLong();
> +    final AtomicLong createdCount = new AtomicLong();
> +    final AtomicLong destroyedCount = new AtomicLong();
> +    final AtomicLong destroyedByEvictorCount = new AtomicLong();
> +    final AtomicLong destroyedByBorrowValidationCount = new AtomicLong();
> +
>       private final StatsStore activeTimes = new StatsStore(MEAN_TIMING_STATS_CACHE_SIZE);
> -
>       private final StatsStore idleTimes = new StatsStore(MEAN_TIMING_STATS_CACHE_SIZE);
> -
>       private final StatsStore waitTimes = new StatsStore(MEAN_TIMING_STATS_CACHE_SIZE);
>   
> -    private final AtomicLong maxBorrowWaitTimeMillis = new AtomicLong(0L);
> +    private final AtomicLong maxBorrowWaitTimeMillis = new AtomicLong();
>   
> -    private volatile SwallowedExceptionListener swallowedExceptionListener = null;
> +    private volatile SwallowedExceptionListener swallowedExceptionListener;
> +    private volatile boolean messageStatistics;
>   
>       /**
>        * Handles JMX registration (if required) and the initialization required for
> @@ -396,6 +410,16 @@ public abstract class BaseGenericObjectPool<T> extends BaseObject {
>       }
>   
>       /**
> +     * Appends statistics if enabled.
> +     *
> +     * @param string The root string.
> +     * @return The root string plus statistics.
> +     */
> +    String appendStats(final String string) {
> +        return messageStatistics ? string + ", " + getStatsString() : string;
> +    }
> +
> +    /**
>        * Verifies that the pool is open.
>        * @throws IllegalStateException if the pool is closed.
>        */
> @@ -680,6 +704,16 @@ public abstract class BaseGenericObjectPool<T> extends BaseObject {
>       }
>   
>       /**
> +     * Gets whether to include statistics in exception messages.
> +     *
> +     * @return whether to include statistics in exception messages.
> +     * @since 2.11.0
> +     */
> +    public boolean getMessageStatistics() {
> +        return messageStatistics;
> +    }
> +
> +    /**
>        * Gets the minimum amount of time an object may sit idle in the pool
>        * before it is eligible for eviction by the idle object evictor (if any -
>        * see {@link #setTimeBetweenEvictionRuns(Duration)}). When non-positive,
> @@ -806,6 +840,22 @@ public abstract class BaseGenericObjectPool<T> extends BaseObject {
>       }
>   
>       /**
> +     * Gets a statistics string.
> +     *
> +     * @return  a statistics string.
> +     */
> +    String getStatsString() {
> +        // Simply listed in AB order.
> +        return String.format(
> +                "activeTimes=%s, blockWhenExhausted=%s, borrowedCount=%,d, closed=%s, createdCount=%,d, destroyedByBorrowValidationCount=%,d, destroyedByEvictorCount=%,d, evictorShutdownTimeout=%s, fairness=%s, idleTimes=%s, lifo=%s, maxBorrowWaitTimeMillis=%,d, maxTotal=%s, maxWaitDuration=%s, minEvictableIdleTime=%s, numTestsPerEvictionRun=%s, returnedCount=%s, softMinEvictableIdleTime=%s, testOnBorrow=%s, testOnCreate=%s, testOnReturn=%s, testWhileIdle=%s, timeBetweenEvictionRuns=%s,  [...]
> +                activeTimes.getCurrentValues(), blockWhenExhausted, borrowedCount.get(), closed, createdCount.get(),
> +                destroyedByBorrowValidationCount.get(), destroyedByEvictorCount.get(), evictorShutdownTimeout, fairness,
> +                idleTimes.getCurrentValues(), lifo, maxBorrowWaitTimeMillis.get(), maxTotal, maxWaitDuration,
> +                minEvictableIdleTime, numTestsPerEvictionRun, returnedCount, softMinEvictableIdleTime, testOnBorrow,
> +                testOnCreate, testOnReturn, testWhileIdle, timeBetweenEvictionRuns, waitTimes.getCurrentValues());
> +    }
> +
> +    /**
>        * The listener used (if any) to receive notifications of exceptions
>        * unavoidably swallowed by the pool.
>        *
> @@ -984,8 +1034,7 @@ public abstract class BaseGenericObjectPool<T> extends BaseObject {
>       final void jmxUnregister() {
>           if (objectName != null) {
>               try {
> -                ManagementFactory.getPlatformMBeanServer().unregisterMBean(
> -                        objectName);
> +                ManagementFactory.getPlatformMBeanServer().unregisterMBean(objectName);
>               } catch (final MBeanRegistrationException | InstanceNotFoundException e) {
>                   swallowException(e);
>               }
> @@ -997,10 +1046,9 @@ public abstract class BaseGenericObjectPool<T> extends BaseObject {
>        * @param pooledObject instance to return to the keyed pool
>        */
>       protected void markReturningState(final PooledObject<T> pooledObject) {
> -        synchronized(pooledObject) {
> +        synchronized (pooledObject) {
>               if (pooledObject.getState() != PooledObjectState.ALLOCATED) {
> -                throw new IllegalStateException(
> -                        "Object has already been returned to this pool or is invalid");
> +                throw new IllegalStateException("Object has already been returned to this pool or is invalid");
>               }
>               pooledObject.markReturning(); // Keep from being marked abandoned
>           }
> @@ -1121,9 +1169,9 @@ public abstract class BaseGenericObjectPool<T> extends BaseObject {
>                       classLoader + ", " + epClassLoader + "] do not implement " + EVICTION_POLICY_TYPE_NAME);
>           } catch (final ClassNotFoundException | InstantiationException | IllegalAccessException |
>                   InvocationTargetException | NoSuchMethodException e) {
> -            final String exMessage = "Unable to create " + EVICTION_POLICY_TYPE_NAME + " instance of type " +
> -                    evictionPolicyClassName;
> -            throw new IllegalArgumentException(exMessage, e);
> +            throw new IllegalArgumentException(
> +                    "Unable to create " + EVICTION_POLICY_TYPE_NAME + " instance of type " + evictionPolicyClassName,
> +                    e);
>           }
>       }
>   
> @@ -1223,6 +1271,16 @@ public abstract class BaseGenericObjectPool<T> extends BaseObject {
>       }
>   
>       /**
> +     * Sets whether to include statistics in exception messages.
> +     *
> +     * @param messagesDetails whether to include statistics in exception messages.
> +     * @since 2.11.0
> +     */
> +    public void setMessagesStatistics(boolean messagesDetails) {
> +        this.messageStatistics = messagesDetails;
> +    }
> +
> +    /**
>        * Sets the minimum amount of time an object may sit idle in the pool
>        * before it is eligible for eviction by the idle object evictor (if any -
>        * see {@link #setTimeBetweenEvictionRuns(Duration)}). When non-positive,
> @@ -1446,6 +1504,8 @@ public abstract class BaseGenericObjectPool<T> extends BaseObject {
>           setTimeBetweenEvictionRuns(Duration.ofMillis(timeBetweenEvictionRunsMillis));
>       }
>   
> +    // Inner classes
> +
>       /**
>        * <p>Starts the evictor with the given delay. If there is an evictor
>        * running when this method is called, it is stopped and replaced with a
> @@ -1478,8 +1538,6 @@ public abstract class BaseGenericObjectPool<T> extends BaseObject {
>           }
>       }
>   
> -    // Inner classes
> -
>       /**
>        * Stops the evictor.
>        */
> diff --git a/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java b/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
> index 11fd633..64274bd 100644
> --- a/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
> +++ b/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
> @@ -121,7 +121,7 @@ public class GenericKeyedObjectPool<K, T> extends BaseGenericObjectPool<T>
>            * Invariant: empty keyed pool will not be dropped unless numInterested
>            *            is 0.
>            */
> -        private final AtomicLong numInterested = new AtomicLong(0);
> +        private final AtomicLong numInterested = new AtomicLong();
>   
>           /**
>            * Constructs a new ObjecDeque with the given fairness policy.
> @@ -267,7 +267,7 @@ public class GenericKeyedObjectPool<K, T> extends BaseGenericObjectPool<T>
>   
>           if (factory == null) {
>               jmxUnregister(); // tidy up
> -            throw new IllegalArgumentException("factory may not be null");
> +            throw new IllegalArgumentException("Factory may not be null");
>           }
>           this.factory = factory;
>           this.fairness = config.getFairness();
> @@ -449,16 +449,15 @@ public class GenericKeyedObjectPool<K, T> extends BaseGenericObjectPool<T>
>                           if (borrowMaxWaitMillis < 0) {
>                               p = objectDeque.getIdleObjects().takeFirst();
>                           } else {
> -                            p = objectDeque.getIdleObjects().pollFirst(
> -                                    borrowMaxWaitMillis, TimeUnit.MILLISECONDS);
> +                            p = objectDeque.getIdleObjects().pollFirst(borrowMaxWaitMillis, TimeUnit.MILLISECONDS);
>                           }
>                       }
>                       if (p == null) {
> -                        throw new NoSuchElementException(
> -                                "Timeout waiting for idle object");
> +                        throw new NoSuchElementException(appendStats(
> +                                "Timeout waiting for idle object, borrowMaxWaitMillis=" + borrowMaxWaitMillis));
>                       }
>                   } else if (p == null) {
> -                    throw new NoSuchElementException("Pool exhausted");
> +                    throw new NoSuchElementException(appendStats("Pool exhausted"));
>                   }
>                   if (!p.allocate()) {
>                       p = null;
> @@ -475,8 +474,7 @@ public class GenericKeyedObjectPool<K, T> extends BaseGenericObjectPool<T>
>                           }
>                           p = null;
>                           if (create) {
> -                            final NoSuchElementException nsee = new NoSuchElementException(
> -                                    "Unable to activate object");
> +                            final NoSuchElementException nsee = new NoSuchElementException(appendStats("Unable to activate object"));
>                               nsee.initCause(e);
>                               throw nsee;
>                           }
> @@ -500,7 +498,7 @@ public class GenericKeyedObjectPool<K, T> extends BaseGenericObjectPool<T>
>                               p = null;
>                               if (create) {
>                                   final NoSuchElementException nsee = new NoSuchElementException(
> -                                        "Unable to validate object");
> +                                        appendStats("Unable to validate object"));
>                                   nsee.initCause(validationThrowable);
>                                   throw nsee;
>                               }
> @@ -517,6 +515,13 @@ public class GenericKeyedObjectPool<K, T> extends BaseGenericObjectPool<T>
>           return p.getObject();
>       }
>   
> +    @Override
> +    String getStatsString() {
> +        // Simply listed in AB order.
> +        return super.getStatsString() +
> +                String.format(", fairness=%s, maxIdlePerKey%,d, maxTotalPerKey=%,d, minIdlePerKey=%,d, numTotal=%,d",
> +                        fairness, maxIdlePerKey, maxTotalPerKey, minIdlePerKey, numTotal.get());
> +    }
>   
>       /**
>        * Calculate the number of objects that need to be created to attempt to
> @@ -820,7 +825,7 @@ public class GenericKeyedObjectPool<K, T> extends BaseGenericObjectPool<T>
>       /**
>        * De-register the use of a key by an object.
>        * <p>
> -     * register() and deregister() must always be used as a pair.
> +     * {@link #register()} and {@link #deregister()} must always be used as a pair.
>        * </p>
>        *
>        * @param k The key to de-register
> @@ -858,12 +863,12 @@ public class GenericKeyedObjectPool<K, T> extends BaseGenericObjectPool<T>
>        * @param toDestroy The wrapped object to be destroyed
>        * @param always Should the object be destroyed even if it is not currently
>        *               in the set of idle objects for the given key
> -     * @param mode DestroyMode context provided to the factory
> +     * @param destroyMode DestroyMode context provided to the factory
>        *
>        * @return {@code true} if the object was destroyed, otherwise {@code false}
>        * @throws Exception If the object destruction failed
>        */
> -    private boolean destroy(final K key, final PooledObject<T> toDestroy, final boolean always, final DestroyMode mode)
> +    private boolean destroy(final K key, final PooledObject<T> toDestroy, final boolean always, final DestroyMode destroyMode)
>               throws Exception {
>   
>           final ObjectDeque<T> objectDeque = register(key);
> @@ -884,7 +889,7 @@ public class GenericKeyedObjectPool<K, T> extends BaseGenericObjectPool<T>
>                   toDestroy.invalidate();
>   
>                   try {
> -                    factory.destroyObject(key, toDestroy, mode);
> +                    factory.destroyObject(key, toDestroy, destroyMode);
>                   } finally {
>                       objectDeque.getCreateCount().decrementAndGet();
>                       destroyedCount.incrementAndGet();
> @@ -1414,7 +1419,7 @@ public class GenericKeyedObjectPool<K, T> extends BaseGenericObjectPool<T>
>        *
>        * @param key pool key
>        * @param obj instance to invalidate
> -     * @param mode DestroyMode context provided to factory
> +     * @param destroyMode DestroyMode context provided to factory
>        *
>        * @throws Exception             if an exception occurs destroying the
>        *                               object
> @@ -1423,15 +1428,15 @@ public class GenericKeyedObjectPool<K, T> extends BaseGenericObjectPool<T>
>        * @since 2.9.0
>        */
>       @Override
> -    public void invalidateObject(final K key, final T obj, final DestroyMode mode) throws Exception {
> +    public void invalidateObject(final K key, final T obj, final DestroyMode destroyMode) throws Exception {
>           final ObjectDeque<T> objectDeque = poolMap.get(key);
>           final PooledObject<T> p = objectDeque.getAllObjects().get(new IdentityWrapper<>(obj));
>           if (p == null) {
> -            throw new IllegalStateException("Object not currently part of this pool");
> +            throw new IllegalStateException(appendStats("Object not currently part of this pool"));
>           }
>           synchronized (p) {
>               if (p.getState() != PooledObjectState.INVALID) {
> -                destroy(key, p, true, mode);
> +                destroy(key, p, true, destroyMode);
>               }
>           }
>           if (objectDeque.idleObjects.hasTakeWaiters()) {
> @@ -1502,7 +1507,7 @@ public class GenericKeyedObjectPool<K, T> extends BaseGenericObjectPool<T>
>       /**
>        * Register the use of a key by an object.
>        * <p>
> -     * register() and deregister() must always be used as a pair.
> +     * {@link #register()} and {@link #deregister()} must always be used as a pair.
>        * </p>
>        *
>        * @param k The key to register
> @@ -1579,6 +1584,7 @@ public class GenericKeyedObjectPool<K, T> extends BaseGenericObjectPool<T>
>                   try {
>                       invalidateObject(pool.getKey(), pooledObject.getObject(), DestroyMode.ABANDONED);
>                   } catch (final Exception e) {
> +                    // TODO handle/log?
>                       e.printStackTrace();
>                   }
>               }
> @@ -1828,9 +1834,9 @@ public class GenericKeyedObjectPool<K, T> extends BaseGenericObjectPool<T>
>        *
>        * @param minIdlePerKey The minimum size of the each keyed pool
>        *
> -     * @see #getMinIdlePerKey
> +     * @see #getMinIdlePerKey()
>        * @see #getMaxIdlePerKey()
> -     * @see #setTimeBetweenEvictionRunsMillis
> +     * @see #setTimeBetweenEvictionRuns(Duration)
>        */
>       public void setMinIdlePerKey(final int minIdlePerKey) {
>           this.minIdlePerKey = minIdlePerKey;
> diff --git a/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java b/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
> index ee27b58..c5fed24 100644
> --- a/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
> +++ b/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
> @@ -115,7 +115,7 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
>        * {@link #create()} will ensure that there are never more than
>        * {@link #_maxActive} objects created at any one time.
>        */
> -    private final AtomicLong createCount = new AtomicLong(0);
> +    private final AtomicLong createCount = new AtomicLong();
>   
>       private long makeObjectCount;
>   
> @@ -155,7 +155,7 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
>   
>           if (factory == null) {
>               jmxUnregister(); // tidy up
> -            throw new IllegalArgumentException("factory may not be null");
> +            throw new IllegalArgumentException("Factory may not be null");
>           }
>           this.factory = factory;
>   
> @@ -214,8 +214,7 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
>       public void addObject() throws Exception {
>           assertOpen();
>           if (factory == null) {
> -            throw new IllegalStateException(
> -                    "Cannot add objects without a factory.");
> +            throw new IllegalStateException("Cannot add objects without a factory.");
>           }
>           final PooledObject<T> p = create();
>           addIdleObject(p);
> @@ -271,7 +270,7 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
>        * available instances in request arrival order.
>        * </p>
>        *
> -     * @param borrowMaxWait The time to wait for an object
> +     * @param borrowMaxWaitDuration The time to wait for an object
>        *                            to become available
>        *
>        * @return object instance from the pool
> @@ -282,11 +281,11 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
>        *                   error
>        * @since 2.10.0
>        */
> -    public T borrowObject(final Duration borrowMaxWait) throws Exception {
> +    public T borrowObject(final Duration borrowMaxWaitDuration) throws Exception {
>           assertOpen();
>   
>           final AbandonedConfig ac = this.abandonedConfig;
> -        if (ac != null && ac.getRemoveAbandonedOnBorrow() && (getNumIdle() < 2) &&
> +        if (ac != null && ac.getRemoveAbandonedOnBorrow() && (getNumIdle() < 2) &&
>                   (getNumActive() > getMaxTotal() - 3)) {
>               removeAbandoned(ac);
>           }
> @@ -311,17 +310,18 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
>               }
>               if (blockWhenExhausted) {
>                   if (p == null) {
> -                    if (borrowMaxWait.isNegative()) {
> +                    if (borrowMaxWaitDuration.isNegative()) {
>                           p = idleObjects.takeFirst();
>                       } else {
> -                        p = idleObjects.pollFirst(borrowMaxWait);
> +                        p = idleObjects.pollFirst(borrowMaxWaitDuration);
>                       }
>                   }
>                   if (p == null) {
> -                    throw new NoSuchElementException("Timeout waiting for idle object");
> +                    throw new NoSuchElementException(appendStats(
> +                            "Timeout waiting for idle object, borrowMaxWaitMillis=" + borrowMaxWaitDuration));
>                   }
>               } else if (p == null) {
> -                throw new NoSuchElementException("Pool exhausted");
> +                throw new NoSuchElementException(appendStats("Pool exhausted"));
>               }
>               if (!p.allocate()) {
>                   p = null;
> @@ -338,7 +338,8 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
>                       }
>                       p = null;
>                       if (create) {
> -                        final NoSuchElementException nsee = new NoSuchElementException("Unable to activate object");
> +                        final NoSuchElementException nsee = new NoSuchElementException(
> +                                appendStats("Unable to activate object"));
>                           nsee.initCause(e);
>                           throw nsee;
>                       }
> @@ -361,7 +362,8 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
>                           }
>                           p = null;
>                           if (create) {
> -                            final NoSuchElementException nsee = new NoSuchElementException("Unable to validate object");
> +                            final NoSuchElementException nsee = new NoSuchElementException(
> +                                    appendStats("Unable to validate object"));
>                               nsee.initCause(validationThrowable);
>                               throw nsee;
>                           }
> @@ -375,6 +377,14 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
>           return p.getObject();
>       }
>   
> +    @Override
> +    String getStatsString() {
> +        // Simply listed in AB order.
> +        return super.getStatsString() +
> +                String.format(", createdCount=%,d, makeObjectCount=%,d, maxIdle=%,d, minIdle=%,d",
> +                        createdCount.get(), makeObjectCount, maxIdle, minIdle);
> +    }
> +
>       /**
>        * Borrows an object from the pool using the specific waiting time which only
>        * applies if {@link #getBlockWhenExhausted()} is true.
> @@ -592,17 +602,17 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
>        * Destroys a wrapped pooled object.
>        *
>        * @param toDestroy The wrapped pooled object to destroy
> -     * @param mode DestroyMode context provided to the factory
> +     * @param destroyMode DestroyMode context provided to the factory
>        *
>        * @throws Exception If the factory fails to destroy the pooled object
>        *                   cleanly
>        */
> -    private void destroy(final PooledObject<T> toDestroy, final DestroyMode mode) throws Exception {
> +    private void destroy(final PooledObject<T> toDestroy, final DestroyMode destroyMode) throws Exception {
>           toDestroy.invalidate();
>           idleObjects.remove(toDestroy);
>           allObjects.remove(new IdentityWrapper<>(toDestroy.getObject()));
>           try {
> -            factory.destroyObject(toDestroy, mode);
> +            factory.destroyObject(toDestroy, destroyMode);
>           } finally {
>               destroyedCount.incrementAndGet();
>               createCount.decrementAndGet();
> @@ -1002,7 +1012,7 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
>        * @since 2.9.0
>        */
>       @Override
> -    public void invalidateObject(final T obj, final DestroyMode mode) throws Exception {
> +    public void invalidateObject(final T obj, final DestroyMode destroyMode) throws Exception {
>           final PooledObject<T> p = allObjects.get(new IdentityWrapper<>(obj));
>           if (p == null) {
>               if (isAbandonedConfig()) {
> @@ -1013,7 +1023,7 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T>
>           }
>           synchronized (p) {
>               if (p.getState() != PooledObjectState.INVALID) {
> -                destroy(p, mode);
> +                destroy(p, destroyMode);
>               }
>           }
>           ensureIdle(1, false);
> diff --git a/src/main/java/org/apache/commons/pool2/impl/SoftReferenceObjectPool.java b/src/main/java/org/apache/commons/pool2/impl/SoftReferenceObjectPool.java
> index 849629f..f7c271c 100644
> --- a/src/main/java/org/apache/commons/pool2/impl/SoftReferenceObjectPool.java
> +++ b/src/main/java/org/apache/commons/pool2/impl/SoftReferenceObjectPool.java
> @@ -216,9 +216,7 @@ public class SoftReferenceObjectPool<T> extends BaseObjectPool<T> {
>                           obj = null;
>                       }
>                       if (newlyCreated) {
> -                        throw new NoSuchElementException(
> -                                "Could not create a validated object, cause: " +
> -                                        t.getMessage());
> +                        throw new NoSuchElementException("Could not create a validated object, cause: " + t);
>                       }
>                   }
>               }
> diff --git a/src/test/java/org/apache/commons/pool2/TestBasePoolableObjectFactory.java b/src/test/java/org/apache/commons/pool2/TestBasePoolableObjectFactory.java
> index 97c411e..43935d5 100644
> --- a/src/test/java/org/apache/commons/pool2/TestBasePoolableObjectFactory.java
> +++ b/src/test/java/org/apache/commons/pool2/TestBasePoolableObjectFactory.java
> @@ -34,8 +34,8 @@ public class TestBasePoolableObjectFactory {
>               return new AtomicInteger(0);
>           }
>           @Override
> -        public void destroyObject(final PooledObject<AtomicInteger> p, final DestroyMode mode){
> -            if (mode.equals(DestroyMode.ABANDONED)) {
> +        public void destroyObject(final PooledObject<AtomicInteger> p, final DestroyMode destroyMode){
> +            if (destroyMode.equals(DestroyMode.ABANDONED)) {
>                   p.getObject().incrementAndGet();
>               }
>           }
> diff --git a/src/test/java/org/apache/commons/pool2/impl/TestAbandonedKeyedObjectPool.java b/src/test/java/org/apache/commons/pool2/impl/TestAbandonedKeyedObjectPool.java
> index 541e1ea..0d850c9 100644
> --- a/src/test/java/org/apache/commons/pool2/impl/TestAbandonedKeyedObjectPool.java
> +++ b/src/test/java/org/apache/commons/pool2/impl/TestAbandonedKeyedObjectPool.java
> @@ -103,7 +103,7 @@ public class TestAbandonedKeyedObjectPool {
>           }
>   
>           @Override
> -        public void destroyObject(final Integer key, final PooledObject<PooledTestObject> obj, final DestroyMode mode) throws Exception {
> +        public void destroyObject(final Integer key, final PooledObject<PooledTestObject> obj, final DestroyMode destroyMode) throws Exception {
>               obj.getObject().setActive(false);
>               // while destroying instances, yield control to other threads
>               // helps simulate threading errors
> @@ -111,7 +111,7 @@ public class TestAbandonedKeyedObjectPool {
>               if (destroyLatency != 0) {
>                   Thread.sleep(destroyLatency);
>               }
> -            obj.getObject().destroy(mode);
> +            obj.getObject().destroy(destroyMode);
>           }
>   
>           @Override
> diff --git a/src/test/java/org/apache/commons/pool2/impl/TestAbandonedObjectPool.java b/src/test/java/org/apache/commons/pool2/impl/TestAbandonedObjectPool.java
> index 14b264c..9b64560 100644
> --- a/src/test/java/org/apache/commons/pool2/impl/TestAbandonedObjectPool.java
> +++ b/src/test/java/org/apache/commons/pool2/impl/TestAbandonedObjectPool.java
> @@ -183,7 +183,7 @@ public class TestAbandonedObjectPool {
>           }
>   
>           @Override
> -        public void destroyObject(final PooledObject<PooledTestObject> obj, final DestroyMode mode) throws Exception {
> +        public void destroyObject(final PooledObject<PooledTestObject> obj, final DestroyMode destroyMode) throws Exception {
>               obj.getObject().setActive(false);
>               // while destroying instances, yield control to other threads
>               // helps simulate threading errors
> @@ -191,7 +191,7 @@ public class TestAbandonedObjectPool {
>               if (destroyLatency != 0) {
>                   Thread.sleep(destroyLatency);
>               }
> -            obj.getObject().destroy(mode);
> +            obj.getObject().destroy(destroyMode);
>           }
>   
>           @Override
> diff --git a/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java b/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
> index 39e898c..cd36876 100644
> --- a/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
> +++ b/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
> @@ -20,6 +20,7 @@ package org.apache.commons.pool2.impl;
>   
>   import static org.junit.jupiter.api.Assertions.assertEquals;
>   import static org.junit.jupiter.api.Assertions.assertFalse;
> +import static org.junit.jupiter.api.Assertions.assertNotEquals;
>   import static org.junit.jupiter.api.Assertions.assertNotNull;
>   import static org.junit.jupiter.api.Assertions.assertNotSame;
>   import static org.junit.jupiter.api.Assertions.assertSame;
> @@ -906,6 +907,18 @@ public class TestGenericKeyedObjectPool extends TestKeyedObjectPool {
>       }
>   
>       @Test
> +    public void testAppendStats() {
> +        assertFalse(gkoPool.getMessageStatistics());
> +        assertEquals("foo", (gkoPool.appendStats("foo")));
> +        try (final GenericKeyedObjectPool<?, ?> pool = new GenericKeyedObjectPool<>(new SimpleFactory<>())) {
> +            pool.setMessagesStatistics(true);
> +            assertNotEquals("foo", (pool.appendStats("foo")));
> +            pool.setMessagesStatistics(false);
> +            assertEquals("foo", (pool.appendStats("foo")));
> +        }
> +    }
> +
> +    @Test
>       @Timeout(value = 60000, unit = TimeUnit.MILLISECONDS)
>       public void testBlockedKeyDoesNotBlockPool() throws Exception {
>           gkoPool.setBlockWhenExhausted(true);
> @@ -1025,6 +1038,7 @@ public class TestGenericKeyedObjectPool extends TestKeyedObjectPool {
>           gkoPool.close();
>       }
>   
> +
>       /**
>        * Test to make sure that clearOldest does not destroy instances that have been checked out.
>        *
> @@ -1061,7 +1075,6 @@ public class TestGenericKeyedObjectPool extends TestKeyedObjectPool {
>           }
>       }
>   
> -
>       // POOL-259
>       @Test
>       public void testClientWaitStats() throws Exception {
> @@ -1483,6 +1496,27 @@ public class TestGenericKeyedObjectPool extends TestKeyedObjectPool {
>   
>       @Test
>       @Timeout(value = 60000, unit = TimeUnit.MILLISECONDS)
> +    public void testExceptionInValidationDuringEviction() throws Exception {
> +        gkoPool.setMaxIdlePerKey(1);
> +        gkoPool.setMinEvictableIdleTime(Duration.ZERO);
> +        gkoPool.setTestWhileIdle(true);
> +
> +        final String obj = gkoPool.borrowObject("one");
> +        gkoPool.returnObject("one", obj);
> +
> +        simpleFactory.setThrowExceptionOnValidate(true);
> +        try {
> +            gkoPool.evict();
> +            fail("Expecting RuntimeException");
> +        } catch (final RuntimeException e) {
> +            // expected
> +        }
> +        assertEquals(0, gkoPool.getNumActive());
> +        assertEquals(0, gkoPool.getNumIdle());
> +    }
> +
> +    @Test
> +    @Timeout(value = 60000, unit = TimeUnit.MILLISECONDS)
>       public void testExceptionOnActivateDuringBorrow() throws Exception {
>           final String obj1 = gkoPool.borrowObject("one");
>           final String obj2 = gkoPool.borrowObject("one");
> @@ -1514,6 +1548,7 @@ public class TestGenericKeyedObjectPool extends TestKeyedObjectPool {
>           assertEquals(0, gkoPool.getNumIdle());
>       }
>   
> +
>       @Test
>       @Timeout(value = 60000, unit = TimeUnit.MILLISECONDS)
>       public void testExceptionOnDestroyDuringBorrow() throws Exception {
> @@ -1550,7 +1585,6 @@ public class TestGenericKeyedObjectPool extends TestKeyedObjectPool {
>           assertEquals(0, gkoPool.getNumIdle());
>       }
>   
> -
>       @Test
>       @Timeout(value = 60000, unit = TimeUnit.MILLISECONDS)
>       public void testExceptionOnPassivateDuringReturn() throws Exception {
> @@ -1563,27 +1597,6 @@ public class TestGenericKeyedObjectPool extends TestKeyedObjectPool {
>   
>       @Test
>       @Timeout(value = 60000, unit = TimeUnit.MILLISECONDS)
> -    public void testExceptionInValidationDuringEviction() throws Exception {
> -        gkoPool.setMaxIdlePerKey(1);
> -        gkoPool.setMinEvictableIdleTime(Duration.ZERO);
> -        gkoPool.setTestWhileIdle(true);
> -
> -        final String obj = gkoPool.borrowObject("one");
> -        gkoPool.returnObject("one", obj);
> -
> -        simpleFactory.setThrowExceptionOnValidate(true);
> -        try {
> -            gkoPool.evict();
> -            fail("Expecting RuntimeException");
> -        } catch (final RuntimeException e) {
> -            // expected
> -        }
> -        assertEquals(0, gkoPool.getNumActive());
> -        assertEquals(0, gkoPool.getNumIdle());
> -    }
> -
> -    @Test
> -    @Timeout(value = 60000, unit = TimeUnit.MILLISECONDS)
>       public void testFIFO() throws Exception {
>           gkoPool.setLifo(false);
>           final String key = "key";
> @@ -1600,6 +1613,11 @@ public class TestGenericKeyedObjectPool extends TestKeyedObjectPool {
>           assertEquals( "key4", gkoPool.borrowObject(key),"new-4");
>       }
>   
> +    @Test
> +    public void testGetStatsString() {
> +        assertNotNull((gkoPool.getStatsString()));
> +    }
> +
>       /**
>        * Verify that threads waiting on a depleted pool get served when a checked out object is
>        * invalidated.
> diff --git a/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java b/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
> index 1014588..fe646a3 100644
> --- a/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
> +++ b/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
> @@ -20,6 +20,7 @@ package org.apache.commons.pool2.impl;
>   
>   import static org.junit.jupiter.api.Assertions.assertEquals;
>   import static org.junit.jupiter.api.Assertions.assertFalse;
> +import static org.junit.jupiter.api.Assertions.assertNotEquals;
>   import static org.junit.jupiter.api.Assertions.assertNotNull;
>   import static org.junit.jupiter.api.Assertions.assertNull;
>   import static org.junit.jupiter.api.Assertions.assertThrows;
> @@ -58,6 +59,7 @@ import org.apache.commons.pool2.VisitTracker;
>   import org.apache.commons.pool2.VisitTrackerFactory;
>   import org.apache.commons.pool2.Waiter;
>   import org.apache.commons.pool2.WaiterFactory;
> +import org.apache.commons.pool2.impl.TestGenericKeyedObjectPool.SimpleFactory;
>   import org.junit.jupiter.api.AfterEach;
>   import org.junit.jupiter.api.BeforeEach;
>   import org.junit.jupiter.api.Test;
> @@ -83,7 +85,9 @@ public class TestGenericObjectPool extends TestBaseObjectPool {
>                   } else {
>                       genericObjectPool.evict();
>                   }
> -            } catch (final Exception e) { /* Ignore */}
> +            } catch (final Exception e) {
> +                // Ignore.
> +            }
>           }
>       }
>   
> @@ -999,6 +1003,18 @@ public class TestGenericObjectPool extends TestBaseObjectPool {
>           assertEquals( 0, genericObjectPool.getNumActive(),"should be zero active");
>       }
>   
> +    @Test
> +    public void testAppendStats() {
> +        assertFalse(genericObjectPool.getMessageStatistics());
> +        assertEquals("foo", (genericObjectPool.appendStats("foo")));
> +        try (final GenericObjectPool<?> pool = new GenericObjectPool<>(new SimpleFactory())) {
> +            pool.setMessagesStatistics(true);
> +            assertNotEquals("foo", (pool.appendStats("foo")));
> +            pool.setMessagesStatistics(false);
> +            assertEquals("foo", (pool.appendStats("foo")));
> +        }
> +    }
> +
>       /*
>        * 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
> @@ -1185,6 +1201,7 @@ public class TestGenericObjectPool extends TestBaseObjectPool {
>           }
>       }
>   
> +
>       /**
>        * POOL-231 - verify that concurrent invalidates of the same object do not
>        * corrupt pool destroyCount.
> @@ -1238,7 +1255,6 @@ public class TestGenericObjectPool extends TestBaseObjectPool {
>           assertEquals(nIterations, genericObjectPool.getDestroyedCount());
>       }
>   
> -
>       @Test
>       public void testConstructorNullFactory() {
>           // add dummy assert (won't be invoked because of IAE) to avoid "unused" warning
> @@ -1916,6 +1932,14 @@ public class TestGenericObjectPool extends TestBaseObjectPool {
>           }
>       }
>   
> +    @Test
> +    public void testGetStatsString() {
> +        try (final GenericObjectPool<String> pool = new GenericObjectPool<>(
> +                new TestSynchronizedPooledObjectFactory<>(createNullPooledObjectFactory()))) {
> +            assertNotNull(pool.getStatsString());
> +        }
> +    }
> +
>       /**
>        * Verify that threads waiting on a depleted pool get served when a checked out object is
>        * invalidated.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [commons-pool] branch master updated: [POOL-395] Improve exception thrown in GenericObjectPool.borrowObject when pool is exhausted. Added BaseGenericObjectPool.setMessagesStatistics(boolean).

Posted by Gary Gregory <ga...@gmail.com>.
I will update Javadoc and add inline comments.

Gary


On Sun, Jun 27, 2021, 11:49 Phil Steitz <ph...@gmail.com> wrote:

> It's hard to tell what the actual change is below with all of the
> formatting / cosmetic changes mixed it, but AFAICT there is no sync
> control to ensure consistency or currency of the stats reported. Some
> note in javadoc or somewhere should be added to make it clear that stats
> may not accurately reflect snapshot state at the time of the exception.
> If you want to get a guaranteed accurate snapshot, you would have to
> lock the pool when you take it, which we don't want to do.
>
> On 6/27/21 7:47 AM, ggregory@apache.org wrote:
> > This is an automated email from the ASF dual-hosted git repository.
> >
> > ggregory pushed a commit to branch master
> > in repository https://gitbox.apache.org/repos/asf/commons-pool.git
> >
> >
> > The following commit(s) were added to refs/heads/master by this push:
> >       new 26b0829  [POOL-395] Improve exception thrown in
> GenericObjectPool.borrowObject when pool is exhausted. Added
> BaseGenericObjectPool.setMessagesStatistics(boolean).
> > 26b0829 is described below
> >
> > commit 26b0829fef39729df84adb1c01eb55c944aff6d7
> > Author: Gary Gregory <ga...@gmail.com>
> > AuthorDate: Sun Jun 27 10:47:03 2021 -0400
> >
> >      [POOL-395] Improve exception thrown in
> GenericObjectPool.borrowObject
> >      when pool is exhausted. Added
> >      BaseGenericObjectPool.setMessagesStatistics(boolean).
> >
> >      - Pick up Checkstyle LineLength module from Commons IO where there
> was
> >      none before.
> >      - Message sentences start with a capital letter.
> >      - Better parameter names.
> >      - Javadoc.
> >      - Inline comments.
> > ---
> >   checkstyle.xml                                     |   4 +
> >   src/changes/changes.xml                            |   3 +
> >   .../org/apache/commons/pool2/KeyedObjectPool.java  |   4 +-
> >   .../commons/pool2/KeyedPooledObjectFactory.java    |   4 +-
> >   .../java/org/apache/commons/pool2/ObjectPool.java  |   4 +-
> >   .../apache/commons/pool2/PooledObjectFactory.java  |   4 +-
> >   .../commons/pool2/impl/BaseGenericObjectPool.java  | 114
> ++++++++++++++++-----
> >   .../commons/pool2/impl/GenericKeyedObjectPool.java |  48 +++++----
> >   .../commons/pool2/impl/GenericObjectPool.java      |  46 +++++----
> >   .../pool2/impl/SoftReferenceObjectPool.java        |   4 +-
> >   .../pool2/TestBasePoolableObjectFactory.java       |   4 +-
> >   .../pool2/impl/TestAbandonedKeyedObjectPool.java   |   4 +-
> >   .../pool2/impl/TestAbandonedObjectPool.java        |   4 +-
> >   .../pool2/impl/TestGenericKeyedObjectPool.java     |  64 +++++++-----
> >   .../commons/pool2/impl/TestGenericObjectPool.java  |  28 ++++-
> >   15 files changed, 230 insertions(+), 109 deletions(-)
> >
> > diff --git a/checkstyle.xml b/checkstyle.xml
> > index dca770b..3f884e1 100644
> > --- a/checkstyle.xml
> > +++ b/checkstyle.xml
> > @@ -74,4 +74,8 @@
> >       <property name="allowLegacy" value="true"/>
> >     </module>
> >
> > +  <module name="LineLength">
> > +    <property name="max" value="160"/>
> > +  </module>
> > +
> >   </module>
> > diff --git a/src/changes/changes.xml b/src/changes/changes.xml
> > index 40ef624..2368098 100644
> > --- a/src/changes/changes.xml
> > +++ b/src/changes/changes.xml
> > @@ -69,6 +69,9 @@ The <action> type attribute can be
> add,update,fix,remove.
> >       <action issue="POOL-396" dev="ggregory" type="add" due-to="Jeremy
> Kong, Phil Steitz">
> >         Handle validation exceptions during eviction. #85.
> >       </action>
> > +    <action issue="POOL-395" dev="ggregory" type="add" due-to="Gary
> Gregory, Arash Nikoo">
> > +      Improve exception thrown in GenericObjectPool.borrowObject when
> pool is exhausted. Added
> BaseGenericObjectPool.setMessagesStatistics(boolean).
> > +    </action>
> >       <!-- FIXES -->
> >       <action dev="ggregory" type="fix" due-to="Gary Gregory">
> >         Fix [WARNING] Old version of checkstyle detected. Consider
> updating to >= v8.30. Update Checktyle to 8.43.
> > diff --git a/src/main/java/org/apache/commons/pool2/KeyedObjectPool.java
> b/src/main/java/org/apache/commons/pool2/KeyedObjectPool.java
> > index ab52908..cac55ac 100644
> > --- a/src/main/java/org/apache/commons/pool2/KeyedObjectPool.java
> > +++ b/src/main/java/org/apache/commons/pool2/KeyedObjectPool.java
> > @@ -289,12 +289,12 @@ public interface KeyedObjectPool<K, V> extends
> Closeable {
> >        *
> >        * @param key the key used to obtain the object
> >        * @param obj a {@link #borrowObject borrowed} instance to be
> returned.
> > -     * @param mode destroy activation context provided to the factory
> > +     * @param destroyMode destroy activation context provided to the
> factory
> >        *
> >        * @throws Exception if the instance cannot be invalidated
> >        * @since 2.9.0
> >        */
> > -    default void invalidateObject(final K key, final V obj, final
> DestroyMode mode) throws Exception {
> > +    default void invalidateObject(final K key, final V obj, final
> DestroyMode destroyMode) throws Exception {
> >           invalidateObject(key, obj);
> >       }
> >
> > diff --git
> a/src/main/java/org/apache/commons/pool2/KeyedPooledObjectFactory.java
> b/src/main/java/org/apache/commons/pool2/KeyedPooledObjectFactory.java
> > index ac93114..a7f75a3 100644
> > ---
> a/src/main/java/org/apache/commons/pool2/KeyedPooledObjectFactory.java
> > +++
> b/src/main/java/org/apache/commons/pool2/KeyedPooledObjectFactory.java
> > @@ -117,7 +117,7 @@ public interface KeyedPooledObjectFactory<K, V> {
> >        *
> >        * @param key the key used when selecting the instance
> >        * @param p a {@code PooledObject} wrapping the instance to be
> destroyed
> > -     * @param mode DestroyMode providing context to the factory
> > +     * @param destroyMode DestroyMode providing context to the factory
> >        *
> >        * @throws Exception should be avoided as it may be swallowed by
> >        *    the pool implementation.
> > @@ -128,7 +128,7 @@ public interface KeyedPooledObjectFactory<K, V> {
> >        * @see DestroyMode
> >        * @since 2.9.0
> >        */
> > -    default void destroyObject(final K key, final PooledObject<V> p,
> final DestroyMode mode) throws Exception {
> > +    default void destroyObject(final K key, final PooledObject<V> p,
> final DestroyMode destroyMode) throws Exception {
> >           destroyObject(key, p);
> >       }
> >
> > diff --git a/src/main/java/org/apache/commons/pool2/ObjectPool.java
> b/src/main/java/org/apache/commons/pool2/ObjectPool.java
> > index 8225d27..8aa2d58 100644
> > --- a/src/main/java/org/apache/commons/pool2/ObjectPool.java
> > +++ b/src/main/java/org/apache/commons/pool2/ObjectPool.java
> > @@ -198,12 +198,12 @@ public interface ObjectPool<T> extends Closeable {
> >        * </p>
> >        *
> >        * @param obj a {@link #borrowObject borrowed} instance to be
> disposed.
> > -     * @param mode destroy activation context provided to the factory
> > +     * @param destroyMode destroy activation context provided to the
> factory
> >        *
> >        * @throws Exception if the instance cannot be invalidated
> >        * @since 2.9.0
> >        */
> > -    default void invalidateObject(final T obj, final DestroyMode mode)
> throws Exception {
> > +    default void invalidateObject(final T obj, final DestroyMode
> destroyMode) throws Exception {
> >           invalidateObject(obj);
> >       }
> >
> > diff --git
> a/src/main/java/org/apache/commons/pool2/PooledObjectFactory.java
> b/src/main/java/org/apache/commons/pool2/PooledObjectFactory.java
> > index 009f341..839e866 100644
> > --- a/src/main/java/org/apache/commons/pool2/PooledObjectFactory.java
> > +++ b/src/main/java/org/apache/commons/pool2/PooledObjectFactory.java
> > @@ -112,7 +112,7 @@ public interface PooledObjectFactory<T> {
> >      * DestroyMode.
> >      *
> >      * @param p a {@code PooledObject} wrapping the instance to be
> destroyed
> > -   * @param mode DestroyMode providing context to the factory
> > +   * @param destroyMode DestroyMode providing context to the factory
> >      *
> >      * @throws Exception should be avoided as it may be swallowed by
> >      *    the pool implementation.
> > @@ -123,7 +123,7 @@ public interface PooledObjectFactory<T> {
> >      * @see DestroyMode
> >      * @since 2.9.0
> >      */
> > -  default void destroyObject(final PooledObject<T> p, final DestroyMode
> mode) throws Exception {
> > +  default void destroyObject(final PooledObject<T> p, final DestroyMode
> destroyMode) throws Exception {
> >         destroyObject(p);
> >     }
> >
> > diff --git
> a/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java
> b/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java
> > index 000a6eb..83f37e1 100644
> > ---
> a/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java
> > +++
> b/src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java
> > @@ -26,9 +26,11 @@ import java.time.Duration;
> >   import java.util.Arrays;
> >   import java.util.Deque;
> >   import java.util.Iterator;
> > +import java.util.List;
> >   import java.util.TimerTask;
> >   import java.util.concurrent.ScheduledFuture;
> >   import java.util.concurrent.atomic.AtomicLong;
> > +import java.util.stream.Collectors;
> >
> >   import javax.management.InstanceAlreadyExistsException;
> >   import javax.management.InstanceNotFoundException;
> > @@ -233,12 +235,14 @@ public abstract class BaseGenericObjectPool<T>
> extends BaseObject {
> >               return builder.toString();
> >           }
> >       }
> > +
> >       /**
> >        * Maintains a cache of values for a single metric and reports
> >        * statistics on the cached values.
> >        */
> >       private class StatsStore {
> >
> > +        private static final int NULL = -1;
> >           private final AtomicLong[] values;
> >           private final int size;
> >           private int index;
> > @@ -252,7 +256,7 @@ public abstract class BaseGenericObjectPool<T>
> extends BaseObject {
> >               this.size = size;
> >               values = new AtomicLong[size];
> >               for (int i = 0; i < size; i++) {
> > -                values[i] = new AtomicLong(-1);
> > +                values[i] = new AtomicLong(NULL);
> >               }
> >           }
> >
> > @@ -275,6 +279,15 @@ public abstract class BaseGenericObjectPool<T>
> extends BaseObject {
> >           }
> >
> >           /**
> > +         * Gets the current values as a List.
> > +         *
> > +         * @return the current values as a List.
> > +         */
> > +        synchronized List<AtomicLong> getCurrentValues() {
> > +            return Arrays.stream(values, 0,
> index).collect(Collectors.toList());
> > +        }
> > +
> > +        /**
> >            * Gets the mean of the cached values.
> >            *
> >            * @return the mean of the cache, truncated to long
> > @@ -284,10 +297,9 @@ public abstract class BaseGenericObjectPool<T>
> extends BaseObject {
> >               int counter = 0;
> >               for (int i = 0; i < size; i++) {
> >                   final long value = values[i].get();
> > -                if (value != -1) {
> > +                if (value != NULL) {
> >                       counter++;
> > -                    result = result * ((counter - 1) / (double)
> counter) +
> > -                            value/(double) counter;
> > +                    result = result * ((counter - 1) / (double)
> counter) + value / (double) counter;
> >                   }
> >               }
> >               return (long) result;
> > @@ -296,16 +308,19 @@ public abstract class BaseGenericObjectPool<T>
> extends BaseObject {
> >           @Override
> >           public String toString() {
> >               final StringBuilder builder = new StringBuilder();
> > -            builder.append("StatsStore [values=");
> > -            builder.append(Arrays.toString(values));
> > -            builder.append(", size=");
> > +            builder.append("StatsStore [");
> > +            // Only append what's been filled in.
> > +            builder.append(getCurrentValues());
> > +            builder.append("], size=");
> >               builder.append(size);
> >               builder.append(", index=");
> >               builder.append(index);
> >               builder.append("]");
> >               return builder.toString();
> >           }
> > +
> >       }
> > +
> >       // Constants
> >       /**
> >        * The size of the caches used to store historical data for some
> attributes
> > @@ -334,7 +349,6 @@ public abstract class BaseGenericObjectPool<T>
> extends BaseObject {
> >       final Object closeLock = new Object();
> >       volatile boolean closed;
> >
> > -
> >       final Object evictionLock = new Object();
> >       private Evictor evictor = null; // @GuardedBy("evictionLock")
> >       EvictionIterator evictionIterator = null; //
> @GuardedBy("evictionLock")
> > @@ -348,21 +362,21 @@ public abstract class BaseGenericObjectPool<T>
> extends BaseObject {
> >       // Monitoring (primarily JMX) attributes
> >       private final ObjectName objectName;
> >       private final String creationStackTrace;
> > -    private final AtomicLong borrowedCount = new AtomicLong(0);
> > -    private final AtomicLong returnedCount = new AtomicLong(0);
> > -    final AtomicLong createdCount = new AtomicLong(0);
> > -    final AtomicLong destroyedCount = new AtomicLong(0);
> > -    final AtomicLong destroyedByEvictorCount = new AtomicLong(0);
> > -    final AtomicLong destroyedByBorrowValidationCount = new
> AtomicLong(0);
> > +    private final AtomicLong borrowedCount = new AtomicLong();
> > +    private final AtomicLong returnedCount = new AtomicLong();
> > +    final AtomicLong createdCount = new AtomicLong();
> > +    final AtomicLong destroyedCount = new AtomicLong();
> > +    final AtomicLong destroyedByEvictorCount = new AtomicLong();
> > +    final AtomicLong destroyedByBorrowValidationCount = new
> AtomicLong();
> > +
> >       private final StatsStore activeTimes = new
> StatsStore(MEAN_TIMING_STATS_CACHE_SIZE);
> > -
> >       private final StatsStore idleTimes = new
> StatsStore(MEAN_TIMING_STATS_CACHE_SIZE);
> > -
> >       private final StatsStore waitTimes = new
> StatsStore(MEAN_TIMING_STATS_CACHE_SIZE);
> >
> > -    private final AtomicLong maxBorrowWaitTimeMillis = new
> AtomicLong(0L);
> > +    private final AtomicLong maxBorrowWaitTimeMillis = new AtomicLong();
> >
> > -    private volatile SwallowedExceptionListener
> swallowedExceptionListener = null;
> > +    private volatile SwallowedExceptionListener
> swallowedExceptionListener;
> > +    private volatile boolean messageStatistics;
> >
> >       /**
> >        * Handles JMX registration (if required) and the initialization
> required for
> > @@ -396,6 +410,16 @@ public abstract class BaseGenericObjectPool<T>
> extends BaseObject {
> >       }
> >
> >       /**
> > +     * Appends statistics if enabled.
> > +     *
> > +     * @param string The root string.
> > +     * @return The root string plus statistics.
> > +     */
> > +    String appendStats(final String string) {
> > +        return messageStatistics ? string + ", " + getStatsString() :
> string;
> > +    }
> > +
> > +    /**
> >        * Verifies that the pool is open.
> >        * @throws IllegalStateException if the pool is closed.
> >        */
> > @@ -680,6 +704,16 @@ public abstract class BaseGenericObjectPool<T>
> extends BaseObject {
> >       }
> >
> >       /**
> > +     * Gets whether to include statistics in exception messages.
> > +     *
> > +     * @return whether to include statistics in exception messages.
> > +     * @since 2.11.0
> > +     */
> > +    public boolean getMessageStatistics() {
> > +        return messageStatistics;
> > +    }
> > +
> > +    /**
> >        * Gets the minimum amount of time an object may sit idle in the
> pool
> >        * before it is eligible for eviction by the idle object evictor
> (if any -
> >        * see {@link #setTimeBetweenEvictionRuns(Duration)}). When
> non-positive,
> > @@ -806,6 +840,22 @@ public abstract class BaseGenericObjectPool<T>
> extends BaseObject {
> >       }
> >
> >       /**
> > +     * Gets a statistics string.
> > +     *
> > +     * @return  a statistics string.
> > +     */
> > +    String getStatsString() {
> > +        // Simply listed in AB order.
> > +        return String.format(
> > +                "activeTimes=%s, blockWhenExhausted=%s,
> borrowedCount=%,d, closed=%s, createdCount=%,d,
> destroyedByBorrowValidationCount=%,d, destroyedByEvictorCount=%,d,
> evictorShutdownTimeout=%s, fairness=%s, idleTimes=%s, lifo=%s,
> maxBorrowWaitTimeMillis=%,d, maxTotal=%s, maxWaitDuration=%s,
> minEvictableIdleTime=%s, numTestsPerEvictionRun=%s, returnedCount=%s,
> softMinEvictableIdleTime=%s, testOnBorrow=%s, testOnCreate=%s,
> testOnReturn=%s, testWhileIdle=%s, timeBetweenEvictionRuns=%s,  [...]
> > +                activeTimes.getCurrentValues(), blockWhenExhausted,
> borrowedCount.get(), closed, createdCount.get(),
> > +                destroyedByBorrowValidationCount.get(),
> destroyedByEvictorCount.get(), evictorShutdownTimeout, fairness,
> > +                idleTimes.getCurrentValues(), lifo,
> maxBorrowWaitTimeMillis.get(), maxTotal, maxWaitDuration,
> > +                minEvictableIdleTime, numTestsPerEvictionRun,
> returnedCount, softMinEvictableIdleTime, testOnBorrow,
> > +                testOnCreate, testOnReturn, testWhileIdle,
> timeBetweenEvictionRuns, waitTimes.getCurrentValues());
> > +    }
> > +
> > +    /**
> >        * The listener used (if any) to receive notifications of
> exceptions
> >        * unavoidably swallowed by the pool.
> >        *
> > @@ -984,8 +1034,7 @@ public abstract class BaseGenericObjectPool<T>
> extends BaseObject {
> >       final void jmxUnregister() {
> >           if (objectName != null) {
> >               try {
> > -
> ManagementFactory.getPlatformMBeanServer().unregisterMBean(
> > -                        objectName);
> > +
> ManagementFactory.getPlatformMBeanServer().unregisterMBean(objectName);
> >               } catch (final MBeanRegistrationException |
> InstanceNotFoundException e) {
> >                   swallowException(e);
> >               }
> > @@ -997,10 +1046,9 @@ public abstract class BaseGenericObjectPool<T>
> extends BaseObject {
> >        * @param pooledObject instance to return to the keyed pool
> >        */
> >       protected void markReturningState(final PooledObject<T>
> pooledObject) {
> > -        synchronized(pooledObject) {
> > +        synchronized (pooledObject) {
> >               if (pooledObject.getState() !=
> PooledObjectState.ALLOCATED) {
> > -                throw new IllegalStateException(
> > -                        "Object has already been returned to this pool
> or is invalid");
> > +                throw new IllegalStateException("Object has already
> been returned to this pool or is invalid");
> >               }
> >               pooledObject.markReturning(); // Keep from being marked
> abandoned
> >           }
> > @@ -1121,9 +1169,9 @@ public abstract class BaseGenericObjectPool<T>
> extends BaseObject {
> >                       classLoader + ", " + epClassLoader + "] do not
> implement " + EVICTION_POLICY_TYPE_NAME);
> >           } catch (final ClassNotFoundException | InstantiationException
> | IllegalAccessException |
> >                   InvocationTargetException | NoSuchMethodException e) {
> > -            final String exMessage = "Unable to create " +
> EVICTION_POLICY_TYPE_NAME + " instance of type " +
> > -                    evictionPolicyClassName;
> > -            throw new IllegalArgumentException(exMessage, e);
> > +            throw new IllegalArgumentException(
> > +                    "Unable to create " + EVICTION_POLICY_TYPE_NAME + "
> instance of type " + evictionPolicyClassName,
> > +                    e);
> >           }
> >       }
> >
> > @@ -1223,6 +1271,16 @@ public abstract class BaseGenericObjectPool<T>
> extends BaseObject {
> >       }
> >
> >       /**
> > +     * Sets whether to include statistics in exception messages.
> > +     *
> > +     * @param messagesDetails whether to include statistics in
> exception messages.
> > +     * @since 2.11.0
> > +     */
> > +    public void setMessagesStatistics(boolean messagesDetails) {
> > +        this.messageStatistics = messagesDetails;
> > +    }
> > +
> > +    /**
> >        * Sets the minimum amount of time an object may sit idle in the
> pool
> >        * before it is eligible for eviction by the idle object evictor
> (if any -
> >        * see {@link #setTimeBetweenEvictionRuns(Duration)}). When
> non-positive,
> > @@ -1446,6 +1504,8 @@ public abstract class BaseGenericObjectPool<T>
> extends BaseObject {
> >
>  setTimeBetweenEvictionRuns(Duration.ofMillis(timeBetweenEvictionRunsMillis));
> >       }
> >
> > +    // Inner classes
> > +
> >       /**
> >        * <p>Starts the evictor with the given delay. If there is an
> evictor
> >        * running when this method is called, it is stopped and replaced
> with a
> > @@ -1478,8 +1538,6 @@ public abstract class BaseGenericObjectPool<T>
> extends BaseObject {
> >           }
> >       }
> >
> > -    // Inner classes
> > -
> >       /**
> >        * Stops the evictor.
> >        */
> > diff --git
> a/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
> b/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
> > index 11fd633..64274bd 100644
> > ---
> a/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
> > +++
> b/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
> > @@ -121,7 +121,7 @@ public class GenericKeyedObjectPool<K, T> extends
> BaseGenericObjectPool<T>
> >            * Invariant: empty keyed pool will not be dropped unless
> numInterested
> >            *            is 0.
> >            */
> > -        private final AtomicLong numInterested = new AtomicLong(0);
> > +        private final AtomicLong numInterested = new AtomicLong();
> >
> >           /**
> >            * Constructs a new ObjecDeque with the given fairness policy.
> > @@ -267,7 +267,7 @@ public class GenericKeyedObjectPool<K, T> extends
> BaseGenericObjectPool<T>
> >
> >           if (factory == null) {
> >               jmxUnregister(); // tidy up
> > -            throw new IllegalArgumentException("factory may not be
> null");
> > +            throw new IllegalArgumentException("Factory may not be
> null");
> >           }
> >           this.factory = factory;
> >           this.fairness = config.getFairness();
> > @@ -449,16 +449,15 @@ public class GenericKeyedObjectPool<K, T> extends
> BaseGenericObjectPool<T>
> >                           if (borrowMaxWaitMillis < 0) {
> >                               p =
> objectDeque.getIdleObjects().takeFirst();
> >                           } else {
> > -                            p = objectDeque.getIdleObjects().pollFirst(
> > -                                    borrowMaxWaitMillis,
> TimeUnit.MILLISECONDS);
> > +                            p =
> objectDeque.getIdleObjects().pollFirst(borrowMaxWaitMillis,
> TimeUnit.MILLISECONDS);
> >                           }
> >                       }
> >                       if (p == null) {
> > -                        throw new NoSuchElementException(
> > -                                "Timeout waiting for idle object");
> > +                        throw new NoSuchElementException(appendStats(
> > +                                "Timeout waiting for idle object,
> borrowMaxWaitMillis=" + borrowMaxWaitMillis));
> >                       }
> >                   } else if (p == null) {
> > -                    throw new NoSuchElementException("Pool exhausted");
> > +                    throw new NoSuchElementException(appendStats("Pool
> exhausted"));
> >                   }
> >                   if (!p.allocate()) {
> >                       p = null;
> > @@ -475,8 +474,7 @@ public class GenericKeyedObjectPool<K, T> extends
> BaseGenericObjectPool<T>
> >                           }
> >                           p = null;
> >                           if (create) {
> > -                            final NoSuchElementException nsee = new
> NoSuchElementException(
> > -                                    "Unable to activate object");
> > +                            final NoSuchElementException nsee = new
> NoSuchElementException(appendStats("Unable to activate object"));
> >                               nsee.initCause(e);
> >                               throw nsee;
> >                           }
> > @@ -500,7 +498,7 @@ public class GenericKeyedObjectPool<K, T> extends
> BaseGenericObjectPool<T>
> >                               p = null;
> >                               if (create) {
> >                                   final NoSuchElementException nsee =
> new NoSuchElementException(
> > -                                        "Unable to validate object");
> > +                                        appendStats("Unable to validate
> object"));
> >                                   nsee.initCause(validationThrowable);
> >                                   throw nsee;
> >                               }
> > @@ -517,6 +515,13 @@ public class GenericKeyedObjectPool<K, T> extends
> BaseGenericObjectPool<T>
> >           return p.getObject();
> >       }
> >
> > +    @Override
> > +    String getStatsString() {
> > +        // Simply listed in AB order.
> > +        return super.getStatsString() +
> > +                String.format(", fairness=%s, maxIdlePerKey%,d,
> maxTotalPerKey=%,d, minIdlePerKey=%,d, numTotal=%,d",
> > +                        fairness, maxIdlePerKey, maxTotalPerKey,
> minIdlePerKey, numTotal.get());
> > +    }
> >
> >       /**
> >        * Calculate the number of objects that need to be created to
> attempt to
> > @@ -820,7 +825,7 @@ public class GenericKeyedObjectPool<K, T> extends
> BaseGenericObjectPool<T>
> >       /**
> >        * De-register the use of a key by an object.
> >        * <p>
> > -     * register() and deregister() must always be used as a pair.
> > +     * {@link #register()} and {@link #deregister()} must always be
> used as a pair.
> >        * </p>
> >        *
> >        * @param k The key to de-register
> > @@ -858,12 +863,12 @@ public class GenericKeyedObjectPool<K, T> extends
> BaseGenericObjectPool<T>
> >        * @param toDestroy The wrapped object to be destroyed
> >        * @param always Should the object be destroyed even if it is not
> currently
> >        *               in the set of idle objects for the given key
> > -     * @param mode DestroyMode context provided to the factory
> > +     * @param destroyMode DestroyMode context provided to the factory
> >        *
> >        * @return {@code true} if the object was destroyed, otherwise
> {@code false}
> >        * @throws Exception If the object destruction failed
> >        */
> > -    private boolean destroy(final K key, final PooledObject<T>
> toDestroy, final boolean always, final DestroyMode mode)
> > +    private boolean destroy(final K key, final PooledObject<T>
> toDestroy, final boolean always, final DestroyMode destroyMode)
> >               throws Exception {
> >
> >           final ObjectDeque<T> objectDeque = register(key);
> > @@ -884,7 +889,7 @@ public class GenericKeyedObjectPool<K, T> extends
> BaseGenericObjectPool<T>
> >                   toDestroy.invalidate();
> >
> >                   try {
> > -                    factory.destroyObject(key, toDestroy, mode);
> > +                    factory.destroyObject(key, toDestroy, destroyMode);
> >                   } finally {
> >                       objectDeque.getCreateCount().decrementAndGet();
> >                       destroyedCount.incrementAndGet();
> > @@ -1414,7 +1419,7 @@ public class GenericKeyedObjectPool<K, T> extends
> BaseGenericObjectPool<T>
> >        *
> >        * @param key pool key
> >        * @param obj instance to invalidate
> > -     * @param mode DestroyMode context provided to factory
> > +     * @param destroyMode DestroyMode context provided to factory
> >        *
> >        * @throws Exception             if an exception occurs destroying
> the
> >        *                               object
> > @@ -1423,15 +1428,15 @@ public class GenericKeyedObjectPool<K, T>
> extends BaseGenericObjectPool<T>
> >        * @since 2.9.0
> >        */
> >       @Override
> > -    public void invalidateObject(final K key, final T obj, final
> DestroyMode mode) throws Exception {
> > +    public void invalidateObject(final K key, final T obj, final
> DestroyMode destroyMode) throws Exception {
> >           final ObjectDeque<T> objectDeque = poolMap.get(key);
> >           final PooledObject<T> p = objectDeque.getAllObjects().get(new
> IdentityWrapper<>(obj));
> >           if (p == null) {
> > -            throw new IllegalStateException("Object not currently part
> of this pool");
> > +            throw new IllegalStateException(appendStats("Object not
> currently part of this pool"));
> >           }
> >           synchronized (p) {
> >               if (p.getState() != PooledObjectState.INVALID) {
> > -                destroy(key, p, true, mode);
> > +                destroy(key, p, true, destroyMode);
> >               }
> >           }
> >           if (objectDeque.idleObjects.hasTakeWaiters()) {
> > @@ -1502,7 +1507,7 @@ public class GenericKeyedObjectPool<K, T> extends
> BaseGenericObjectPool<T>
> >       /**
> >        * Register the use of a key by an object.
> >        * <p>
> > -     * register() and deregister() must always be used as a pair.
> > +     * {@link #register()} and {@link #deregister()} must always be
> used as a pair.
> >        * </p>
> >        *
> >        * @param k The key to register
> > @@ -1579,6 +1584,7 @@ public class GenericKeyedObjectPool<K, T> extends
> BaseGenericObjectPool<T>
> >                   try {
> >                       invalidateObject(pool.getKey(),
> pooledObject.getObject(), DestroyMode.ABANDONED);
> >                   } catch (final Exception e) {
> > +                    // TODO handle/log?
> >                       e.printStackTrace();
> >                   }
> >               }
> > @@ -1828,9 +1834,9 @@ public class GenericKeyedObjectPool<K, T> extends
> BaseGenericObjectPool<T>
> >        *
> >        * @param minIdlePerKey The minimum size of the each keyed pool
> >        *
> > -     * @see #getMinIdlePerKey
> > +     * @see #getMinIdlePerKey()
> >        * @see #getMaxIdlePerKey()
> > -     * @see #setTimeBetweenEvictionRunsMillis
> > +     * @see #setTimeBetweenEvictionRuns(Duration)
> >        */
> >       public void setMinIdlePerKey(final int minIdlePerKey) {
> >           this.minIdlePerKey = minIdlePerKey;
> > diff --git
> a/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
> b/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
> > index ee27b58..c5fed24 100644
> > --- a/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
> > +++ b/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
> > @@ -115,7 +115,7 @@ public class GenericObjectPool<T> extends
> BaseGenericObjectPool<T>
> >        * {@link #create()} will ensure that there are never more than
> >        * {@link #_maxActive} objects created at any one time.
> >        */
> > -    private final AtomicLong createCount = new AtomicLong(0);
> > +    private final AtomicLong createCount = new AtomicLong();
> >
> >       private long makeObjectCount;
> >
> > @@ -155,7 +155,7 @@ public class GenericObjectPool<T> extends
> BaseGenericObjectPool<T>
> >
> >           if (factory == null) {
> >               jmxUnregister(); // tidy up
> > -            throw new IllegalArgumentException("factory may not be
> null");
> > +            throw new IllegalArgumentException("Factory may not be
> null");
> >           }
> >           this.factory = factory;
> >
> > @@ -214,8 +214,7 @@ public class GenericObjectPool<T> extends
> BaseGenericObjectPool<T>
> >       public void addObject() throws Exception {
> >           assertOpen();
> >           if (factory == null) {
> > -            throw new IllegalStateException(
> > -                    "Cannot add objects without a factory.");
> > +            throw new IllegalStateException("Cannot add objects without
> a factory.");
> >           }
> >           final PooledObject<T> p = create();
> >           addIdleObject(p);
> > @@ -271,7 +270,7 @@ public class GenericObjectPool<T> extends
> BaseGenericObjectPool<T>
> >        * available instances in request arrival order.
> >        * </p>
> >        *
> > -     * @param borrowMaxWait The time to wait for an object
> > +     * @param borrowMaxWaitDuration The time to wait for an object
> >        *                            to become available
> >        *
> >        * @return object instance from the pool
> > @@ -282,11 +281,11 @@ public class GenericObjectPool<T> extends
> BaseGenericObjectPool<T>
> >        *                   error
> >        * @since 2.10.0
> >        */
> > -    public T borrowObject(final Duration borrowMaxWait) throws
> Exception {
> > +    public T borrowObject(final Duration borrowMaxWaitDuration) throws
> Exception {
> >           assertOpen();
> >
> >           final AbandonedConfig ac = this.abandonedConfig;
> > -        if (ac != null && ac.getRemoveAbandonedOnBorrow() &&
> (getNumIdle() < 2) &&
> > +        if (ac != null && ac.getRemoveAbandonedOnBorrow() &&
> (getNumIdle() < 2) &&
> >                   (getNumActive() > getMaxTotal() - 3)) {
> >               removeAbandoned(ac);
> >           }
> > @@ -311,17 +310,18 @@ public class GenericObjectPool<T> extends
> BaseGenericObjectPool<T>
> >               }
> >               if (blockWhenExhausted) {
> >                   if (p == null) {
> > -                    if (borrowMaxWait.isNegative()) {
> > +                    if (borrowMaxWaitDuration.isNegative()) {
> >                           p = idleObjects.takeFirst();
> >                       } else {
> > -                        p = idleObjects.pollFirst(borrowMaxWait);
> > +                        p =
> idleObjects.pollFirst(borrowMaxWaitDuration);
> >                       }
> >                   }
> >                   if (p == null) {
> > -                    throw new NoSuchElementException("Timeout waiting
> for idle object");
> > +                    throw new NoSuchElementException(appendStats(
> > +                            "Timeout waiting for idle object,
> borrowMaxWaitMillis=" + borrowMaxWaitDuration));
> >                   }
> >               } else if (p == null) {
> > -                throw new NoSuchElementException("Pool exhausted");
> > +                throw new NoSuchElementException(appendStats("Pool
> exhausted"));
> >               }
> >               if (!p.allocate()) {
> >                   p = null;
> > @@ -338,7 +338,8 @@ public class GenericObjectPool<T> extends
> BaseGenericObjectPool<T>
> >                       }
> >                       p = null;
> >                       if (create) {
> > -                        final NoSuchElementException nsee = new
> NoSuchElementException("Unable to activate object");
> > +                        final NoSuchElementException nsee = new
> NoSuchElementException(
> > +                                appendStats("Unable to activate
> object"));
> >                           nsee.initCause(e);
> >                           throw nsee;
> >                       }
> > @@ -361,7 +362,8 @@ public class GenericObjectPool<T> extends
> BaseGenericObjectPool<T>
> >                           }
> >                           p = null;
> >                           if (create) {
> > -                            final NoSuchElementException nsee = new
> NoSuchElementException("Unable to validate object");
> > +                            final NoSuchElementException nsee = new
> NoSuchElementException(
> > +                                    appendStats("Unable to validate
> object"));
> >                               nsee.initCause(validationThrowable);
> >                               throw nsee;
> >                           }
> > @@ -375,6 +377,14 @@ public class GenericObjectPool<T> extends
> BaseGenericObjectPool<T>
> >           return p.getObject();
> >       }
> >
> > +    @Override
> > +    String getStatsString() {
> > +        // Simply listed in AB order.
> > +        return super.getStatsString() +
> > +                String.format(", createdCount=%,d, makeObjectCount=%,d,
> maxIdle=%,d, minIdle=%,d",
> > +                        createdCount.get(), makeObjectCount, maxIdle,
> minIdle);
> > +    }
> > +
> >       /**
> >        * Borrows an object from the pool using the specific waiting time
> which only
> >        * applies if {@link #getBlockWhenExhausted()} is true.
> > @@ -592,17 +602,17 @@ public class GenericObjectPool<T> extends
> BaseGenericObjectPool<T>
> >        * Destroys a wrapped pooled object.
> >        *
> >        * @param toDestroy The wrapped pooled object to destroy
> > -     * @param mode DestroyMode context provided to the factory
> > +     * @param destroyMode DestroyMode context provided to the factory
> >        *
> >        * @throws Exception If the factory fails to destroy the pooled
> object
> >        *                   cleanly
> >        */
> > -    private void destroy(final PooledObject<T> toDestroy, final
> DestroyMode mode) throws Exception {
> > +    private void destroy(final PooledObject<T> toDestroy, final
> DestroyMode destroyMode) throws Exception {
> >           toDestroy.invalidate();
> >           idleObjects.remove(toDestroy);
> >           allObjects.remove(new
> IdentityWrapper<>(toDestroy.getObject()));
> >           try {
> > -            factory.destroyObject(toDestroy, mode);
> > +            factory.destroyObject(toDestroy, destroyMode);
> >           } finally {
> >               destroyedCount.incrementAndGet();
> >               createCount.decrementAndGet();
> > @@ -1002,7 +1012,7 @@ public class GenericObjectPool<T> extends
> BaseGenericObjectPool<T>
> >        * @since 2.9.0
> >        */
> >       @Override
> > -    public void invalidateObject(final T obj, final DestroyMode mode)
> throws Exception {
> > +    public void invalidateObject(final T obj, final DestroyMode
> destroyMode) throws Exception {
> >           final PooledObject<T> p = allObjects.get(new
> IdentityWrapper<>(obj));
> >           if (p == null) {
> >               if (isAbandonedConfig()) {
> > @@ -1013,7 +1023,7 @@ public class GenericObjectPool<T> extends
> BaseGenericObjectPool<T>
> >           }
> >           synchronized (p) {
> >               if (p.getState() != PooledObjectState.INVALID) {
> > -                destroy(p, mode);
> > +                destroy(p, destroyMode);
> >               }
> >           }
> >           ensureIdle(1, false);
> > diff --git
> a/src/main/java/org/apache/commons/pool2/impl/SoftReferenceObjectPool.java
> b/src/main/java/org/apache/commons/pool2/impl/SoftReferenceObjectPool.java
> > index 849629f..f7c271c 100644
> > ---
> a/src/main/java/org/apache/commons/pool2/impl/SoftReferenceObjectPool.java
> > +++
> b/src/main/java/org/apache/commons/pool2/impl/SoftReferenceObjectPool.java
> > @@ -216,9 +216,7 @@ public class SoftReferenceObjectPool<T> extends
> BaseObjectPool<T> {
> >                           obj = null;
> >                       }
> >                       if (newlyCreated) {
> > -                        throw new NoSuchElementException(
> > -                                "Could not create a validated object,
> cause: " +
> > -                                        t.getMessage());
> > +                        throw new NoSuchElementException("Could not
> create a validated object, cause: " + t);
> >                       }
> >                   }
> >               }
> > diff --git
> a/src/test/java/org/apache/commons/pool2/TestBasePoolableObjectFactory.java
> b/src/test/java/org/apache/commons/pool2/TestBasePoolableObjectFactory.java
> > index 97c411e..43935d5 100644
> > ---
> a/src/test/java/org/apache/commons/pool2/TestBasePoolableObjectFactory.java
> > +++
> b/src/test/java/org/apache/commons/pool2/TestBasePoolableObjectFactory.java
> > @@ -34,8 +34,8 @@ public class TestBasePoolableObjectFactory {
> >               return new AtomicInteger(0);
> >           }
> >           @Override
> > -        public void destroyObject(final PooledObject<AtomicInteger> p,
> final DestroyMode mode){
> > -            if (mode.equals(DestroyMode.ABANDONED)) {
> > +        public void destroyObject(final PooledObject<AtomicInteger> p,
> final DestroyMode destroyMode){
> > +            if (destroyMode.equals(DestroyMode.ABANDONED)) {
> >                   p.getObject().incrementAndGet();
> >               }
> >           }
> > diff --git
> a/src/test/java/org/apache/commons/pool2/impl/TestAbandonedKeyedObjectPool.java
> b/src/test/java/org/apache/commons/pool2/impl/TestAbandonedKeyedObjectPool.java
> > index 541e1ea..0d850c9 100644
> > ---
> a/src/test/java/org/apache/commons/pool2/impl/TestAbandonedKeyedObjectPool.java
> > +++
> b/src/test/java/org/apache/commons/pool2/impl/TestAbandonedKeyedObjectPool.java
> > @@ -103,7 +103,7 @@ public class TestAbandonedKeyedObjectPool {
> >           }
> >
> >           @Override
> > -        public void destroyObject(final Integer key, final
> PooledObject<PooledTestObject> obj, final DestroyMode mode) throws
> Exception {
> > +        public void destroyObject(final Integer key, final
> PooledObject<PooledTestObject> obj, final DestroyMode destroyMode) throws
> Exception {
> >               obj.getObject().setActive(false);
> >               // while destroying instances, yield control to other
> threads
> >               // helps simulate threading errors
> > @@ -111,7 +111,7 @@ public class TestAbandonedKeyedObjectPool {
> >               if (destroyLatency != 0) {
> >                   Thread.sleep(destroyLatency);
> >               }
> > -            obj.getObject().destroy(mode);
> > +            obj.getObject().destroy(destroyMode);
> >           }
> >
> >           @Override
> > diff --git
> a/src/test/java/org/apache/commons/pool2/impl/TestAbandonedObjectPool.java
> b/src/test/java/org/apache/commons/pool2/impl/TestAbandonedObjectPool.java
> > index 14b264c..9b64560 100644
> > ---
> a/src/test/java/org/apache/commons/pool2/impl/TestAbandonedObjectPool.java
> > +++
> b/src/test/java/org/apache/commons/pool2/impl/TestAbandonedObjectPool.java
> > @@ -183,7 +183,7 @@ public class TestAbandonedObjectPool {
> >           }
> >
> >           @Override
> > -        public void destroyObject(final PooledObject<PooledTestObject>
> obj, final DestroyMode mode) throws Exception {
> > +        public void destroyObject(final PooledObject<PooledTestObject>
> obj, final DestroyMode destroyMode) throws Exception {
> >               obj.getObject().setActive(false);
> >               // while destroying instances, yield control to other
> threads
> >               // helps simulate threading errors
> > @@ -191,7 +191,7 @@ public class TestAbandonedObjectPool {
> >               if (destroyLatency != 0) {
> >                   Thread.sleep(destroyLatency);
> >               }
> > -            obj.getObject().destroy(mode);
> > +            obj.getObject().destroy(destroyMode);
> >           }
> >
> >           @Override
> > diff --git
> a/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
> b/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
> > index 39e898c..cd36876 100644
> > ---
> a/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
> > +++
> b/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
> > @@ -20,6 +20,7 @@ package org.apache.commons.pool2.impl;
> >
> >   import static org.junit.jupiter.api.Assertions.assertEquals;
> >   import static org.junit.jupiter.api.Assertions.assertFalse;
> > +import static org.junit.jupiter.api.Assertions.assertNotEquals;
> >   import static org.junit.jupiter.api.Assertions.assertNotNull;
> >   import static org.junit.jupiter.api.Assertions.assertNotSame;
> >   import static org.junit.jupiter.api.Assertions.assertSame;
> > @@ -906,6 +907,18 @@ public class TestGenericKeyedObjectPool extends
> TestKeyedObjectPool {
> >       }
> >
> >       @Test
> > +    public void testAppendStats() {
> > +        assertFalse(gkoPool.getMessageStatistics());
> > +        assertEquals("foo", (gkoPool.appendStats("foo")));
> > +        try (final GenericKeyedObjectPool<?, ?> pool = new
> GenericKeyedObjectPool<>(new SimpleFactory<>())) {
> > +            pool.setMessagesStatistics(true);
> > +            assertNotEquals("foo", (pool.appendStats("foo")));
> > +            pool.setMessagesStatistics(false);
> > +            assertEquals("foo", (pool.appendStats("foo")));
> > +        }
> > +    }
> > +
> > +    @Test
> >       @Timeout(value = 60000, unit = TimeUnit.MILLISECONDS)
> >       public void testBlockedKeyDoesNotBlockPool() throws Exception {
> >           gkoPool.setBlockWhenExhausted(true);
> > @@ -1025,6 +1038,7 @@ public class TestGenericKeyedObjectPool extends
> TestKeyedObjectPool {
> >           gkoPool.close();
> >       }
> >
> > +
> >       /**
> >        * Test to make sure that clearOldest does not destroy instances
> that have been checked out.
> >        *
> > @@ -1061,7 +1075,6 @@ public class TestGenericKeyedObjectPool extends
> TestKeyedObjectPool {
> >           }
> >       }
> >
> > -
> >       // POOL-259
> >       @Test
> >       public void testClientWaitStats() throws Exception {
> > @@ -1483,6 +1496,27 @@ public class TestGenericKeyedObjectPool extends
> TestKeyedObjectPool {
> >
> >       @Test
> >       @Timeout(value = 60000, unit = TimeUnit.MILLISECONDS)
> > +    public void testExceptionInValidationDuringEviction() throws
> Exception {
> > +        gkoPool.setMaxIdlePerKey(1);
> > +        gkoPool.setMinEvictableIdleTime(Duration.ZERO);
> > +        gkoPool.setTestWhileIdle(true);
> > +
> > +        final String obj = gkoPool.borrowObject("one");
> > +        gkoPool.returnObject("one", obj);
> > +
> > +        simpleFactory.setThrowExceptionOnValidate(true);
> > +        try {
> > +            gkoPool.evict();
> > +            fail("Expecting RuntimeException");
> > +        } catch (final RuntimeException e) {
> > +            // expected
> > +        }
> > +        assertEquals(0, gkoPool.getNumActive());
> > +        assertEquals(0, gkoPool.getNumIdle());
> > +    }
> > +
> > +    @Test
> > +    @Timeout(value = 60000, unit = TimeUnit.MILLISECONDS)
> >       public void testExceptionOnActivateDuringBorrow() throws Exception
> {
> >           final String obj1 = gkoPool.borrowObject("one");
> >           final String obj2 = gkoPool.borrowObject("one");
> > @@ -1514,6 +1548,7 @@ public class TestGenericKeyedObjectPool extends
> TestKeyedObjectPool {
> >           assertEquals(0, gkoPool.getNumIdle());
> >       }
> >
> > +
> >       @Test
> >       @Timeout(value = 60000, unit = TimeUnit.MILLISECONDS)
> >       public void testExceptionOnDestroyDuringBorrow() throws Exception {
> > @@ -1550,7 +1585,6 @@ public class TestGenericKeyedObjectPool extends
> TestKeyedObjectPool {
> >           assertEquals(0, gkoPool.getNumIdle());
> >       }
> >
> > -
> >       @Test
> >       @Timeout(value = 60000, unit = TimeUnit.MILLISECONDS)
> >       public void testExceptionOnPassivateDuringReturn() throws
> Exception {
> > @@ -1563,27 +1597,6 @@ public class TestGenericKeyedObjectPool extends
> TestKeyedObjectPool {
> >
> >       @Test
> >       @Timeout(value = 60000, unit = TimeUnit.MILLISECONDS)
> > -    public void testExceptionInValidationDuringEviction() throws
> Exception {
> > -        gkoPool.setMaxIdlePerKey(1);
> > -        gkoPool.setMinEvictableIdleTime(Duration.ZERO);
> > -        gkoPool.setTestWhileIdle(true);
> > -
> > -        final String obj = gkoPool.borrowObject("one");
> > -        gkoPool.returnObject("one", obj);
> > -
> > -        simpleFactory.setThrowExceptionOnValidate(true);
> > -        try {
> > -            gkoPool.evict();
> > -            fail("Expecting RuntimeException");
> > -        } catch (final RuntimeException e) {
> > -            // expected
> > -        }
> > -        assertEquals(0, gkoPool.getNumActive());
> > -        assertEquals(0, gkoPool.getNumIdle());
> > -    }
> > -
> > -    @Test
> > -    @Timeout(value = 60000, unit = TimeUnit.MILLISECONDS)
> >       public void testFIFO() throws Exception {
> >           gkoPool.setLifo(false);
> >           final String key = "key";
> > @@ -1600,6 +1613,11 @@ public class TestGenericKeyedObjectPool extends
> TestKeyedObjectPool {
> >           assertEquals( "key4", gkoPool.borrowObject(key),"new-4");
> >       }
> >
> > +    @Test
> > +    public void testGetStatsString() {
> > +        assertNotNull((gkoPool.getStatsString()));
> > +    }
> > +
> >       /**
> >        * Verify that threads waiting on a depleted pool get served when
> a checked out object is
> >        * invalidated.
> > diff --git
> a/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
> b/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
> > index 1014588..fe646a3 100644
> > ---
> a/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
> > +++
> b/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
> > @@ -20,6 +20,7 @@ package org.apache.commons.pool2.impl;
> >
> >   import static org.junit.jupiter.api.Assertions.assertEquals;
> >   import static org.junit.jupiter.api.Assertions.assertFalse;
> > +import static org.junit.jupiter.api.Assertions.assertNotEquals;
> >   import static org.junit.jupiter.api.Assertions.assertNotNull;
> >   import static org.junit.jupiter.api.Assertions.assertNull;
> >   import static org.junit.jupiter.api.Assertions.assertThrows;
> > @@ -58,6 +59,7 @@ import org.apache.commons.pool2.VisitTracker;
> >   import org.apache.commons.pool2.VisitTrackerFactory;
> >   import org.apache.commons.pool2.Waiter;
> >   import org.apache.commons.pool2.WaiterFactory;
> > +import
> org.apache.commons.pool2.impl.TestGenericKeyedObjectPool.SimpleFactory;
> >   import org.junit.jupiter.api.AfterEach;
> >   import org.junit.jupiter.api.BeforeEach;
> >   import org.junit.jupiter.api.Test;
> > @@ -83,7 +85,9 @@ public class TestGenericObjectPool extends
> TestBaseObjectPool {
> >                   } else {
> >                       genericObjectPool.evict();
> >                   }
> > -            } catch (final Exception e) { /* Ignore */}
> > +            } catch (final Exception e) {
> > +                // Ignore.
> > +            }
> >           }
> >       }
> >
> > @@ -999,6 +1003,18 @@ public class TestGenericObjectPool extends
> TestBaseObjectPool {
> >           assertEquals( 0, genericObjectPool.getNumActive(),"should be
> zero active");
> >       }
> >
> > +    @Test
> > +    public void testAppendStats() {
> > +        assertFalse(genericObjectPool.getMessageStatistics());
> > +        assertEquals("foo", (genericObjectPool.appendStats("foo")));
> > +        try (final GenericObjectPool<?> pool = new
> GenericObjectPool<>(new SimpleFactory())) {
> > +            pool.setMessagesStatistics(true);
> > +            assertNotEquals("foo", (pool.appendStats("foo")));
> > +            pool.setMessagesStatistics(false);
> > +            assertEquals("foo", (pool.appendStats("foo")));
> > +        }
> > +    }
> > +
> >       /*
> >        * 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
> > @@ -1185,6 +1201,7 @@ public class TestGenericObjectPool extends
> TestBaseObjectPool {
> >           }
> >       }
> >
> > +
> >       /**
> >        * POOL-231 - verify that concurrent invalidates of the same
> object do not
> >        * corrupt pool destroyCount.
> > @@ -1238,7 +1255,6 @@ public class TestGenericObjectPool extends
> TestBaseObjectPool {
> >           assertEquals(nIterations,
> genericObjectPool.getDestroyedCount());
> >       }
> >
> > -
> >       @Test
> >       public void testConstructorNullFactory() {
> >           // add dummy assert (won't be invoked because of IAE) to avoid
> "unused" warning
> > @@ -1916,6 +1932,14 @@ public class TestGenericObjectPool extends
> TestBaseObjectPool {
> >           }
> >       }
> >
> > +    @Test
> > +    public void testGetStatsString() {
> > +        try (final GenericObjectPool<String> pool = new
> GenericObjectPool<>(
> > +                new
> TestSynchronizedPooledObjectFactory<>(createNullPooledObjectFactory()))) {
> > +            assertNotNull(pool.getStatsString());
> > +        }
> > +    }
> > +
> >       /**
> >        * Verify that threads waiting on a depleted pool get served when
> a checked out object is
> >        * invalidated.
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>