From 2056d6c65455b5482f55b717c7d8964b43d80521 Mon Sep 17 00:00:00 2001 From: Pratyay Date: Fri, 22 Aug 2025 00:57:20 +0530 Subject: [PATCH 1/9] made statistic collection optional --- .../pool3/impl/BaseGenericObjectPool.java | 62 ++++-- .../pool3/impl/BaseObjectPoolConfig.java | 42 ++++ .../pool3/impl/TestBaseGenericObjectPool.java | 204 ++++++++++++++++++ 3 files changed, 293 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/apache/commons/pool3/impl/BaseGenericObjectPool.java b/src/main/java/org/apache/commons/pool3/impl/BaseGenericObjectPool.java index fc3bfbbf5..d7ca63b46 100644 --- a/src/main/java/org/apache/commons/pool3/impl/BaseGenericObjectPool.java +++ b/src/main/java/org/apache/commons/pool3/impl/BaseGenericObjectPool.java @@ -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; @@ -835,6 +836,19 @@ public boolean getMessageStatistics() { return messageStatistics; } + /** + * Gets whether detailed timing statistics collection is enabled. + * When {@code false}, the pool will not collect detailed timing statistics + * such as 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 + */ + 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 - @@ -1270,6 +1284,7 @@ protected void setConfig(final BaseObjectPoolConfig config) { setEvictionPolicy(policy); } setEvictorShutdownTimeout(config.getEvictorShutdownTimeoutDuration()); + setCollectDetailedStatistics(config.getCollectDetailedStatistics()); } /** @@ -1455,6 +1470,18 @@ 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. + * This setting does not affect basic counters like borrowedCount, createdCount, etc. + * + * @param collectDetailedStatistics whether to collect detailed statistics + */ + 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 - @@ -1738,20 +1765,21 @@ protected void toStringAppendFields(final StringBuilder builder) { */ final void updateStatsBorrow(final PooledObject 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)); + } } /** @@ -1762,7 +1790,11 @@ final void updateStatsBorrow(final PooledObject 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); + } } } diff --git a/src/main/java/org/apache/commons/pool3/impl/BaseObjectPoolConfig.java b/src/main/java/org/apache/commons/pool3/impl/BaseObjectPoolConfig.java index 5abbbece7..d30d7573e 100644 --- a/src/main/java/org/apache/commons/pool3/impl/BaseObjectPoolConfig.java +++ b/src/main/java/org/apache/commons/pool3/impl/BaseObjectPoolConfig.java @@ -166,6 +166,14 @@ public abstract class BaseObjectPoolConfig 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. + */ + public static final boolean DEFAULT_COLLECT_DETAILED_STATISTICS = true; + private boolean lifo = DEFAULT_LIFO; private boolean fairness = DEFAULT_FAIRNESS; @@ -203,6 +211,8 @@ public abstract class BaseObjectPoolConfig extends BaseObject implements Clon private String jmxNameBase = DEFAULT_JMX_NAME_BASE; + private boolean collectDetailedStatistics = DEFAULT_COLLECT_DETAILED_STATISTICS; + /** * Constructs a new instance. */ @@ -224,6 +234,20 @@ public boolean getBlockWhenExhausted() { return blockWhenExhausted; } + /** + * Gets the value for the {@code collectDetailedStatistics} configuration attribute + * for pools created with this configuration instance. + * + * @return {@code true} if detailed statistics collection is enabled, + * {@code false} if disabled for improved performance + * + * @see GenericObjectPool#getCollectDetailedStatistics() + * @see GenericKeyedObjectPool#getCollectDetailedStatistics() + */ + public boolean getCollectDetailedStatistics() { + return collectDetailedStatistics; + } + /** * Gets the value for the {@code timeBetweenEvictionRuns} configuration * attribute for pools created with this configuration instance. @@ -478,6 +502,22 @@ 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. + * + * @param collectDetailedStatistics The new setting of {@code collectDetailedStatistics} + * for this configuration instance + * + * @see GenericObjectPool#getCollectDetailedStatistics() + * @see GenericKeyedObjectPool#getCollectDetailedStatistics() + */ + 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. @@ -755,5 +795,7 @@ protected void toStringAppendFields(final StringBuilder builder) { builder.append(jmxNamePrefix); builder.append(", jmxNameBase="); builder.append(jmxNameBase); + builder.append(", collectDetailedStatistics="); + builder.append(collectDetailedStatistics); } } diff --git a/src/test/java/org/apache/commons/pool3/impl/TestBaseGenericObjectPool.java b/src/test/java/org/apache/commons/pool3/impl/TestBaseGenericObjectPool.java index d9fb5ec23..8e8e2687f 100644 --- a/src/test/java/org/apache/commons/pool3/impl/TestBaseGenericObjectPool.java +++ b/src/test/java/org/apache/commons/pool3/impl/TestBaseGenericObjectPool.java @@ -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; @@ -140,4 +147,201 @@ 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 config = new GenericObjectPoolConfig<>(); + config.setCollectDetailedStatistics(false); + + try (GenericObjectPool 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 pooledObject = (DefaultPooledObject) 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 pooledObject = (DefaultPooledObject) 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 pooledObject = (DefaultPooledObject) 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> 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 pooledObject = (DefaultPooledObject) 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 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 pooledObject = (DefaultPooledObject) 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 config = new GenericObjectPoolConfig<>(); + config.setCollectDetailedStatistics(false); + + try (GenericObjectPool 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"); + } + } } From 53b8b70f13cfbe50c060e2be6f08d384626fb904 Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Mon, 1 Dec 2025 08:49:04 -0500 Subject: [PATCH 2/9] Add Javadoc @since 2.13.0 --- .../apache/commons/pool3/impl/BaseGenericObjectPool.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/apache/commons/pool3/impl/BaseGenericObjectPool.java b/src/main/java/org/apache/commons/pool3/impl/BaseGenericObjectPool.java index d7ca63b46..b1022906d 100644 --- a/src/main/java/org/apache/commons/pool3/impl/BaseGenericObjectPool.java +++ b/src/main/java/org/apache/commons/pool3/impl/BaseGenericObjectPool.java @@ -843,7 +843,8 @@ public boolean getMessageStatistics() { * improving performance under high load. * * @return {@code true} if detailed statistics collection is enabled, - * {@code false} if disabled for improved performance + * {@code false} if disabled for improved performance. + * @since 2.13.0 */ public boolean getCollectDetailedStatistics() { return collectDetailedStatistics; @@ -1476,7 +1477,8 @@ public void setMessagesStatistics(final boolean messagesDetails) { * improving performance under high load at the cost of reduced monitoring capabilities. * This setting does not affect basic counters like borrowedCount, createdCount, etc. * - * @param collectDetailedStatistics whether to collect detailed statistics + * @param collectDetailedStatistics whether to collect detailed statistics. + * @since 2.13.0 */ public void setCollectDetailedStatistics(final boolean collectDetailedStatistics) { this.collectDetailedStatistics = collectDetailedStatistics; From 4d828f81990297541da069a90970298514a64d5c Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Mon, 1 Dec 2025 08:50:09 -0500 Subject: [PATCH 3/9] Reduce whitespace --- .../org/apache/commons/pool3/impl/BaseGenericObjectPool.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/main/java/org/apache/commons/pool3/impl/BaseGenericObjectPool.java b/src/main/java/org/apache/commons/pool3/impl/BaseGenericObjectPool.java index b1022906d..06ca1e610 100644 --- a/src/main/java/org/apache/commons/pool3/impl/BaseGenericObjectPool.java +++ b/src/main/java/org/apache/commons/pool3/impl/BaseGenericObjectPool.java @@ -1767,12 +1767,10 @@ protected void toStringAppendFields(final StringBuilder builder) { */ final void updateStatsBorrow(final PooledObject p, final Duration waitDuration) { borrowedCount.incrementAndGet(); - // Only collect detailed statistics if enabled if (collectDetailedStatistics) { idleTimes.add(p.getIdleDuration()); waitTimes.add(waitDuration); - // lock-free optimistic-locking maximum Duration currentMaxDuration; do { @@ -1792,7 +1790,6 @@ final void updateStatsBorrow(final PooledObject p, final Duration waitDuratio */ final void updateStatsReturn(final Duration activeTime) { returnedCount.incrementAndGet(); - // Only collect detailed statistics if enabled if (collectDetailedStatistics) { activeTimes.add(activeTime); From 5dad1a860d6fcb44e5adf350f2a3f9214ac54a11 Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Mon, 1 Dec 2025 08:52:14 -0500 Subject: [PATCH 4/9] Add Javadoc @since 2.13.0 --- .../apache/commons/pool3/impl/BaseObjectPoolConfig.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/apache/commons/pool3/impl/BaseObjectPoolConfig.java b/src/main/java/org/apache/commons/pool3/impl/BaseObjectPoolConfig.java index d30d7573e..88d8ceead 100644 --- a/src/main/java/org/apache/commons/pool3/impl/BaseObjectPoolConfig.java +++ b/src/main/java/org/apache/commons/pool3/impl/BaseObjectPoolConfig.java @@ -171,6 +171,8 @@ public abstract class BaseObjectPoolConfig extends BaseObject implements Clon * 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. + * + * @since 2.13.0 */ public static final boolean DEFAULT_COLLECT_DETAILED_STATISTICS = true; @@ -239,10 +241,11 @@ public boolean getBlockWhenExhausted() { * for pools created with this configuration instance. * * @return {@code true} if detailed statistics collection is enabled, - * {@code false} if disabled for improved performance + * {@code false} if disabled for improved performance. * * @see GenericObjectPool#getCollectDetailedStatistics() * @see GenericKeyedObjectPool#getCollectDetailedStatistics() + * @since 2.13.0 */ public boolean getCollectDetailedStatistics() { return collectDetailedStatistics; @@ -509,10 +512,11 @@ public void setBlockWhenExhausted(final boolean blockWhenExhausted) { * at the cost of reduced monitoring capabilities. * * @param collectDetailedStatistics The new setting of {@code collectDetailedStatistics} - * for this configuration instance + * for this configuration instance. * * @see GenericObjectPool#getCollectDetailedStatistics() * @see GenericKeyedObjectPool#getCollectDetailedStatistics() + * @since 2.13.0 */ public void setCollectDetailedStatistics(final boolean collectDetailedStatistics) { this.collectDetailedStatistics = collectDetailedStatistics; From 6a6fbb2298d7973e52ac71e366d46c0b142cbaf5 Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Mon, 1 Dec 2025 09:45:51 -0500 Subject: [PATCH 5/9] Fix Checkstyle trailing whitespace --- .../pool3/impl/TestBaseGenericObjectPool.java | 27 ------------------- 1 file changed, 27 deletions(-) diff --git a/src/test/java/org/apache/commons/pool3/impl/TestBaseGenericObjectPool.java b/src/test/java/org/apache/commons/pool3/impl/TestBaseGenericObjectPool.java index 8e8e2687f..2f324a9e3 100644 --- a/src/test/java/org/apache/commons/pool3/impl/TestBaseGenericObjectPool.java +++ b/src/test/java/org/apache/commons/pool3/impl/TestBaseGenericObjectPool.java @@ -159,15 +159,12 @@ void testCollectDetailedStatisticsConfiguration() { // Test configuration through config object final GenericObjectPoolConfig config = new GenericObjectPoolConfig<>(); config.setCollectDetailedStatistics(false); - try (GenericObjectPool testPool = new GenericObjectPool<>(factory, config)) { assertFalse(testPool.getCollectDetailedStatistics()); } - // Test runtime configuration pool.setCollectDetailedStatistics(false); assertFalse(pool.getCollectDetailedStatistics()); - pool.setCollectDetailedStatistics(true); assertTrue(pool.getCollectDetailedStatistics()); } @@ -176,23 +173,18 @@ void testCollectDetailedStatisticsConfiguration() { void testCollectDetailedStatisticsDisabled() throws Exception { // Configure pool to disable detailed statistics pool.setCollectDetailedStatistics(false); - final DefaultPooledObject pooledObject = (DefaultPooledObject) 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()); @@ -224,24 +216,19 @@ void testCollectDetailedStatisticsEnabled() throws Exception { @Test void testCollectDetailedStatisticsToggling() throws Exception { final DefaultPooledObject pooledObject = (DefaultPooledObject) 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()); @@ -255,19 +242,15 @@ void testStatsStoreConcurrentAccess() throws Exception { final ExecutorService executor = Executors.newFixedThreadPool(numThreads); final CountDownLatch startLatch = new CountDownLatch(1); final CountDownLatch completeLatch = new CountDownLatch(numThreads); - final List> 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 pooledObject = (DefaultPooledObject) 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)); @@ -281,22 +264,17 @@ void testStatsStoreConcurrentAccess() throws Exception { 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 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); @@ -310,19 +288,16 @@ void testStatsStoreConcurrentAccess() throws Exception { void testStatsStoreCircularBuffer() throws Exception { // Test that StatsStore properly handles circular buffer behavior final DefaultPooledObject pooledObject = (DefaultPooledObject) 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 @@ -333,11 +308,9 @@ void testDetailedStatisticsConfigIntegration() { // Test that config property is properly applied during pool construction final GenericObjectPoolConfig config = new GenericObjectPoolConfig<>(); config.setCollectDetailedStatistics(false); - try (GenericObjectPool 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"), From 72f201d52c3acca5b9c00d4c194e6f54171fb002 Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Mon, 1 Dec 2025 10:50:57 -0500 Subject: [PATCH 6/9] Clean up blank lines in testCollectDetailedStatisticsEnabled Removed unnecessary blank lines in test method. --- .../apache/commons/pool3/impl/TestBaseGenericObjectPool.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/test/java/org/apache/commons/pool3/impl/TestBaseGenericObjectPool.java b/src/test/java/org/apache/commons/pool3/impl/TestBaseGenericObjectPool.java index 2f324a9e3..f4aef1190 100644 --- a/src/test/java/org/apache/commons/pool3/impl/TestBaseGenericObjectPool.java +++ b/src/test/java/org/apache/commons/pool3/impl/TestBaseGenericObjectPool.java @@ -196,17 +196,13 @@ void testCollectDetailedStatisticsDisabled() throws Exception { void testCollectDetailedStatisticsEnabled() throws Exception { // Ensure detailed statistics are enabled (default) pool.setCollectDetailedStatistics(true); - final DefaultPooledObject pooledObject = (DefaultPooledObject) 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()); @@ -279,7 +275,6 @@ void testStatsStoreConcurrentAccess() throws Exception { assertTrue(pool.getMeanActiveTimeMillis() >= 0); assertTrue(pool.getMeanBorrowWaitTimeMillis() >= 0); assertTrue(pool.getMaxBorrowWaitTimeMillis() >= 0); - executor.shutdown(); assertTrue(executor.awaitTermination(5, TimeUnit.SECONDS)); } From 9ab905aed958e273fe3827d7a4299dfc806e8895 Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Mon, 1 Dec 2025 14:52:16 -0500 Subject: [PATCH 7/9] Fix Checkstyle trailing whitespace --- .../commons/pool3/impl/TestBaseGenericObjectPool.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/test/java/org/apache/commons/pool3/impl/TestBaseGenericObjectPool.java b/src/test/java/org/apache/commons/pool3/impl/TestBaseGenericObjectPool.java index f4aef1190..caeb2d851 100644 --- a/src/test/java/org/apache/commons/pool3/impl/TestBaseGenericObjectPool.java +++ b/src/test/java/org/apache/commons/pool3/impl/TestBaseGenericObjectPool.java @@ -298,18 +298,16 @@ void testStatsStoreCircularBuffer() throws Exception { assertTrue(pool.getMeanBorrowWaitTimeMillis() >= 50); // Should be influenced by recent higher values } - @Test + @Test void testDetailedStatisticsConfigIntegration() { // Test that config property is properly applied during pool construction final GenericObjectPoolConfig config = new GenericObjectPoolConfig<>(); config.setCollectDetailedStatistics(false); try (GenericObjectPool testPool = new GenericObjectPool<>(factory, config)) { - assertFalse(testPool.getCollectDetailedStatistics(), - "Pool should respect collectDetailedStatistics setting from 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"); + assertTrue(configString.contains("collectDetailedStatistics"), "Config toString should include collectDetailedStatistics property"); } } } From b04c022530fd456196916cde99394cd1020cd9bd Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Mon, 1 Dec 2025 15:25:53 -0500 Subject: [PATCH 8/9] Javadoc --- .../apache/commons/pool3/impl/BaseGenericObjectPool.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/apache/commons/pool3/impl/BaseGenericObjectPool.java b/src/main/java/org/apache/commons/pool3/impl/BaseGenericObjectPool.java index 06ca1e610..e128d52a1 100644 --- a/src/main/java/org/apache/commons/pool3/impl/BaseGenericObjectPool.java +++ b/src/main/java/org/apache/commons/pool3/impl/BaseGenericObjectPool.java @@ -838,8 +838,8 @@ public boolean getMessageStatistics() { /** * Gets whether detailed timing statistics collection is enabled. - * When {@code false}, the pool will not collect detailed timing statistics - * such as mean active time, mean idle time, and mean borrow wait time, + * 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, @@ -1475,7 +1475,9 @@ public void setMessagesStatistics(final boolean 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. - * This setting does not affect basic counters like borrowedCount, createdCount, etc. + *

+ * This setting affects data collection for mean active time, mean idle time, and mean borrow wait time. + *

* * @param collectDetailedStatistics whether to collect detailed statistics. * @since 2.13.0 From d0531c64eb74a309e0fa9d90bc0ca01a7e67db86 Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Mon, 1 Dec 2025 15:27:57 -0500 Subject: [PATCH 9/9] Javadoc --- .../commons/pool3/impl/BaseObjectPoolConfig.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/apache/commons/pool3/impl/BaseObjectPoolConfig.java b/src/main/java/org/apache/commons/pool3/impl/BaseObjectPoolConfig.java index 88d8ceead..e47a04510 100644 --- a/src/main/java/org/apache/commons/pool3/impl/BaseObjectPoolConfig.java +++ b/src/main/java/org/apache/commons/pool3/impl/BaseObjectPoolConfig.java @@ -171,6 +171,9 @@ public abstract class BaseObjectPoolConfig extends BaseObject implements Clon * 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. + *

+ * This setting affects data collection for mean active time, mean idle time, and mean borrow wait time. + *

* * @since 2.13.0 */ @@ -239,10 +242,12 @@ public boolean getBlockWhenExhausted() { /** * Gets the value for the {@code collectDetailedStatistics} configuration attribute * for pools created with this configuration instance. + *

+ * This setting affects data collection for mean active time, mean idle time, and mean borrow wait time. + *

* * @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 @@ -510,6 +515,9 @@ public void setBlockWhenExhausted(final boolean blockWhenExhausted) { * 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. + *

+ * This setting affects data collection for mean active time, mean idle time, and mean borrow wait time. + *

* * @param collectDetailedStatistics The new setting of {@code collectDetailedStatistics} * for this configuration instance.