GERONIMO-6623 GERONIMO-6622 fixing pool configuration and simplifying MethodMeta
diff --git a/src/main/java/org/apache/geronimo/jcache/simple/SimpleCache.java b/src/main/java/org/apache/geronimo/jcache/simple/SimpleCache.java
index 9788a16..feb52d7 100644
--- a/src/main/java/org/apache/geronimo/jcache/simple/SimpleCache.java
+++ b/src/main/java/org/apache/geronimo/jcache/simple/SimpleCache.java
@@ -21,6 +21,7 @@
import static org.apache.geronimo.jcache.simple.Asserts.assertNotNull;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
@@ -32,8 +33,11 @@
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.CopyOnWriteArraySet;
+import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.atomic.AtomicReference;
import javax.cache.Cache;
import javax.cache.CacheException;
@@ -84,10 +88,13 @@
private final Serializations serializations;
+ private final Collection<Future<?>> poolTasks = new CopyOnWriteArraySet<>();
+
private volatile boolean closed = false;
public SimpleCache(final ClassLoader classLoader, final SimpleManager mgr, final String cacheName,
- final SimpleConfiguration<K, V> configuration, final Properties properties) {
+ final SimpleConfiguration<K, V> configuration, final Properties properties,
+ final ExecutorService executorService) {
manager = mgr;
name = cacheName;
@@ -97,21 +104,13 @@
final int concurrencyLevel = Integer.parseInt(property(properties, cacheName, "concurrencyLevel", "16"));
delegate = new ConcurrentHashMap<>(capacity, loadFactor, concurrencyLevel);
config = configuration;
-
- ExecutorService executorService = rawProperty(properties, cacheName, "pool"); // lookup etc support
- if (executorService == null) {
- final int poolSize = Integer.parseInt(property(properties, cacheName, "pool.size", "3"));
- final SimpleThreadFactory threadFactory = new SimpleThreadFactory("geronimo-simple-jcache-" + cacheName + "-");
- executorService = poolSize > 0 ? Executors.newFixedThreadPool(poolSize, threadFactory)
- : Executors.newCachedThreadPool(threadFactory);
- }
pool = executorService;
final long evictionPause = Long.parseLong(
properties.getProperty(cacheName + ".evictionPause", properties.getProperty("evictionPause", "30000")));
if (evictionPause > 0) {
final long maxDeleteByEvictionRun = Long.parseLong(property(properties, cacheName, "maxDeleteByEvictionRun", "100"));
- pool.submit(new EvictionThread(evictionPause, maxDeleteByEvictionRun));
+ addPoolTask(new EvictionThread(evictionPause, maxDeleteByEvictionRun));
}
serializations = new Serializations(property(properties, cacheName, "serialization.whitelist", null));
@@ -522,7 +521,7 @@
for (final K k : keys) {
assertNotNull(k, "a key");
}
- pool.submit(new Runnable() {
+ addPoolTask(new Runnable() {
@Override
public void run() {
@@ -531,6 +530,28 @@
});
}
+ private void addPoolTask(final Runnable runnable) {
+ final AtomicReference<Future<?>> ref = new AtomicReference<>();
+ final CountDownLatch refIsSet = new CountDownLatch(1);
+ ref.set(pool.submit(new Runnable() {
+ @Override
+ public void run() {
+ try {
+ runnable.run();
+ } finally {
+ try {
+ refIsSet.await();
+ } catch (final InterruptedException e) {
+ Thread.currentThread().interrupt();
+ }
+ poolTasks.remove(ref.get());
+ }
+ }
+ }));
+ refIsSet.countDown();
+ poolTasks.add(ref.get());
+ }
+
private void doLoadAll(final Set<? extends K> keys, final boolean replaceExistingValues,
final CompletionListener completionListener) {
try {
@@ -681,31 +702,30 @@
return;
}
- for (final Runnable task : pool.shutdownNow()) {
- task.run();
+ for (final Future<?> task : poolTasks) {
+ task.cancel(true);
}
- // todo: better error handling (try/catch/log/suppressed)
+ final CacheException ce = new CacheException();
manager.release(getName());
closed = true;
- try {
- close(loader);
- close(writer);
- close(expiryPolicy);
- } catch (final Exception e) {
- throw new CacheException(e);
- }
+ close(loader, ce);
+ close(writer, ce);
+ close(expiryPolicy, ce);
for (final SimpleListener<K, V> listener : listeners.values()) {
try {
listener.close();
} catch (final Exception e) {
- throw new CacheException(e);
+ ce.addSuppressed(e);
}
}
listeners.clear();
JMXs.unregister(cacheConfigObjectName);
JMXs.unregister(cacheStatsObjectName);
delegate.clear();
+ if (ce.getSuppressed().length > 0) {
+ throw ce;
+ }
}
@Override
@@ -756,14 +776,6 @@
return properties.getProperty(cacheName + "." + name, properties.getProperty(name, defaultValue));
}
- private static <T> T rawProperty(final Properties properties, final String cacheName, final String name) {
- final Object value = properties.get(cacheName + "." + name);
- if (value == null) {
- return (T) properties.get(name);
- }
- return (T) value;
- }
-
private static boolean isNotZero(final Duration duration) {
return duration == null || !duration.isZero();
}
@@ -775,9 +787,13 @@
throw new EntryProcessorException(ex);
}
- private static void close(final Object potentiallyCloseable) throws Exception {
+ private static void close(final Object potentiallyCloseable, final CacheException wrapper) {
if (AutoCloseable.class.isInstance(potentiallyCloseable)) {
- AutoCloseable.class.cast(potentiallyCloseable).close();
+ try {
+ AutoCloseable.class.cast(potentiallyCloseable).close();
+ } catch (final Exception re) {
+ wrapper.addSuppressed(re);
+ }
}
}
diff --git a/src/main/java/org/apache/geronimo/jcache/simple/SimpleManager.java b/src/main/java/org/apache/geronimo/jcache/simple/SimpleManager.java
index 3b05898..13a94ed 100644
--- a/src/main/java/org/apache/geronimo/jcache/simple/SimpleManager.java
+++ b/src/main/java/org/apache/geronimo/jcache/simple/SimpleManager.java
@@ -30,6 +30,8 @@
import java.util.Properties;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
import javax.cache.Cache;
import javax.cache.CacheException;
@@ -51,6 +53,8 @@
private final Properties configProperties;
+ private final ExecutorService executorService;
+
private volatile boolean closed = false;
SimpleManager(final CachingProvider provider, final URI uri, final ClassLoader loader, final Properties properties) {
@@ -59,12 +63,30 @@
this.loader = loader;
this.properties = readConfig(uri, loader, properties);
this.configProperties = properties;
+
+ ExecutorService executorService = rawProperty("geronimo.pool");
+ if (executorService == null) {
+ final int poolSize = Integer.parseInt(this.properties.getProperty("pool.size", "16"));
+ final SimpleThreadFactory threadFactory = new SimpleThreadFactory("geronimo-simple-jcache-[" + uri.toASCIIString() + "]-");
+ executorService = poolSize > 0 ? Executors.newFixedThreadPool(poolSize, threadFactory)
+ : Executors.newCachedThreadPool(threadFactory);
+ }
+ this.executorService = executorService;
+ }
+
+ private <T> T rawProperty(final String name) {
+ final Object value = this.properties.get(name);
+ if (value == null) {
+ return (T) this.properties.get(name);
+ }
+ return (T) value;
}
private Properties readConfig(final URI uri, final ClassLoader loader, final Properties properties) {
final Properties props = new Properties();
try {
- if (SimpleProvider.DEFAULT_URI.toString().equals(uri.toString()) || uri.getScheme().equals("geronimo")) {
+ if (SimpleProvider.DEFAULT_URI.toString().equals(uri.toString()) || uri.getScheme().equals("geronimo")
+ || uri.getScheme().equals("classpath")) {
final Enumeration<URL> resources = loader.getResources(uri.toASCIIString().substring((uri.getScheme() + "://").length()));
while (resources.hasMoreElements()) {
@@ -108,7 +130,7 @@
final Class<V> valueType = configuration.getValueType();
if (!caches.containsKey(cacheName)) {
final Cache<K, V> cache = ClassLoaderAwareCache.wrap(loader, new SimpleCache<K, V>(loader, this, cacheName,
- new SimpleConfiguration<>(configuration, keyType, valueType), properties));
+ new SimpleConfiguration<>(configuration, keyType, valueType), properties, executorService));
caches.putIfAbsent(cacheName, cache);
} else {
throw new CacheException("cache " + cacheName + " already exists");
@@ -170,6 +192,9 @@
c.close();
}
caches.clear();
+ for (final Runnable task : executorService.shutdownNow()) {
+ task.run();
+ }
closed = true;
if (SimpleProvider.class.isInstance(provider)) {
SimpleProvider.class.cast(provider).remove(this);
diff --git a/src/main/java/org/apache/geronimo/jcache/simple/cdi/CDIJCacheHelper.java b/src/main/java/org/apache/geronimo/jcache/simple/cdi/CDIJCacheHelper.java
index 2f2a69b..e9f0002 100644
--- a/src/main/java/org/apache/geronimo/jcache/simple/cdi/CDIJCacheHelper.java
+++ b/src/main/java/org/apache/geronimo/jcache/simple/cdi/CDIJCacheHelper.java
@@ -151,31 +151,12 @@
: cacheResolverFactoryFor(defaults, cacheRemoveAll.cacheResolverFactory());
return new MethodMeta(parameterTypes, annotations, mtdAnnotations, keyParameterIndexes(ic.getMethod()),
- getValueParameter(annotations), getKeyParameters(annotations), cacheResultCacheResultName,
+ getValueParameter(annotations), cacheResultCacheResultName,
cacheResultCacheResolverFactory, cacheResultCacheKeyGenerator, cacheResult, cachePutCachePutName,
cachePutCacheResolverFactory, cachePutCacheKeyGenerator, cachePut != null && cachePut.afterInvocation(), cachePut,
cacheRemoveCacheRemoveName, cacheRemoveCacheResolverFactory, cacheRemoveCacheKeyGenerator,
cacheRemove != null && cacheRemove.afterInvocation(), cacheRemove, cacheRemoveAllCacheRemoveAllName,
- cacheRemoveAllCacheResolverFactory, cacheRemoveAll != null && cacheRemoveAll.afterInvocation(), cacheRemoveAll);
- }
-
- private Integer[] getKeyParameters(final List<Set<Annotation>> annotations) {
- final Collection<Integer> list = new ArrayList<Integer>();
- int idx = 0;
- for (final Set<Annotation> set : annotations) {
- for (final Annotation a : set) {
- if (a.annotationType() == CacheKey.class) {
- list.add(idx);
- }
- }
- idx++;
- }
- if (list.isEmpty()) {
- for (int i = 0; i < annotations.size(); i++) {
- list.add(i);
- }
- }
- return list.toArray(new Integer[list.size()]);
+ cacheRemoveAllCacheResolverFactory, cacheRemoveAll);
}
private Integer getValueParameter(final List<Set<Annotation>> annotations) {
@@ -409,8 +390,6 @@
private final Integer valueIndex;
- private final Integer[] parameterIndices;
-
private final String cacheResultCacheName;
private final CacheResolverFactory cacheResultResolverFactory;
@@ -443,24 +422,21 @@
private final CacheResolverFactory cacheRemoveAllResolverFactory;
- private final boolean cacheRemoveAllAfter;
-
private final CacheRemoveAll cacheRemoveAll;
public MethodMeta(Class<?>[] parameterTypes, List<Set<Annotation>> parameterAnnotations, Set<Annotation> annotations,
- Integer[] keysIndices, Integer valueIndex, Integer[] parameterIndices, String cacheResultCacheName,
+ Integer[] keysIndices, Integer valueIndex, String cacheResultCacheName,
CacheResolverFactory cacheResultResolverFactory, CacheKeyGenerator cacheResultKeyGenerator,
CacheResult cacheResult, String cachePutCacheName, CacheResolverFactory cachePutResolverFactory,
CacheKeyGenerator cachePutKeyGenerator, boolean cachePutAfter, CachePut cachePut, String cacheRemoveCacheName,
CacheResolverFactory cacheRemoveResolverFactory, CacheKeyGenerator cacheRemoveKeyGenerator,
boolean cacheRemoveAfter, CacheRemove cacheRemove, String cacheRemoveAllCacheName,
- CacheResolverFactory cacheRemoveAllResolverFactory, boolean cacheRemoveAllAfter, CacheRemoveAll cacheRemoveAll) {
+ CacheResolverFactory cacheRemoveAllResolverFactory, CacheRemoveAll cacheRemoveAll) {
this.parameterTypes = parameterTypes;
this.parameterAnnotations = parameterAnnotations;
this.annotations = annotations;
this.keysIndices = keysIndices;
this.valueIndex = valueIndex;
- this.parameterIndices = parameterIndices;
this.cacheResultCacheName = cacheResultCacheName;
this.cacheResultResolverFactory = cacheResultResolverFactory;
this.cacheResultKeyGenerator = cacheResultKeyGenerator;
@@ -477,7 +453,6 @@
this.cacheRemove = cacheRemove;
this.cacheRemoveAllCacheName = cacheRemoveAllCacheName;
this.cacheRemoveAllResolverFactory = cacheRemoveAllResolverFactory;
- this.cacheRemoveAllAfter = cacheRemoveAllAfter;
this.cacheRemoveAll = cacheRemoveAll;
}
@@ -513,10 +488,6 @@
return cacheResult;
}
- public Integer[] getParameterIndices() {
- return parameterIndices;
- }
-
public Set<Annotation> getAnnotations() {
return annotations;
}
@@ -525,10 +496,6 @@
return keysIndices;
}
- public Integer getValuesIndex() {
- return valueIndex;
- }
-
public Integer getValueIndex() {
return valueIndex;
}
@@ -573,10 +540,6 @@
return cacheRemoveAllResolverFactory;
}
- public boolean isCacheRemoveAllAfter() {
- return cacheRemoveAllAfter;
- }
-
public CacheRemoveAll getCacheRemoveAll() {
return cacheRemoveAll;
}
