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 <garydgreg...@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.
