You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@plc4x.apache.org by sr...@apache.org on 2018/10/26 16:35:21 UTC
[incubator-plc4x] branch master updated: [plc4j-pool] added test
for pool removal method. + Added TODO for maybe necessary connection
eviction.
This is an automated email from the ASF dual-hosted git repository.
sruehl pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-plc4x.git
The following commit(s) were added to refs/heads/master by this push:
new 1b9ca25 [plc4j-pool] added test for pool removal method. + Added TODO for maybe necessary connection eviction.
1b9ca25 is described below
commit 1b9ca25d942ce79702d027cbb946c5b084cf0c98
Author: Sebastian Rühl <sr...@apache.org>
AuthorDate: Fri Oct 26 18:32:57 2018 +0200
[plc4j-pool] added test for pool removal method.
+ Added TODO for maybe necessary connection eviction.
---
.../connectionpool/PooledPlcDriverManager.java | 5 ++
.../connectionpool/PooledPlcDriverManagerTest.java | 64 +++++++++++++++++++++-
2 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/plc4j/utils/connection-pool/src/main/java/org/apache/plc4x/java/utils/connectionpool/PooledPlcDriverManager.java b/plc4j/utils/connection-pool/src/main/java/org/apache/plc4x/java/utils/connectionpool/PooledPlcDriverManager.java
index a266ddb..f46910d 100644
--- a/plc4j/utils/connection-pool/src/main/java/org/apache/plc4x/java/utils/connectionpool/PooledPlcDriverManager.java
+++ b/plc4j/utils/connection-pool/src/main/java/org/apache/plc4x/java/utils/connectionpool/PooledPlcDriverManager.java
@@ -96,6 +96,7 @@ public class PooledPlcDriverManager extends PlcDriverManager {
pool.returnObject(plcConnection);
return null;
} else {
+ // TODO: add exception handler which catches exceptions like plcConnectionException and then invalidates them
return method.invoke(plcConnection, args);
}
});
@@ -144,15 +145,18 @@ public class PooledPlcDriverManager extends PlcDriverManager {
Lock lock = readWriteLock.writeLock();
lock.lock();
try {
+ LOGGER.debug("doing cleanup now on {}", poolMap);
Set<PoolKey> itemsToBeremoved = new LinkedHashSet<>();
poolMap.forEach((key, value) -> {
// TODO: check if this pool has been used in the last time and if not remove it.
// TODO: evicting empty pools for now
+ LOGGER.debug("pool {}:{} has numActive: {} numIdle: {}", key, value, value.getNumActive(), value.getNumIdle());
if (value.getNumActive() == 0 && value.getNumIdle() == 0) {
LOGGER.info("Removing unused pool {}", value);
itemsToBeremoved.add(key);
}
});
+ LOGGER.debug("items to be removed {} on {}", itemsToBeremoved, poolMap);
itemsToBeremoved.forEach(poolMap::remove);
} finally {
lock.unlock();
@@ -165,6 +169,7 @@ public class PooledPlcDriverManager extends PlcDriverManager {
try {
lock.lock();
HashMap<String, Number> statistics = new HashMap<>();
+ statistics.put("pools.count", poolMap.size());
for (Map.Entry<PoolKey, ObjectPool<PlcConnection>> poolEntry : poolMap.entrySet()) {
PoolKey pair = poolEntry.getKey();
ObjectPool<PlcConnection> objectPool = poolEntry.getValue();
diff --git a/plc4j/utils/connection-pool/src/test/java/org/apache/plc4x/java/utils/connectionpool/PooledPlcDriverManagerTest.java b/plc4j/utils/connection-pool/src/test/java/org/apache/plc4x/java/utils/connectionpool/PooledPlcDriverManagerTest.java
index 724daf1..1e0bb6b 100644
--- a/plc4j/utils/connection-pool/src/test/java/org/apache/plc4x/java/utils/connectionpool/PooledPlcDriverManagerTest.java
+++ b/plc4j/utils/connection-pool/src/test/java/org/apache/plc4x/java/utils/connectionpool/PooledPlcDriverManagerTest.java
@@ -34,6 +34,7 @@ import org.apache.plc4x.java.spi.PlcDriver;
import org.assertj.core.api.WithAssertions;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Answers;
@@ -46,6 +47,7 @@ import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.concurrent.*;
+import java.util.concurrent.atomic.AtomicReference;
import java.util.stream.IntStream;
import static org.mockito.ArgumentMatchers.any;
@@ -75,7 +77,7 @@ class PooledPlcDriverManagerTest implements WithAssertions {
driverMap.put("dummydummy", plcDriver);
executorService = Executors.newFixedThreadPool(100);
- assertThat(SUT.getStatistics()).isEmpty();
+ assertThat(SUT.getStatistics()).containsOnly(entry("pools.count", 0));
}
@AfterEach
@@ -220,6 +222,66 @@ class PooledPlcDriverManagerTest implements WithAssertions {
assertThatThrownBy(connection::unsubscriptionRequestBuilder).isInstanceOf(IllegalStateException.class).hasMessage("Proxy not valid anymore");
}
+ @Nested
+ class PoolCleanup {
+ @Test
+ void poolRemovedUnusedPoolsNormal() throws Exception {
+ // special config needed
+ SUT = new PooledPlcDriverManager(pooledPlcConnectionFactory -> {
+ GenericObjectPoolConfig<PlcConnection> plcConnectionGenericObjectPoolConfig = new GenericObjectPoolConfig<>();
+ plcConnectionGenericObjectPoolConfig.setMinIdle(1);
+ return new GenericObjectPool<>(pooledPlcConnectionFactory, plcConnectionGenericObjectPoolConfig);
+ });
+ setUp();
+
+ assertThat(SUT.getStatistics()).containsEntry("pools.count", 0);
+ SUT.removedUnusedPools();
+ assertThat(SUT.getStatistics()).containsEntry("pools.count", 0);
+ PlcConnection connection = SUT.getConnection("dummydummy:single/socket1/socket2?fancyOption=true");
+ connection.close();
+ assertThat(SUT.getStatistics()).containsEntry("pools.count", 1);
+ SUT.removedUnusedPools();
+ assertThat(SUT.getStatistics()).containsEntry("pools.count", 1);
+ // Usually the removedUnusedPools should do nothing at this place.
+ }
+
+ @Test
+ void poolRemovedUnusedPoolsNoIdles() throws Exception {
+ // special config needed
+ AtomicReference<GenericObjectPool<PlcConnection>> atomicReference = new AtomicReference<>();
+ SUT = new PooledPlcDriverManager(pooledPlcConnectionFactory -> {
+ GenericObjectPoolConfig<PlcConnection> plcConnectionGenericObjectPoolConfig = new GenericObjectPoolConfig<>();
+ plcConnectionGenericObjectPoolConfig.setMinIdle(0);
+ GenericObjectPool<PlcConnection> plcConnectionGenericObjectPool = new GenericObjectPool<>(pooledPlcConnectionFactory, plcConnectionGenericObjectPoolConfig);
+ atomicReference.set(plcConnectionGenericObjectPool);
+ // Evict under all circumstances
+ plcConnectionGenericObjectPool.setEvictionPolicy((config, underTest, idleCount) -> true);
+ return plcConnectionGenericObjectPool;
+ });
+ setUp();
+
+ // Pool should be empty at the beginning
+ assertThat(SUT.getStatistics()).containsEntry("pools.count", 0);
+ SUT.removedUnusedPools();
+ assertThat(SUT.getStatistics()).containsEntry("pools.count", 0);
+ PlcConnection connection = SUT.getConnection("dummydummy:single/socket1/socket2?fancyOption=true");
+ connection.close();
+
+ // after a connection we should have one pool
+ GenericObjectPool<PlcConnection> pool = atomicReference.get();
+ assertThat(SUT.getStatistics()).containsEntry("pools.count", 1);
+ SUT.removedUnusedPools();
+ assertThat(SUT.getStatistics()).containsEntry("pools.count", 1);
+
+ // now we force a eviction with your dummy policy
+ pool.evict();
+ SUT.removedUnusedPools();
+ assertThat(SUT.getStatistics()).containsEntry("pools.count", 0);
+ // and have no more open pools
+ }
+ }
+
+
class DummyPlcConnection implements PlcConnection, PlcConnectionMetadata {
private final String url;