From cfac7be40065c7ee3094c7ab5195dc15be3a921d Mon Sep 17 00:00:00 2001 From: codeforkjeff Date: Wed, 12 Oct 2016 23:22:02 -0400 Subject: [PATCH] prevent ConcurrentModificationExceptions under high load in cache expire thread, by using ConcurrentHashMap in Cache --- src/main/java/com/codefork/refine/Cache.java | 8 ++++---- src/main/java/com/codefork/refine/CacheExpire.java | 8 +++++++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/codefork/refine/Cache.java b/src/main/java/com/codefork/refine/Cache.java index 2e44334..a9e639b 100644 --- a/src/main/java/com/codefork/refine/Cache.java +++ b/src/main/java/com/codefork/refine/Cache.java @@ -5,7 +5,7 @@ import java.util.Arrays; import java.util.Comparator; -import java.util.HashMap; +import java.util.concurrent.ConcurrentHashMap; /** * Simple cache implementation. This does NOT do any locking. Clients should @@ -22,7 +22,7 @@ public class Cache { private int lifetime = DEFAULT_LIFETIME; // in seconds private int maxSize = DEFAULT_MAXSIZE; - private HashMap cacheMap = new HashMap(); + private ConcurrentHashMap cacheMap = new ConcurrentHashMap(); public int getLifetime() { return lifetime; @@ -82,7 +82,7 @@ public Cache expireCache() { // processing keys in descending timestamp order makes // it easier to break out of the copy loop when we hit maxSize - final HashMap oldMap = getMap(); + final ConcurrentHashMap oldMap = getMap(); K[] sorted = (K[]) oldMap.keySet().toArray(); Arrays.sort(sorted, new ReverseTimestampComparator()); int total = oldMap.size(); @@ -110,7 +110,7 @@ public Cache expireCache() { return newCache; } - private HashMap getMap() { + private ConcurrentHashMap getMap() { return cacheMap; } diff --git a/src/main/java/com/codefork/refine/CacheExpire.java b/src/main/java/com/codefork/refine/CacheExpire.java index 736799f..9051f81 100644 --- a/src/main/java/com/codefork/refine/CacheExpire.java +++ b/src/main/java/com/codefork/refine/CacheExpire.java @@ -19,7 +19,13 @@ public void run() { try { while(keepGoing) { Thread.sleep(60000); - cacheManager.expireCache(); + try { + cacheManager.expireCache(); + } catch(Exception e) { + // if we don't catch here, a possibly intermittent or edge-case error + // causes the cache expire thread to die, and the cache will grow uncontrollably + log.error("Ignoring error that occurred in cacheManager.expireCache(): " + e.toString()); + } } } catch (InterruptedException e) { // noop