Skip to content

Commit

Permalink
feat(cache) per object ttl for cache items. This adds TTL capibilites…
Browse files Browse the repository at this point in the history 
… to our CaffineCache and really deprecates the timed cache provider.

ref:#30670
  • Loading branch information
@wezell
wezell committed Nov 20, 2024
1 parent 64443d1 commit d6437ca
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 63 deletions.
46 changes: 16 additions & 30 deletions dotCMS/src/enterprise/java/com/dotcms/enterprise/cache/provider/CacheProviderAPIImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@

package com.dotcms.enterprise.cache.provider;

import com.dotcms.cache.DynamicTTLCache;
import com.dotcms.enterprise.LicenseUtil;
import com.dotcms.enterprise.license.LicenseLevel;
import com.dotmarketing.business.APILocator;
Expand All @@ -55,6 +56,8 @@
import com.dotmarketing.business.cache.provider.caffine.CaffineCache;
import com.dotmarketing.util.Config;
import com.dotmarketing.util.Logger;
import com.github.benmanes.caffeine.cache.Cache;
import com.github.benmanes.caffeine.cache.Caffeine;
import com.google.common.collect.Lists;
import io.vavr.control.Try;
import java.util.ArrayList;
Expand All @@ -76,8 +79,8 @@ public class CacheProviderAPIImpl implements CacheProviderAPI, CacheOSGIService

private static final String CACHE_POOL_DEFAULT_CHAIN = "cache.default.chain";


private final Map<String, List<CacheProvider>> configuredChainsPerRegion= new ConcurrentHashMap<>();
// we use a cache for providers because ConcurrentHashMap has a recusion problem in its computeIfAbsent method
private static final Cache<String, List<CacheProvider>> configuredChainsPerRegion = Caffeine.newBuilder().maximumSize(10000).build();
private final List<String> noLicenseProviders = List.of(CaffineCache.class.getCanonicalName());


Expand Down Expand Up @@ -127,7 +130,7 @@ private List<CacheProvider> getAllProviders () {

Map<String,CacheProvider> providers = new HashMap<>();

for(List<CacheProvider> providerList : configuredChainsPerRegion.values()){
for(List<CacheProvider> providerList : configuredChainsPerRegion.asMap().values()){
providerList.forEach(p->providers.put(p.getClass().getCanonicalName(),p));
}
return new ArrayList<>(providers.values());
Expand Down Expand Up @@ -160,41 +163,28 @@ private List<String> getProviderNamesPerRegion(String group){
* memory the initialized providers in order to impact as much as possible performance, keep in mind this class
* is heavily used!.
*
* It turns out that using Map.computeIfAbsent in a ConcurrentHashMap throws a recursion exception, so we need to
* do it old school
*
* @param group
* @return
*/
private List<CacheProvider> getProvidersForRegion ( String group ) {

//The case is very simple here, no license no chance to modify any region chain

List<CacheProvider> caches = configuredChainsPerRegion.get(group);
if(caches != null){
return caches;
}

synchronized (CacheProviderAPIImpl.class) {
caches = configuredChainsPerRegion.get(group);
if(caches != null){
return caches;
}
return configuredChainsPerRegion.get(group, k -> {
List<CacheProvider> providers = new ArrayList<>();
getProviderNamesPerRegion(group).forEach(c->getInstanceFor(c).ifPresent(providers::add));
configuredChainsPerRegion.put(group,List.copyOf(initProviders(providers)));
return configuredChainsPerRegion.get(group);
}


for(String providerClassName : getProviderNamesPerRegion(group)){
getInstanceFor(providerClassName).ifPresent(providers::add);
}
return List.copyOf(initProviders(providers));
});

}

List<CacheProvider> initProviders(List<CacheProvider> cacheProviders) {
cacheProviders.forEach(provider ->
cacheProviders.forEach(provider -> {
Try.run(provider::init).onFailure(
e -> Logger.error(this, "Error initializing CacheProvider [" + provider.getName() + "].", e))
);
e -> Logger.error(this, "Error initializing CacheProvider [" + provider.getName() + "].", e));
});
return cacheProviders;
}

Expand Down Expand Up @@ -272,10 +262,6 @@ public void put ( String group, String key, Object content ) {
try {
provider.put(group, key, content);
} catch ( Exception e ) {

e.printStackTrace();


//On Error we don't want to stop the execution of the other providers
Logger.error(this.getClass(), "Error adding record to CacheProvider [" + provider.getName() + "]: group [" + group + "] - key [" + key + "].", e);
}
Expand Down Expand Up @@ -378,7 +364,7 @@ public void removeAll (boolean ignoreDistributed) {
Logger.error(this.getClass(), "Error flushing CacheProvider [" + provider.getName() + "].", e);
}
}
configuredChainsPerRegion.clear();
configuredChainsPerRegion.invalidateAll();
shutdownProviders(providers);


Expand Down
44 changes: 11 additions & 33 deletions dotCMS/src/main/java/com/dotmarketing/business/cache/provider/caffine/CaffineCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,12 @@
import com.dotmarketing.util.Config;
import com.dotmarketing.util.Logger;
import com.dotmarketing.util.UtilMethods;
import com.github.benmanes.caffeine.cache.Cache;
import com.github.benmanes.caffeine.cache.Caffeine;
import java.text.DecimalFormat;
import java.text.NumberFormat;
import java.util.Set;
import java.util.TreeSet;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicBoolean;


Expand Down Expand Up @@ -43,7 +44,9 @@ public class CaffineCache extends CacheProvider {
private static final String SECONDS_POSTFIX = ".seconds";
private static final String DEFAULT_CACHE = CacheProviderAPI.DEFAULT_CACHE;
private static final long serialVersionUID = 1L;
private static final ConcurrentHashMap<String, DynamicTTLCache<String, Object>> groups = new ConcurrentHashMap<>();

// we use a cache for groups because concurrentHashMap has a recusion problem in its computeIfAbsent method
private static final Cache<String, DynamicTTLCache<String, Object>> groups = Caffeine.newBuilder().maximumSize(10000).build();
private static final AtomicBoolean isInitialized = new AtomicBoolean(false);

@Override
Expand All @@ -64,7 +67,7 @@ public boolean isDistributed() {
@Override
public void init() {
if(!isInitialized.getAndSet(true)){
groups.clear();
groups.invalidateAll();
Logger.info(this.getClass(), "===== Initializing [" + getName() + "].");
}
}
Expand Down Expand Up @@ -111,7 +114,7 @@ public void remove(String group) {
if (group == null) {
return;
}
groups.remove(group.toLowerCase());
groups.invalidate(group.toLowerCase());
}

@Override
Expand All @@ -126,7 +129,7 @@ public Set<String> getKeys(String group) {

@Override
public Set<String> getGroups() {
return groups.keySet();
return groups.asMap().keySet();
}

private long getTTLMillis(String group) {
Expand Down Expand Up @@ -206,41 +209,16 @@ public void shutdown() {
}


/**
* Tried to make this nice and functional, but unfortunately ConcurrentHashMap.computeIfAbsent throws a recursion
* error if when computing the group value. See: <a href="https://bugs.openjdk.java.net/browse/JDK-8062841">...</a> or
* <a href="https://stackoverflow.com/questions/54824656/since-java-9-hashmap-computeifabsent-throws-concurrentmodificationexception-on">...</a>
*
* @param cacheName
* @return
*/
private DynamicTTLCache<String, Object> getCache(String cacheName) {
if (cacheName == null) {
throw new DotStateException("Null cache region passed in");
}
DynamicTTLCache<String, Object> cache = groups.get(cacheName);

if (cache != null) {
return cache;
}
synchronized (groups) {
cache = groups.get(cacheName);
if (cache != null) {
return cache;
}

return groups.get(cacheName, k -> {
final int maxSize = getMaxSize(cacheName);
final long ttlSeconds = getTTLMillis(cacheName);

Logger.infoEvery(this.getClass(),
"***\t Building Cache : " + cacheName + ", size:" + maxSize + ", seconds:" + ttlSeconds
+ ",Concurrency:"
+ Config.getIntProperty("cache.concurrencylevel", 32), 60000);

groups.put(cacheName,new DynamicTTLCache<>(maxSize, ttlSeconds));
return groups.get(cacheName);

}
return new DynamicTTLCache<>(maxSize, ttlSeconds);
});



Expand Down

0 comments on commit d6437ca

Please sign in to comment.