Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,7 @@ public String toString() {

private volatile SwallowedExceptionListener swallowedExceptionListener;
private volatile boolean messageStatistics;
private volatile boolean collectDetailedStatistics = BaseObjectPoolConfig.DEFAULT_COLLECT_DETAILED_STATISTICS;

/** Additional configuration properties for abandoned object tracking. */
protected volatile AbandonedConfig abandonedConfig;
Expand Down Expand Up @@ -835,6 +836,20 @@ public boolean getMessageStatistics() {
return messageStatistics;
}

/**
* Gets whether detailed timing statistics collection is enabled.
* When {@code false}, the pool will not collect detailed timing statistics for
* mean active time, mean idle time, and mean borrow wait time,
* improving performance under high load.
*
* @return {@code true} if detailed statistics collection is enabled,
* {@code false} if disabled for improved performance.
* @since 2.13.0
*/
public boolean getCollectDetailedStatistics() {
return collectDetailedStatistics;
}

/**
* 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 -
Expand Down Expand Up @@ -1270,6 +1285,7 @@ protected void setConfig(final BaseObjectPoolConfig<T> config) {
setEvictionPolicy(policy);
}
setEvictorShutdownTimeout(config.getEvictorShutdownTimeoutDuration());
setCollectDetailedStatistics(config.getCollectDetailedStatistics());
}

/**
Expand Down Expand Up @@ -1455,6 +1471,21 @@ public void setMessagesStatistics(final boolean messagesDetails) {
this.messageStatistics = messagesDetails;
}

/**
* Sets whether detailed timing statistics collection is enabled.
* When {@code false}, the pool will not collect detailed timing statistics,
* improving performance under high load at the cost of reduced monitoring capabilities.
* <p>
* This setting affects data collection for mean active time, mean idle time, and mean borrow wait time.
* </p>
*
* @param collectDetailedStatistics whether to collect detailed statistics.
* @since 2.13.0
*/
public void setCollectDetailedStatistics(final boolean collectDetailedStatistics) {
this.collectDetailedStatistics = collectDetailedStatistics;
}

/**
* 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 -
Expand Down Expand Up @@ -1738,20 +1769,19 @@ protected void toStringAppendFields(final StringBuilder builder) {
*/
final void updateStatsBorrow(final PooledObject<T> p, final Duration waitDuration) {
borrowedCount.incrementAndGet();
idleTimes.add(p.getIdleDuration());
waitTimes.add(waitDuration);

// lock-free optimistic-locking maximum
Duration currentMaxDuration;
do {
currentMaxDuration = maxBorrowWaitDuration.get();
// if (currentMaxDuration >= waitDuration) {
// break;
// }
if (currentMaxDuration.compareTo(waitDuration) >= 0) {
break;
}
} while (!maxBorrowWaitDuration.compareAndSet(currentMaxDuration, waitDuration));
// Only collect detailed statistics if enabled
if (collectDetailedStatistics) {
idleTimes.add(p.getIdleDuration());
waitTimes.add(waitDuration);
// lock-free optimistic-locking maximum
Duration currentMaxDuration;
do {
currentMaxDuration = maxBorrowWaitDuration.get();
if (currentMaxDuration.compareTo(waitDuration) >= 0) {
break;
}
} while (!maxBorrowWaitDuration.compareAndSet(currentMaxDuration, waitDuration));
}
}

/**
Expand All @@ -1762,7 +1792,10 @@ final void updateStatsBorrow(final PooledObject<T> p, final Duration waitDuratio
*/
final void updateStatsReturn(final Duration activeTime) {
returnedCount.incrementAndGet();
activeTimes.add(activeTime);
// Only collect detailed statistics if enabled
if (collectDetailedStatistics) {
activeTimes.add(activeTime);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,19 @@ public abstract class BaseObjectPoolConfig<T> extends BaseObject implements Clon
*/
public static final String DEFAULT_EVICTION_POLICY_CLASS_NAME = DefaultEvictionPolicy.class.getName();

/**
* The default value for the {@code collectDetailedStatistics} configuration
* attribute. When {@code true}, the pool will collect detailed timing statistics
* for monitoring purposes. When {@code false}, detailed statistics collection
* is disabled, improving performance under high load.
* <p>
* This setting affects data collection for mean active time, mean idle time, and mean borrow wait time.
* </p>
*
* @since 2.13.0
*/
public static final boolean DEFAULT_COLLECT_DETAILED_STATISTICS = true;

private boolean lifo = DEFAULT_LIFO;

private boolean fairness = DEFAULT_FAIRNESS;
Expand Down Expand Up @@ -203,6 +216,8 @@ public abstract class BaseObjectPoolConfig<T> extends BaseObject implements Clon

private String jmxNameBase = DEFAULT_JMX_NAME_BASE;

private boolean collectDetailedStatistics = DEFAULT_COLLECT_DETAILED_STATISTICS;

/**
* Constructs a new instance.
*/
Expand All @@ -224,6 +239,23 @@ public boolean getBlockWhenExhausted() {
return blockWhenExhausted;
}

/**
* Gets the value for the {@code collectDetailedStatistics} configuration attribute
* for pools created with this configuration instance.
* <p>
* This setting affects data collection for mean active time, mean idle time, and mean borrow wait time.
* </p>
*
* @return {@code true} if detailed statistics collection is enabled,
* {@code false} if disabled for improved performance.
* @see GenericObjectPool#getCollectDetailedStatistics()
* @see GenericKeyedObjectPool#getCollectDetailedStatistics()
* @since 2.13.0
*/
public boolean getCollectDetailedStatistics() {
return collectDetailedStatistics;
}

/**
* Gets the value for the {@code timeBetweenEvictionRuns} configuration
* attribute for pools created with this configuration instance.
Expand Down Expand Up @@ -478,6 +510,26 @@ public void setBlockWhenExhausted(final boolean blockWhenExhausted) {
this.blockWhenExhausted = blockWhenExhausted;
}

/**
* Sets the value for the {@code collectDetailedStatistics} configuration attribute
* for pools created with this configuration instance. When {@code false}, the pool
* will not collect detailed timing statistics, improving performance under high load
* at the cost of reduced monitoring capabilities.
* <p>
* This setting affects data collection for mean active time, mean idle time, and mean borrow wait time.
* </p>
*
* @param collectDetailedStatistics The new setting of {@code collectDetailedStatistics}
* for this configuration instance.
*
* @see GenericObjectPool#getCollectDetailedStatistics()
* @see GenericKeyedObjectPool#getCollectDetailedStatistics()
* @since 2.13.0
*/
public void setCollectDetailedStatistics(final boolean collectDetailedStatistics) {
this.collectDetailedStatistics = collectDetailedStatistics;
}

/**
* Sets the value for the {@code timeBetweenEvictionRuns} configuration
* attribute for pools created with this configuration instance.
Expand Down Expand Up @@ -755,5 +807,7 @@ protected void toStringAppendFields(final StringBuilder builder) {
builder.append(jmxNamePrefix);
builder.append(", jmxNameBase=");
builder.append(jmxNameBase);
builder.append(", collectDetailedStatistics=");
builder.append(collectDetailedStatistics);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,17 @@
package org.apache.commons.pool3.impl;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.lang.management.ManagementFactory;
import java.time.Duration;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;

Expand Down Expand Up @@ -140,4 +147,167 @@ void testJMXRegistrationLatency() {
pools.forEach(GenericObjectPool::close);
}
}

@Test
void testCollectDetailedStatisticsDefault() {
// Test that collectDetailedStatistics defaults to true for backward compatibility
assertTrue(pool.getCollectDetailedStatistics());
}

@Test
void testCollectDetailedStatisticsConfiguration() {
// Test configuration through config object
final GenericObjectPoolConfig<String> config = new GenericObjectPoolConfig<>();
config.setCollectDetailedStatistics(false);
try (GenericObjectPool<String, TestException> testPool = new GenericObjectPool<>(factory, config)) {
assertFalse(testPool.getCollectDetailedStatistics());
}
// Test runtime configuration
pool.setCollectDetailedStatistics(false);
assertFalse(pool.getCollectDetailedStatistics());
pool.setCollectDetailedStatistics(true);
assertTrue(pool.getCollectDetailedStatistics());
}

@Test
void testCollectDetailedStatisticsDisabled() throws Exception {
// Configure pool to disable detailed statistics
pool.setCollectDetailedStatistics(false);
final DefaultPooledObject<String> pooledObject = (DefaultPooledObject<String>) factory.makeObject();
// Record initial values
final long initialActiveTime = pool.getMeanActiveTimeMillis();
final long initialIdleTime = pool.getMeanIdleDuration().toMillis();
final long initialWaitTime = pool.getMeanBorrowWaitTimeMillis();
final long initialMaxWaitTime = pool.getMaxBorrowWaitTimeMillis();
// Update statistics - should be ignored for detailed stats
pool.updateStatsBorrow(pooledObject, Duration.ofMillis(100));
pool.updateStatsReturn(Duration.ofMillis(200));
// Basic counters should still work
assertEquals(1, pool.getBorrowedCount());
assertEquals(1, pool.getReturnedCount());
// Detailed statistics should remain unchanged
assertEquals(initialActiveTime, pool.getMeanActiveTimeMillis());
assertEquals(initialIdleTime, pool.getMeanIdleDuration().toMillis());
assertEquals(initialWaitTime, pool.getMeanBorrowWaitTimeMillis());
assertEquals(initialMaxWaitTime, pool.getMaxBorrowWaitTimeMillis());
}

@Test
void testCollectDetailedStatisticsEnabled() throws Exception {
// Ensure detailed statistics are enabled (default)
pool.setCollectDetailedStatistics(true);
final DefaultPooledObject<String> pooledObject = (DefaultPooledObject<String>) factory.makeObject();
// Update statistics
pool.updateStatsBorrow(pooledObject, Duration.ofMillis(100));
pool.updateStatsReturn(Duration.ofMillis(200));
// All counters should work
assertEquals(1, pool.getBorrowedCount());
assertEquals(1, pool.getReturnedCount());
// Detailed statistics should be updated
assertEquals(200, pool.getMeanActiveTimeMillis());
assertEquals(100, pool.getMeanBorrowWaitTimeMillis());
assertEquals(100, pool.getMaxBorrowWaitTimeMillis());
}

@Test
void testCollectDetailedStatisticsToggling() throws Exception {
final DefaultPooledObject<String> pooledObject = (DefaultPooledObject<String>) factory.makeObject();
// Start with detailed stats enabled
pool.setCollectDetailedStatistics(true);
pool.updateStatsBorrow(pooledObject, Duration.ofMillis(50));
pool.updateStatsReturn(Duration.ofMillis(100));
assertEquals(50, pool.getMeanBorrowWaitTimeMillis());
assertEquals(100, pool.getMeanActiveTimeMillis());
// Disable detailed stats
pool.setCollectDetailedStatistics(false);
pool.updateStatsBorrow(pooledObject, Duration.ofMillis(200));
pool.updateStatsReturn(Duration.ofMillis(300));
// Detailed stats should remain at previous values
assertEquals(50, pool.getMeanBorrowWaitTimeMillis());
assertEquals(100, pool.getMeanActiveTimeMillis());
// Basic counters should continue to increment
assertEquals(2, pool.getBorrowedCount());
assertEquals(2, pool.getReturnedCount());
}

@Test
void testStatsStoreConcurrentAccess() throws Exception {
// Test the lock-free StatsStore implementation under concurrent load
final int numThreads = 10;
final int operationsPerThread = 1000;
final ExecutorService executor = Executors.newFixedThreadPool(numThreads);
final CountDownLatch startLatch = new CountDownLatch(1);
final CountDownLatch completeLatch = new CountDownLatch(numThreads);
final List<Future<Void>> futures = new ArrayList<>();
// Create threads that will concurrently update statistics
for (int i = 0; i < numThreads; i++) {
final int threadId = i;
futures.add(executor.submit(() -> {
try {
final DefaultPooledObject<String> pooledObject = (DefaultPooledObject<String>) factory.makeObject();
// Wait for all threads to be ready
startLatch.await();
// Perform concurrent operations
for (int j = 0; j < operationsPerThread; j++) {
pool.updateStatsBorrow(pooledObject, Duration.ofMillis(threadId * 10 + j));
pool.updateStatsReturn(Duration.ofMillis(threadId * 20 + j));
}
} catch (Exception e) {
throw new RuntimeException(e);
} finally {
completeLatch.countDown();
}
return null;
}));
}
// Start all threads simultaneously
startLatch.countDown();
// Wait for completion
assertTrue(completeLatch.await(30, TimeUnit.SECONDS), "Concurrent test should complete within 30 seconds");
// Verify no exceptions occurred
for (Future<Void> future : futures) {
future.get(); // Will throw if there was an exception
}
// Verify that statistics were collected (exact values may vary due to race conditions)
assertEquals(numThreads * operationsPerThread, pool.getBorrowedCount());
assertEquals(numThreads * operationsPerThread, pool.getReturnedCount());
// Mean values should be reasonable (not zero or wildly incorrect)
assertTrue(pool.getMeanActiveTimeMillis() >= 0);
assertTrue(pool.getMeanBorrowWaitTimeMillis() >= 0);
assertTrue(pool.getMaxBorrowWaitTimeMillis() >= 0);
executor.shutdown();
assertTrue(executor.awaitTermination(5, TimeUnit.SECONDS));
}

@Test
void testStatsStoreCircularBuffer() throws Exception {
// Test that StatsStore properly handles circular buffer behavior
final DefaultPooledObject<String> pooledObject = (DefaultPooledObject<String>) factory.makeObject();
// Fill beyond the cache size (100) to test circular behavior
final int cacheSize = 100; // BaseGenericObjectPool.MEAN_TIMING_STATS_CACHE_SIZE
for (int i = 0; i < cacheSize + 50; i++) {
pool.updateStatsBorrow(pooledObject, Duration.ofMillis(i));
pool.updateStatsReturn(Duration.ofMillis(i * 2));
}
// Statistics should still be meaningful after circular buffer wrapping
assertTrue(pool.getMeanActiveTimeMillis() > 0);
assertTrue(pool.getMeanBorrowWaitTimeMillis() > 0);
assertTrue(pool.getMaxBorrowWaitTimeMillis() > 0);
// The mean should reflect recent values, not all historical values
// (exact assertion depends on circular buffer implementation)
assertTrue(pool.getMeanBorrowWaitTimeMillis() >= 50); // Should be influenced by recent higher values
}

@Test
void testDetailedStatisticsConfigIntegration() {
// Test that config property is properly applied during pool construction
final GenericObjectPoolConfig<String> config = new GenericObjectPoolConfig<>();
config.setCollectDetailedStatistics(false);
try (GenericObjectPool<String, TestException> testPool = new GenericObjectPool<>(factory, config)) {
assertFalse(testPool.getCollectDetailedStatistics(), "Pool should respect collectDetailedStatistics setting from config");
// Test that toString includes the new property
final String configString = config.toString();
assertTrue(configString.contains("collectDetailedStatistics"), "Config toString should include collectDetailedStatistics property");
}
}
}