From b65a6fe1a838d1c74b8d685de2fef57ef14a1abc Mon Sep 17 00:00:00 2001 From: Derek Hulley Date: Thu, 14 Jun 2007 08:25:13 +0000 Subject: [PATCH] Relaxed TransactionalCache - Treating the shared cache like a database w.r.t. concurrency leads to too many conflicts, when it is quite reasonable to just throw the cached values away when in doubt. Fixed cache size tracing git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@5949 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../org/alfresco/repo/cache/CacheTest.java | 140 ++++++++++-------- .../alfresco/repo/cache/EhCacheTracerJob.java | 2 +- .../repo/cache/TransactionalCache.java | 105 +------------ 3 files changed, 81 insertions(+), 166 deletions(-) diff --git a/source/java/org/alfresco/repo/cache/CacheTest.java b/source/java/org/alfresco/repo/cache/CacheTest.java index 66d3de1464..b4815a1e78 100644 --- a/source/java/org/alfresco/repo/cache/CacheTest.java +++ b/source/java/org/alfresco/repo/cache/CacheTest.java @@ -33,14 +33,12 @@ import javax.transaction.UserTransaction; import junit.framework.TestCase; import net.sf.ehcache.CacheManager; -import org.alfresco.error.ExceptionStackUtil; import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; import org.alfresco.service.ServiceRegistry; import org.alfresco.service.transaction.TransactionService; import org.alfresco.util.ApplicationContextHelper; import org.springframework.context.ApplicationContext; import org.springframework.context.support.ClassPathXmlApplicationContext; -import org.springframework.dao.ConcurrencyFailureException; /** * @see org.alfresco.repo.cache.EhCacheAdapter @@ -54,9 +52,9 @@ public class CacheTest extends TestCase ); private ServiceRegistry serviceRegistry; - private SimpleCache standaloneCache; - private SimpleCache backingCache; - private SimpleCache transactionalCache; + private SimpleCache standaloneCache; + private SimpleCache backingCache; + private SimpleCache transactionalCache; private SimpleCache objectCache; @SuppressWarnings("unchecked") @@ -64,9 +62,9 @@ public class CacheTest extends TestCase public void setUp() throws Exception { serviceRegistry = (ServiceRegistry) ctx.getBean(ServiceRegistry.SERVICE_REGISTRY); - standaloneCache = (SimpleCache) ctx.getBean("ehCache1"); - backingCache = (SimpleCache) ctx.getBean("backingCache"); - transactionalCache = (SimpleCache) ctx.getBean("transactionalCache"); + standaloneCache = (SimpleCache) ctx.getBean("ehCache1"); + backingCache = (SimpleCache) ctx.getBean("backingCache"); + transactionalCache = (SimpleCache) ctx.getBean("transactionalCache"); objectCache = (SimpleCache) ctx.getBean("objectCache"); } @@ -196,7 +194,7 @@ public class CacheTest extends TestCase * @param objectCount * @return Returns the time it took in nanoseconds. */ - public long runPerformanceTestOnCache(SimpleCache cache, int objectCount) + public long runPerformanceTestOnCache(SimpleCache cache, int objectCount) { // preload for (int i = 0; i < objectCount; i++) @@ -279,9 +277,8 @@ public class CacheTest extends TestCase } } - private static final Class[] CONCURRENCY_EXCEPTIONS = {ConcurrencyFailureException.class}; - /** Execute the callback and ensure that the concurrent condition is detected */ - private void executeAndCheck(RetryingTransactionCallback callback) throws Exception + /** Execute the callback and ensure that the backing cache is left with the expected value */ + private void executeAndCheck(RetryingTransactionCallback callback, Serializable key, Object expectedValue) throws Throwable { TransactionService transactionService = serviceRegistry.getTransactionService(); UserTransaction txn = transactionService.getUserTransaction(); @@ -290,13 +287,6 @@ public class CacheTest extends TestCase txn.begin(); callback.execute(); txn.commit(); - fail("Failed to detect concurrent modification"); - } - catch (Throwable e) - { - assertNotNull( - "Expected a concurrency failure", - ExceptionStackUtil.getCause(e, CONCURRENCY_EXCEPTIONS)); } finally { @@ -304,6 +294,7 @@ public class CacheTest extends TestCase } } + private static final String COMMON_KEY = "A"; /** *
    *
  • Add to the transaction cache
    • @@ -311,18 +302,39 @@ public class CacheTest extends TestCase *
  • Commit
    • *
*/ - public void testConcurrentAddAgainstAdd() throws Exception + public void testConcurrentAddAgainstAdd()throws Throwable { RetryingTransactionCallback callback = new RetryingTransactionCallback() { public Object execute() throws Throwable { - transactionalCache.put("A", "AAA"); - backingCache.put("A", "aaa"); + transactionalCache.put(COMMON_KEY, "AAA"); + backingCache.put(COMMON_KEY, "aaa"); return null; } }; - executeAndCheck(callback); + executeAndCheck(callback, COMMON_KEY, null); + } + /** + *
    + *
  • Add to the transaction cache
    • + *
  • Add to the backing cache
    • + *
  • Commit
    • + *
+ */ + public void testConcurrentAddAgainstAddSame()throws Throwable + { + final Object commonValue = "AAA"; + RetryingTransactionCallback callback = new RetryingTransactionCallback() + { + public Object execute() throws Throwable + { + transactionalCache.put(COMMON_KEY, commonValue); + backingCache.put(COMMON_KEY, commonValue); + return null; + } + }; + executeAndCheck(callback, COMMON_KEY, commonValue); } /** *
    @@ -331,18 +343,18 @@ public class CacheTest extends TestCase *
  • Commit
    • *
*/ - public void testConcurrentAddAgainstClear() throws Exception + public void testConcurrentAddAgainstClear()throws Throwable { RetryingTransactionCallback callback = new RetryingTransactionCallback() { public Object execute() throws Throwable { - transactionalCache.put("A", "AAA"); + transactionalCache.put(COMMON_KEY, "AAA"); backingCache.clear(); return null; } }; - executeAndCheck(callback); + executeAndCheck(callback, COMMON_KEY, null); } /** *
    @@ -352,19 +364,19 @@ public class CacheTest extends TestCase *
  • Commit
    • *
*/ - public void testConcurrentUpdateAgainstUpdate() throws Exception + public void testConcurrentUpdateAgainstUpdate()throws Throwable { RetryingTransactionCallback callback = new RetryingTransactionCallback() { public Object execute() throws Throwable { - backingCache.put("A", "aaa1"); - transactionalCache.put("A", "AAA"); - backingCache.put("A", "aaa2"); + backingCache.put(COMMON_KEY, "aaa1"); + transactionalCache.put(COMMON_KEY, "AAA"); + backingCache.put(COMMON_KEY, "aaa2"); return null; } }; - executeAndCheck(callback); + executeAndCheck(callback, COMMON_KEY, null); } /** *
    @@ -374,19 +386,19 @@ public class CacheTest extends TestCase *
  • Commit
    • *
*/ - public void testConcurrentUpdateAgainstUpdateNull() throws Exception + public void testConcurrentUpdateAgainstUpdateNull()throws Throwable { RetryingTransactionCallback callback = new RetryingTransactionCallback() { public Object execute() throws Throwable { - backingCache.put("A", "aaa1"); - transactionalCache.put("A", "AAA"); - backingCache.put("A", null); + backingCache.put(COMMON_KEY, "aaa1"); + transactionalCache.put(COMMON_KEY, "AAA"); + backingCache.put(COMMON_KEY, null); return null; } }; - executeAndCheck(callback); + executeAndCheck(callback, COMMON_KEY, null); } /** *
    @@ -396,19 +408,19 @@ public class CacheTest extends TestCase *
  • Commit
    • *
*/ - public void testConcurrentUpdateNullAgainstUpdate() throws Exception + public void testConcurrentUpdateNullAgainstUpdate()throws Throwable { RetryingTransactionCallback callback = new RetryingTransactionCallback() { public Object execute() throws Throwable { - backingCache.put("A", "aaa1"); - transactionalCache.put("A", null); - backingCache.put("A", "aaa2"); + backingCache.put(COMMON_KEY, "aaa1"); + transactionalCache.put(COMMON_KEY, null); + backingCache.put(COMMON_KEY, "aaa2"); return null; } }; - executeAndCheck(callback); + executeAndCheck(callback, COMMON_KEY, null); } /** *
    @@ -418,19 +430,19 @@ public class CacheTest extends TestCase *
  • Commit
    • *
*/ - public void testConcurrentUpdateAgainstRemove() throws Exception + public void testConcurrentUpdateAgainstRemove()throws Throwable { RetryingTransactionCallback callback = new RetryingTransactionCallback() { public Object execute() throws Throwable { - backingCache.put("A", "aaa1"); - transactionalCache.put("A", "AAA"); - backingCache.remove("A"); + backingCache.put(COMMON_KEY, "aaa1"); + transactionalCache.put(COMMON_KEY, "AAA"); + backingCache.remove(COMMON_KEY); return null; } }; - executeAndCheck(callback); + executeAndCheck(callback, COMMON_KEY, null); } /** *
    @@ -440,19 +452,19 @@ public class CacheTest extends TestCase *
  • Commit
    • *
*/ - public void testConcurrentUpdateAgainstClear() throws Exception + public void testConcurrentUpdateAgainstClear()throws Throwable { RetryingTransactionCallback callback = new RetryingTransactionCallback() { public Object execute() throws Throwable { - backingCache.put("A", "aaa1"); - transactionalCache.put("A", "AAA"); + backingCache.put(COMMON_KEY, "aaa1"); + transactionalCache.put(COMMON_KEY, "AAA"); backingCache.clear(); return null; } }; - executeAndCheck(callback); + executeAndCheck(callback, COMMON_KEY, null); } /** *
    @@ -462,19 +474,19 @@ public class CacheTest extends TestCase *
  • Commit
    • *
*/ - public void testConcurrentRemoveAgainstUpdate() throws Exception + public void testConcurrentRemoveAgainstUpdate()throws Throwable { RetryingTransactionCallback callback = new RetryingTransactionCallback() { public Object execute() throws Throwable { - backingCache.put("A", "aaa1"); - transactionalCache.remove("A"); - backingCache.put("A", "aaa2"); + backingCache.put(COMMON_KEY, "aaa1"); + transactionalCache.remove(COMMON_KEY); + backingCache.put(COMMON_KEY, "aaa2"); return null; } }; - executeAndCheck(callback); + executeAndCheck(callback, COMMON_KEY, null); } /** *
    @@ -484,19 +496,19 @@ public class CacheTest extends TestCase *
  • Commit
    • *
*/ - public void testConcurrentRemoveAgainstRemove() throws Exception + public void testConcurrentRemoveAgainstRemove()throws Throwable { RetryingTransactionCallback callback = new RetryingTransactionCallback() { public Object execute() throws Throwable { - backingCache.put("A", "aaa1"); - transactionalCache.remove("A"); - backingCache.remove("A"); + backingCache.put(COMMON_KEY, "aaa1"); + transactionalCache.remove(COMMON_KEY); + backingCache.remove(COMMON_KEY); return null; } }; - executeAndCheck(callback); + executeAndCheck(callback, COMMON_KEY, null); } /** *
    @@ -506,18 +518,18 @@ public class CacheTest extends TestCase *
  • Commit
    • *
*/ - public void testConcurrentRemoveAgainstClear() throws Exception + public void testConcurrentRemoveAgainstClear()throws Throwable { RetryingTransactionCallback callback = new RetryingTransactionCallback() { public Object execute() throws Throwable { - backingCache.put("A", "aaa1"); - transactionalCache.remove("A"); + backingCache.put(COMMON_KEY, "aaa1"); + transactionalCache.remove(COMMON_KEY); backingCache.clear(); return null; } }; - executeAndCheck(callback); + executeAndCheck(callback, COMMON_KEY, null); } } diff --git a/source/java/org/alfresco/repo/cache/EhCacheTracerJob.java b/source/java/org/alfresco/repo/cache/EhCacheTracerJob.java index f96c7c7ad0..e4f9ab3046 100644 --- a/source/java/org/alfresco/repo/cache/EhCacheTracerJob.java +++ b/source/java/org/alfresco/repo/cache/EhCacheTracerJob.java @@ -82,7 +82,7 @@ public class EhCacheTracerJob implements Job { if (cacheManager == null) { - cacheManager = CacheManager.getInstance(); + cacheManager = InternalEhCacheManagerFactoryBean.getInstance(); } long maxHeapSize = Runtime.getRuntime().maxMemory(); diff --git a/source/java/org/alfresco/repo/cache/TransactionalCache.java b/source/java/org/alfresco/repo/cache/TransactionalCache.java index 0b26be12e9..6f27ae7f85 100644 --- a/source/java/org/alfresco/repo/cache/TransactionalCache.java +++ b/source/java/org/alfresco/repo/cache/TransactionalCache.java @@ -41,7 +41,6 @@ import org.alfresco.util.EqualsHelper; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.InitializingBean; -import org.springframework.dao.ConcurrencyFailureException; import org.springframework.util.Assert; /** @@ -535,40 +534,19 @@ public class TransactionalCache { } + /** + * NO-OP + */ public void beforeCompletion() { } /** - * Check that the cache used is not out of date with the shared cache. - * Note that the clear flag is ignored as this would have removed all - * entries from the local caches anyway. + * NO-OP */ @SuppressWarnings("unchecked") public void beforeCommit(boolean readOnly) { - if (isDebugEnabled) - { - logger.debug("Processing pre-commit"); - } - - TransactionData txnData = getTransactionData(); - // Process all the updates or new entries - for (Object obj : txnData.updatedItemsCache.getKeys()) - { - Serializable key = (Serializable) obj; - Element element = txnData.updatedItemsCache.get(key); - CacheBucket bucket = (CacheBucket) element.getValue(); - bucket.doPreCommit(sharedCache, key); - } - // Process all the removals - for (Object obj : txnData.removedItemsCache.getKeys()) - { - Serializable key = (Serializable) obj; - Element element = txnData.removedItemsCache.get(key); - CacheBucket bucket = (CacheBucket) element.getValue(); - bucket.doPreCommit(sharedCache, key); - } } /** @@ -667,13 +645,6 @@ public class TransactionalCache * @return Returns the bucket's value */ BV getValue(); - /** - * Check that any new cache values have not been superceded by new values in the shared cache. - * - * @param sharedCache the cache to check - * @param key the key that the bucket was stored against - */ - void doPreCommit(SimpleCache sharedCache, Serializable key); /** * Flush the current bucket to the shared cache as far as possible. * @@ -702,19 +673,6 @@ public class TransactionalCache { return value; } - public void doPreCommit(SimpleCache sharedCache, Serializable key) - { - if (sharedCache.contains(key)) - { - // The shared cache has a value where there wasn't one before. More than likely, - // this transaction would have used or modified that shared value. - throw new ConcurrencyFailureException( - "New cache bucket found new shared cache entry: \n" + - " Cache: " + name + "\n" + - " Key: " + key); - - } - } public void doPostCommit(SimpleCache sharedCache, Serializable key) { if (sharedCache.contains(key)) @@ -750,33 +708,6 @@ public class TransactionalCache { return originalValue; } - public void doPreCommit(SimpleCache sharedCache, Serializable key) - { - if (sharedCache.contains(key)) - { - BV sharedValue = sharedCache.get(key); - if (sharedValue == getOriginalValue()) - { - // The cache entry is safe for writing - } - else - { - // The shared value has moved on since - throw new ConcurrencyFailureException( - "Update cache bucket found newer shared cache entry: \n" + - " Cache: " + name + "\n" + - " Key: " + key); - } - } - else - { - // The key was removed from the shared cache. This instance cannot update the entry. - throw new ConcurrencyFailureException( - "Update cache bucket couldn't fine entry to update: \n" + - " Cache: " + name + "\n" + - " Key: " + key); - } - } public void doPostCommit(SimpleCache sharedCache, Serializable key) { BV sharedValue = sharedCache.get(key); @@ -812,34 +743,6 @@ public class TransactionalCache { super(originalValue, null); } - public void doPreCommit(SimpleCache sharedCache, Serializable key) - { - BV sharedValue = sharedCache.get(key); - if (sharedValue != null) - { - if (sharedValue == getOriginalValue()) - { - // The shared cache entry is the same as the one we were flagged to remove - } - else - { - // The shared value has moved on since - throw new ConcurrencyFailureException( - "Remove cache bucket found newer shared cache entry: \n" + - " Cache: " + name + "\n" + - " Key: " + key); - } - } - else - { - // The shared cache no longer has the value. It is possible that the removal of the - // item is something that would have affected the behaviour of the current transaction. - throw new ConcurrencyFailureException( - "Remove cache bucket found new shared cache entry: \n" + - " Cache: " + name + "\n" + - " Key: " + key); - } - } public void doPostCommit(SimpleCache sharedCache, Serializable key) { if (sharedCache.contains(key))