From 2a9df326cde007ddf6e7efb22d3b25c93d9ce4ba Mon Sep 17 00:00:00 2001 From: pkupczyk <pkupczyk> Date: Tue, 25 Aug 2015 13:50:58 +0000 Subject: [PATCH] SSDM-2294 : V3 AS API - paging and sorting of search results - make EHCache check memory consumption + extract cache key and entry to separate classes to make sure it does not follow inner class "outer this" reference during the calculation SVN: 34517 --- .../v3/cache/SearchCacheCleanupListener.java | 57 +++++++++ .../server/api/v3/cache/SearchCacheEntry.java | 41 ++++++ .../v3/cache/SearchCacheEventListener.java | 45 +++++-- .../server/api/v3/cache/SearchCacheKey.java | 70 +++++++++++ .../method/AbstractSearchMethodExecutor.java | 118 +++--------------- .../translator/AbstractCachingTranslator.java | 16 +-- .../api/v3/translator/TranslationCache.java | 20 ++- openbis/source/java/ehcache.xml | 7 +- 8 files changed, 247 insertions(+), 127 deletions(-) create mode 100644 openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/cache/SearchCacheCleanupListener.java create mode 100644 openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/cache/SearchCacheEntry.java create mode 100644 openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/cache/SearchCacheKey.java diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/cache/SearchCacheCleanupListener.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/cache/SearchCacheCleanupListener.java new file mode 100644 index 00000000000..a8809d5f495 --- /dev/null +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/cache/SearchCacheCleanupListener.java @@ -0,0 +1,57 @@ +/* + * Copyright 2015 ETH Zuerich, CISD + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package ch.ethz.sis.openbis.generic.server.api.v3.cache; + +import org.apache.log4j.Logger; +import org.springframework.cache.Cache; +import org.springframework.cache.Cache.ValueWrapper; + +import ch.systemsx.cisd.common.logging.LogCategory; +import ch.systemsx.cisd.common.logging.LogFactory; +import ch.systemsx.cisd.openbis.generic.shared.dto.Session; + +/** + * @author pkupczyk + */ +public class SearchCacheCleanupListener<CRITERION, FETCH_OPTIONS> implements Session.ISessionCleaner +{ + + private final Logger operationLog = LogFactory.getLogger(LogCategory.OPERATION, getClass()); + + private Cache cache; + + private SearchCacheKey<CRITERION, FETCH_OPTIONS> key; + + public SearchCacheCleanupListener(Cache cache, SearchCacheKey<CRITERION, FETCH_OPTIONS> key) + { + this.cache = cache; + this.key = key; + } + + @Override + public void cleanup() + { + ValueWrapper wrapper = cache.get(key); + + if (wrapper != null) + { + operationLog.info("Clean up cached search result on logout."); + cache.evict(key); + } + } + +} \ No newline at end of file diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/cache/SearchCacheEntry.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/cache/SearchCacheEntry.java new file mode 100644 index 00000000000..460012a065f --- /dev/null +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/cache/SearchCacheEntry.java @@ -0,0 +1,41 @@ +/* + * Copyright 2015 ETH Zuerich, CISD + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package ch.ethz.sis.openbis.generic.server.api.v3.cache; + +import java.io.Serializable; +import java.util.Collection; + +/** + * @author pkupczyk + */ +public class SearchCacheEntry<OBJECT> implements Serializable +{ + private static final long serialVersionUID = 1L; + + private Collection<OBJECT> objects; + + public Collection<OBJECT> getObjects() + { + return objects; + } + + public void setObjects(Collection<OBJECT> objects) + { + this.objects = objects; + } + +} diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/cache/SearchCacheEventListener.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/cache/SearchCacheEventListener.java index 4ea05afca16..c37f8699f6f 100644 --- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/cache/SearchCacheEventListener.java +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/cache/SearchCacheEventListener.java @@ -23,7 +23,6 @@ import net.sf.ehcache.event.CacheEventListener; import org.apache.log4j.Logger; -import ch.ethz.sis.openbis.generic.server.api.v3.executor.method.AbstractSearchMethodExecutor.CacheEntry; import ch.systemsx.cisd.common.logging.LogCategory; import ch.systemsx.cisd.common.logging.LogFactory; @@ -44,31 +43,31 @@ public class SearchCacheEventListener implements CacheEventListener @Override public void notifyElementEvicted(Ehcache cache, Element element) { - logOperation(element, "has been evicted from the cache"); + logOperation(cache, element, "has been evicted from the cache"); } @Override public void notifyElementExpired(Ehcache cache, Element element) { - logOperation(element, "has expired"); + logOperation(cache, element, "has expired"); } @Override public void notifyElementPut(Ehcache cache, Element element) throws CacheException { - logOperation(element, "has been put to the cache"); + logOperation(cache, element, "has been put to the cache"); } @Override public void notifyElementRemoved(Ehcache cache, Element element) throws CacheException { - logOperation(element, "has been removed from the cache"); + logOperation(cache, element, "has been removed from the cache"); } @Override public void notifyElementUpdated(Ehcache cache, Element element) throws CacheException { - logOperation(element, "has been updated"); + logOperation(cache, element, "has been updated"); } @Override @@ -78,13 +77,39 @@ public class SearchCacheEventListener implements CacheEventListener } @SuppressWarnings("rawtypes") - private void logOperation(Element element, String operation) + private void logOperation(Ehcache cache, Element element, String operation) { - CacheEntry entry = (CacheEntry) element.getObjectValue(); + SearchCacheEntry entry = (SearchCacheEntry) element.getObjectValue(); + if (entry != null) { - operationLog.info("Cache entry " + entry.hashCode() + " that contains search result with " - + (entry.getObjects() != null ? entry.getObjects().size() : 0) + " objects " + operation + "."); + if (operationLog.isInfoEnabled()) + { + StringBuilder sb = new StringBuilder(); + sb.append("Cache entry " + entry.hashCode() + " that contains search result with "); + + int objectsSize = entry.getObjects() != null ? entry.getObjects().size() : 0; + + if (objectsSize == 1) + { + sb.append("1 object " + operation + "."); + } else + { + sb.append(objectsSize + " objects " + operation + "."); + } + + int cacheSize = cache.getSize(); + + if (cacheSize == 1) + { + sb.append(" Cache now contains 1 entry."); + } else + { + sb.append(" Cache now contains " + cacheSize + " entries."); + } + + operationLog.info(sb.toString()); + } } } diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/cache/SearchCacheKey.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/cache/SearchCacheKey.java new file mode 100644 index 00000000000..dc056b8b87d --- /dev/null +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/cache/SearchCacheKey.java @@ -0,0 +1,70 @@ +/* + * Copyright 2015 ETH Zuerich, CISD + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package ch.ethz.sis.openbis.generic.server.api.v3.cache; + +import java.io.Serializable; + +import ch.ethz.sis.openbis.generic.shared.api.v3.dto.fetchoptions.FetchOptionsMatcher; + +/** + * @author pkupczyk + */ +public class SearchCacheKey<CRITERION, FETCH_OPTIONS> implements Serializable +{ + + private static final long serialVersionUID = 1L; + + private String sessionToken; + + private CRITERION criterion; + + private FETCH_OPTIONS fetchOptions; + + public SearchCacheKey(String sessionToken, CRITERION criterion, FETCH_OPTIONS fetchOptions) + { + this.sessionToken = sessionToken; + this.criterion = criterion; + this.fetchOptions = fetchOptions; + } + + @Override + public int hashCode() + { + return sessionToken.hashCode() + criterion.getClass().hashCode() + fetchOptions.getClass().hashCode(); + } + + @Override + public boolean equals(Object obj) + { + if (this == obj) + { + return true; + } + if (obj == null) + { + return false; + } + if (getClass() != obj.getClass()) + { + return false; + } + + SearchCacheKey<?, ?> other = (SearchCacheKey<?, ?>) obj; + return sessionToken.equals(other.sessionToken) && criterion.equals(other.criterion) + && FetchOptionsMatcher.arePartsEqual(fetchOptions, other.fetchOptions); + } +} \ No newline at end of file diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/method/AbstractSearchMethodExecutor.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/method/AbstractSearchMethodExecutor.java index 8960b2e870c..7b428b9d7c1 100644 --- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/method/AbstractSearchMethodExecutor.java +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/method/AbstractSearchMethodExecutor.java @@ -16,7 +16,6 @@ package ch.ethz.sis.openbis.generic.server.api.v3.executor.method; -import java.io.Serializable; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -29,19 +28,20 @@ import org.springframework.cache.Cache; import org.springframework.cache.Cache.ValueWrapper; import org.springframework.cache.CacheManager; +import ch.ethz.sis.openbis.generic.server.api.v3.cache.SearchCacheCleanupListener; +import ch.ethz.sis.openbis.generic.server.api.v3.cache.SearchCacheEntry; +import ch.ethz.sis.openbis.generic.server.api.v3.cache.SearchCacheKey; import ch.ethz.sis.openbis.generic.server.api.v3.executor.IOperationContext; import ch.ethz.sis.openbis.generic.server.api.v3.executor.common.ISearchObjectExecutor; import ch.ethz.sis.openbis.generic.server.api.v3.translator.ITranslator; import ch.ethz.sis.openbis.generic.server.api.v3.translator.TranslationContext; import ch.ethz.sis.openbis.generic.shared.api.v3.dto.fetchoptions.CacheMode; import ch.ethz.sis.openbis.generic.shared.api.v3.dto.fetchoptions.FetchOptions; -import ch.ethz.sis.openbis.generic.shared.api.v3.dto.fetchoptions.FetchOptionsMatcher; import ch.ethz.sis.openbis.generic.shared.api.v3.dto.fetchoptions.sort.SortAndPage; import ch.ethz.sis.openbis.generic.shared.api.v3.dto.search.AbstractObjectSearchCriterion; import ch.ethz.sis.openbis.generic.shared.api.v3.dto.search.SearchResult; import ch.systemsx.cisd.common.logging.LogCategory; import ch.systemsx.cisd.common.logging.LogFactory; -import ch.systemsx.cisd.openbis.generic.shared.dto.Session; /** * @author pkupczyk @@ -91,7 +91,7 @@ public abstract class AbstractSearchMethodExecutor<OBJECT, OBJECT_PE, CRITERION return doSearchAndTranslate(context, criterion, fetchOptions); } else if (CacheMode.CACHE.equals(fetchOptions.getCacheMode()) || CacheMode.RELOAD_AND_CACHE.equals(fetchOptions.getCacheMode())) { - CacheEntry entry = getCacheEntry(context, criterion, fetchOptions); + SearchCacheEntry<OBJECT> entry = getCacheEntry(context, criterion, fetchOptions); populateCacheEntry(context, criterion, fetchOptions, entry); return entry.getObjects(); } else @@ -138,11 +138,12 @@ public abstract class AbstractSearchMethodExecutor<OBJECT, OBJECT_PE, CRITERION } @SuppressWarnings("unchecked") - private CacheEntry getCacheEntry(IOperationContext context, CRITERION criterion, FETCH_OPTIONS fetchOptions) + private SearchCacheEntry<OBJECT> getCacheEntry(IOperationContext context, CRITERION criterion, FETCH_OPTIONS fetchOptions) { Cache cache = getCache(); - CacheKey key = new CacheKey(context.getSession().getSessionToken(), criterion, fetchOptions); - CacheEntry entry = null; + SearchCacheKey<CRITERION, FETCH_OPTIONS> key = + new SearchCacheKey<CRITERION, FETCH_OPTIONS>(context.getSession().getSessionToken(), criterion, fetchOptions); + SearchCacheEntry<OBJECT> entry = null; if (operationLog.isDebugEnabled()) { @@ -165,12 +166,12 @@ public abstract class AbstractSearchMethodExecutor<OBJECT, OBJECT_PE, CRITERION if (wrapper == null) { - entry = new CacheEntry(); + entry = new SearchCacheEntry<OBJECT>(); cache.put(key, entry); - context.getSession().addCleanupListener(new CacheCleanupListener(key)); + context.getSession().addCleanupListener(new SearchCacheCleanupListener<CRITERION, FETCH_OPTIONS>(cache, key)); } else { - entry = (CacheEntry) wrapper.get(); + entry = (SearchCacheEntry<OBJECT>) wrapper.get(); } if (operationLog.isDebugEnabled()) @@ -182,7 +183,7 @@ public abstract class AbstractSearchMethodExecutor<OBJECT, OBJECT_PE, CRITERION return entry; } - private void populateCacheEntry(IOperationContext context, CRITERION criterion, FETCH_OPTIONS fetchOptions, CacheEntry entry) + private void populateCacheEntry(IOperationContext context, CRITERION criterion, FETCH_OPTIONS fetchOptions, SearchCacheEntry<OBJECT> entry) { if (operationLog.isDebugEnabled()) { @@ -199,9 +200,9 @@ public abstract class AbstractSearchMethodExecutor<OBJECT, OBJECT_PE, CRITERION if (entry.getObjects() == null) { entry.setObjects(doSearchAndTranslate(context, criterion, fetchOptions)); - - operationLog.info("Updated cache entry " + entry.hashCode() + " to contain search result with " + entry.getObjects().size() - + " object(s)."); + SearchCacheKey<CRITERION, FETCH_OPTIONS> key = + new SearchCacheKey<CRITERION, FETCH_OPTIONS>(context.getSession().getSessionToken(), criterion, fetchOptions); + getCache().put(key, entry); } else { operationLog.info("Found cache entry " + entry.hashCode() + " that contains search result with " + entry.getObjects().size() @@ -215,93 +216,4 @@ public abstract class AbstractSearchMethodExecutor<OBJECT, OBJECT_PE, CRITERION } } - public class CacheKey implements Serializable - { - - private static final long serialVersionUID = 1L; - - private String sessionToken; - - private CRITERION criterion; - - private FETCH_OPTIONS fetchOptions; - - public CacheKey(String sessionToken, CRITERION criterion, FETCH_OPTIONS fetchOptions) - { - this.sessionToken = sessionToken; - this.criterion = criterion; - this.fetchOptions = fetchOptions; - } - - @Override - public int hashCode() - { - return sessionToken.hashCode() + criterion.getClass().hashCode() + fetchOptions.getClass().hashCode(); - } - - @Override - @SuppressWarnings("unchecked") - public boolean equals(Object obj) - { - if (this == obj) - { - return true; - } - if (obj == null) - { - return false; - } - if (getClass() != obj.getClass()) - { - return false; - } - - CacheKey other = (CacheKey) obj; - return sessionToken.equals(other.sessionToken) && criterion.equals(other.criterion) - && FetchOptionsMatcher.arePartsEqual(fetchOptions, other.fetchOptions); - } - } - - public class CacheEntry implements Serializable - { - private static final long serialVersionUID = 1L; - - private Collection<OBJECT> objects; - - public Collection<OBJECT> getObjects() - { - return objects; - } - - public void setObjects(Collection<OBJECT> objects) - { - this.objects = objects; - } - - } - - private class CacheCleanupListener implements Session.ISessionCleaner - { - - private CacheKey key; - - public CacheCleanupListener(CacheKey key) - { - this.key = key; - } - - @Override - public void cleanup() - { - ValueWrapper wrapper = getCache().get(key); - - if (wrapper != null) - { - operationLog.info("Clean up cached search result on logout."); - getCache().evict(key); - } - } - - } - } diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/AbstractCachingTranslator.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/AbstractCachingTranslator.java index c31d143e156..18f7d390be0 100644 --- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/AbstractCachingTranslator.java +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/AbstractCachingTranslator.java @@ -39,6 +39,8 @@ public abstract class AbstractCachingTranslator<I, O, F extends FetchOptions<?>> private final Logger operationLog = LogFactory.getLogger(LogCategory.OPERATION, getClass()); + private final String namespace = getClass().getName(); + @Override protected final O doTranslate(TranslationContext context, I object, F fetchOptions) { @@ -63,7 +65,7 @@ public abstract class AbstractCachingTranslator<I, O, F extends FetchOptions<?>> for (I input : inputs) { - if (cache.hasTranslatedObject(getClass().getName(), getId(input))) + if (cache.hasTranslatedObject(namespace, getId(input))) { handleAlreadyTranslatedInput(context, input, translated, updated, fetchOptions); } else @@ -91,7 +93,7 @@ public abstract class AbstractCachingTranslator<I, O, F extends FetchOptions<?>> Long id = getId(input); TranslationCache cache = context.getTranslationCache(); - O output = (O) cache.getTranslatedObject(getClass().getName(), id); + O output = (O) cache.getTranslatedObject(namespace, id); if (output == null) { @@ -134,7 +136,7 @@ public abstract class AbstractCachingTranslator<I, O, F extends FetchOptions<?>> operationLog.debug("Created: " + output.getClass() + " with id: " + id); } - cache.putTranslatedObject(getClass().getName(), id, output); + cache.putTranslatedObject(namespace, id, output); cache.setFetchedWithOptions(output, fetchOptions); updated.put(input, output); translated.put(input, output); @@ -169,9 +171,9 @@ public abstract class AbstractCachingTranslator<I, O, F extends FetchOptions<?>> { Long id = getId(input); - if (cache.hasShouldTranslateObject(getClass().getName(), id)) + if (cache.hasShouldTranslateObject(namespace, id)) { - boolean should = cache.getShouldTranslateObject(getClass().getName(), id); + boolean should = cache.getShouldTranslateObject(namespace, id); if (should) { toTranslate.add(input); @@ -199,7 +201,7 @@ public abstract class AbstractCachingTranslator<I, O, F extends FetchOptions<?>> for (I input : checked) { Long id = getId(input); - cache.putShouldTranslateObject(getClass().getName(), id, true); + cache.putShouldTranslateObject(namespace, id, true); if (operationLog.isDebugEnabled()) { operationLog.debug("Should translate object with id: " + id); @@ -210,7 +212,7 @@ public abstract class AbstractCachingTranslator<I, O, F extends FetchOptions<?>> for (I input : toCheck) { Long id = getId(input); - cache.putShouldTranslateObject(getClass().getName(), id, false); + cache.putShouldTranslateObject(namespace, id, false); if (operationLog.isDebugEnabled()) { operationLog.debug("Should NOT translate object with id: " + id); diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/TranslationCache.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/TranslationCache.java index 872be0654aa..2a2f946f733 100644 --- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/TranslationCache.java +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/TranslationCache.java @@ -56,16 +56,28 @@ public class TranslationCache public boolean isFetchedWithOptions(Object object, Object fetchOptions) { - return usedFetchOptions.containsKey(object) && usedFetchOptions.get(object).contains(fetchOptions); + Set<Object> set = usedFetchOptions.get(object); + + if (set == null) + { + return false; + } else + { + return set.contains(fetchOptions); + } } public void setFetchedWithOptions(Object object, Object fetchOptions) { - if (false == usedFetchOptions.containsKey(object)) + Set<Object> set = usedFetchOptions.get(object); + + if (set == null) { - usedFetchOptions.put(object, new HashSet<Object>()); + set = new HashSet<Object>(); + usedFetchOptions.put(object, set); } - usedFetchOptions.get(object).add(fetchOptions); + + set.add(fetchOptions); } private String getObjectKey(String namespace, Long objectId) diff --git a/openbis/source/java/ehcache.xml b/openbis/source/java/ehcache.xml index 3ba7d110916..c83eba355a6 100644 --- a/openbis/source/java/ehcache.xml +++ b/openbis/source/java/ehcache.xml @@ -13,10 +13,11 @@ overflowToDisk="false" diskPersistent="false" memoryStoreEvictionPolicy="LFU" /> - <cache name="searchCache" eternal="false" maxElementsInMemory="100" - overflowToDisk="false" diskPersistent="false" timeToIdleSeconds="0" - timeToLiveSeconds="300" memoryStoreEvictionPolicy="LRU"> + <cache name="searchCache" eternal="false" maxBytesLocalHeap="1G" + overflowToDisk="false" diskPersistent="false" timeToIdleSeconds="3600" + timeToLiveSeconds="0" memoryStoreEvictionPolicy="LRU"> <cacheEventListenerFactory class="ch.ethz.sis.openbis.generic.server.api.v3.cache.SearchCacheEventListenerFactory" /> + <sizeOfPolicy maxDepth="100000000" maxDepthExceededBehavior="continue"/> </cache> </ehcache> \ No newline at end of file -- GitLab