From 25382b991fdd7a486b746a9d77445b1014c3f7b0 Mon Sep 17 00:00:00 2001 From: Derek Hulley Date: Thu, 8 Dec 2011 23:51:36 +0000 Subject: [PATCH] Merged DEV to HEAD: Cache write optimizations - Refix ALF-10665: Caches that use @@VALUE_NOT_FOUND@@ are not really immutable - Add NodeBulkLoader.setCheckNodeConsistency - Use in a transaction to ensure that the node cache views are consistent with the database views. - Increase size of contentDataCache and make it support equals checking - Details: 32162: Read-through cache changes 32163: TransactionalCache changes to support more efficient consistency guarantees - 'allowEqualsCheck' property allows cache to do a full equals check against changed shared cache values - In-transaction option 'setDisableSharedCacheReadForTransaction'. Values are cache in-transaction and written back at the end of the transaction (subject to collision rules) but the first read will not go to the shared cache. - Drop optimistic write-through in read-only transactions; they are equally likely to want to flush stale data. - Add simpler logic for mutable and allowEqualsCheck and make sure all conditions are covered by tests 32164: Cache node entity support TransactionalCache's allowEqualsCheck 32165: Add NodeDAO.setCheckNodeConsistency() method - Allows code to request that node metadata is consistent with whatever view the DB is providing - Incorporate into node concurrency tests without issue - Only one cache is affected (nodesCache) and it is enhanced by having 'allowEqualsCheck' to prevent massive flushing when multiple read transactions are all trying to push data into the shared caches, particularly during (re)indexing operations. - Further reduces the cache invalidation messages required in order to maintain consistency across the cluster 32166: Make Lucene reindex work (trackers and FTS) use enforced node consistency - bulkLoader.setCheckNodeConsistency() incorporated where 'isReadThrough' is on 32167: SOLR tracking uses NodeDAO.setCheckNodeConsistency() during node metadata retrieval - Ensures that any stale node metadata does not find its way into indexed SOLR node metadata 32207: Fix ALF-11644: AVM cleanup jobs run when WCM is not installed - Moved scheduled jobs to installable wcm-bootstrap-context.xml - Also got rid of orphan reaper warnings when running in a cluster 32208: Better hashcode for NodeVersionKey 32209: RECORD ONLY 32210: RECORD ONLY 32212: Proper fix for ALF-10665: Immutable caches do not respond well to null (=> @@VALUE_NOT_FOUND@@) - The following caches were incorrectly classed as 'immutable': propertyValueCache immutableEntityCache rootNodesCache allRootNodesCache authorityCache tagscopeSummaryCache imapMessageCache - The 'immutable' caches are: node.aspectsCache node.propertiesCache node.parentAssocsCache - The following caches support equals checks: node.nodesCache authorityLookupCache 32213: Fixed getNodeRefStatus(): nodesCache caches deleted entries as well. git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@32657 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- config/alfresco/cache-context.xml | 27 +- config/alfresco/ehcache-default.xml | 2 +- .../ehcache-custom.xml.sample.cluster | 2 +- .../Search/lucene/lucene-search-context.xml | 38 +- .../org/alfresco/repo/cache/CacheTest.java | 352 +++++++++++++----- .../org/alfresco/repo/cache/NullCache.java | 12 + .../repo/cache/TransactionalCache.java | 167 ++++++--- .../repo/domain/node/AbstractNodeDAOImpl.java | 100 +++-- .../alfresco/repo/domain/node/NodeDAO.java | 7 +- .../repo/domain/node/NodeDAOTest.java | 7 +- .../alfresco/repo/domain/node/NodeEntity.java | 19 + .../repo/domain/node/NodeVersionKey.java | 2 +- .../repo/node/ConcurrentNodeServiceTest.java | 6 +- .../alfresco/repo/node/NodeBulkLoader.java | 6 + .../repo/node/db/DbNodeServiceImpl.java | 2 +- .../ADMLuceneIndexerAndSearcherFactory.java | 15 +- .../impl/lucene/ADMLuceneIndexerImpl.java | 2 +- ...uceneUnIndexedIndexAndSearcherFactory.java | 1 + .../lucene/AbstractLuceneIndexerImpl.java | 15 + .../repo/solr/SOLRTrackingComponent.java | 1 - .../repo/solr/SOLRTrackingComponentImpl.java | 6 +- .../repo/solr/SOLRTrackingComponentTest.java | 12 +- 22 files changed, 536 insertions(+), 265 deletions(-) diff --git a/config/alfresco/cache-context.xml b/config/alfresco/cache-context.xml index 4115f4905a..fede969a64 100644 --- a/config/alfresco/cache-context.xml +++ b/config/alfresco/cache-context.xml @@ -45,8 +45,8 @@ org.alfresco.cache.propertyValueTransactionalCache - - + + @@ -77,8 +77,9 @@ org.alfresco.cache.contentDataTransactionalCache - + + @@ -111,8 +112,8 @@ org.alfresco.cache.immutableEntityTransactionalCache - - + + @@ -144,8 +145,8 @@ org.alfresco.cache.node.rootNodesTransactionalCache - - + + @@ -173,8 +174,8 @@ org.alfresco.cache.node.allRootNodesTransactionalCache - - + + @@ -207,6 +208,7 @@ + @@ -455,6 +457,7 @@ org.alfresco.authorityTransactionalCache + @@ -1272,8 +1275,8 @@ org.alfresco.cache.tagscopeSummaryTransactionalCache - - + + @@ -1305,7 +1308,7 @@ org.alfresco.cache.imapMessageTransactionalCache - + diff --git a/config/alfresco/ehcache-default.xml b/config/alfresco/ehcache-default.xml index 183444854a..9a74dbece9 100644 --- a/config/alfresco/ehcache-default.xml +++ b/config/alfresco/ehcache-default.xml @@ -148,7 +148,7 @@ /> - - - - - - - - - - - - - - - - - - - - - - - - + parent="search.abstractLuceneIndexerAndSearcherFactory" + class="org.alfresco.repo.search.impl.lucene.ADMLuceneIndexerAndSearcherFactory"> + + + + + + + + - + diff --git a/source/java/org/alfresco/repo/cache/CacheTest.java b/source/java/org/alfresco/repo/cache/CacheTest.java index 5e9426050f..ab634d6a23 100644 --- a/source/java/org/alfresco/repo/cache/CacheTest.java +++ b/source/java/org/alfresco/repo/cache/CacheTest.java @@ -29,11 +29,13 @@ import net.sf.ehcache.CacheManager; import org.alfresco.repo.transaction.AlfrescoTransactionSupport; import org.alfresco.repo.transaction.RetryingTransactionHelper; +import org.alfresco.repo.transaction.AlfrescoTransactionSupport.TxnReadState; import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; import org.alfresco.repo.transaction.TransactionListenerAdapter; import org.alfresco.service.ServiceRegistry; import org.alfresco.service.transaction.TransactionService; import org.alfresco.util.ApplicationContextHelper; +import org.apache.commons.lang.mutable.MutableLong; import org.springframework.context.ApplicationContext; /** @@ -58,14 +60,23 @@ public class CacheTest extends TestCase @Override public void setUp() throws Exception { + if (AlfrescoTransactionSupport.getTransactionReadState() != TxnReadState.TXN_NONE) + { + fail("A transaction is still running"); + } + serviceRegistry = (ServiceRegistry) ctx.getBean(ServiceRegistry.SERVICE_REGISTRY); standaloneCache = (SimpleCache) ctx.getBean("ehCache1"); backingCache = (SimpleCache) ctx.getBean("backingCache"); transactionalCache = (TransactionalCache) ctx.getBean("transactionalCache"); objectCache = (SimpleCache) ctx.getBean("objectCache"); + // Make sure that the backing cache is empty + backingCache.clear(); + // Make the cache mutable (default) transactionalCache.setMutable(true); + transactionalCache.setAllowEqualsChecks(false); } @Override @@ -331,6 +342,53 @@ public class CacheTest extends TestCase } } + public void testTransactionalCacheDisableSharedCaches() throws Throwable + { + // add item to global cache + backingCache.put(NEW_GLOBAL_ONE, NEW_GLOBAL_ONE); + backingCache.put(NEW_GLOBAL_TWO, NEW_GLOBAL_TWO); + backingCache.put(NEW_GLOBAL_THREE, NEW_GLOBAL_THREE); + + TransactionService transactionService = serviceRegistry.getTransactionService(); + UserTransaction txn = transactionService.getUserTransaction(); + try + { + // begin a transaction + txn.begin(); + + // Go directly past ALL shared caches + transactionalCache.setDisableSharedCacheReadForTransaction(true); + + // Try to get results in shared caches + assertNull("Read of mutable shared cache MUST NOT use backing cache", transactionalCache.get(NEW_GLOBAL_ONE)); + assertNull("Value should not be in any cache", transactionalCache.get(UPDATE_TXN_THREE)); + + // Update the transactional caches + transactionalCache.put(NEW_GLOBAL_TWO, "An update"); + transactionalCache.put(UPDATE_TXN_THREE, UPDATE_TXN_THREE); + + // Try to get results in shared caches + assertNull("Read of mutable shared cache MUST NOT use backing cache", transactionalCache.get(NEW_GLOBAL_ONE)); + assertEquals("Value should be in transactional cache", "An update", transactionalCache.get(NEW_GLOBAL_TWO)); + assertEquals("Value should be in transactional cache", UPDATE_TXN_THREE, transactionalCache.get(UPDATE_TXN_THREE)); + + txn.commit(); + + // Now check that values were not written through for any caches + assertEquals("Out-of-txn read must return shared value", NEW_GLOBAL_ONE, transactionalCache.get(NEW_GLOBAL_ONE)); + assertNull("Value should be removed from shared cache", transactionalCache.get(NEW_GLOBAL_TWO)); + assertEquals("New values must be written to shared cache", UPDATE_TXN_THREE, transactionalCache.get(UPDATE_TXN_THREE)); + } + catch (Throwable e) + { + if (txn.getStatus() == Status.STATUS_ACTIVE) + { + txn.rollback(); + } + throw e; + } + } + /** * Preloads the cache, then performs a simultaneous addition of N new values and * removal of the N preloaded values. @@ -417,7 +475,7 @@ public class CacheTest extends TestCase } finally { - try { txn.rollback(); } catch (Throwable ee) {} + try { txn.rollback(); } catch (Throwable ee) {ee.printStackTrace();} } } long end = System.nanoTime(); @@ -462,22 +520,24 @@ public class CacheTest extends TestCase * Starts off with a null in the backing cache and adds a value to the * transactional cache. There should be no problem with this. */ - public void testNullValue() throws Exception + public void testNullValue() throws Throwable { TransactionService transactionService = serviceRegistry.getTransactionService(); UserTransaction txn = transactionService.getUserTransaction(); + + txn.begin(); + + backingCache.put("A", null); + transactionalCache.put("A", "AAA"); + try { - txn.begin(); - - backingCache.put("A", null); - transactionalCache.put("A", "AAA"); - txn.commit(); } - finally + catch (Throwable e) { - try { txn.rollback(); } catch (Throwable ee) {} + try {txn.rollback();} catch (Throwable ee) {} + throw e; } } @@ -540,9 +600,15 @@ public class CacheTest extends TestCase } Object actualValue = backingCache.get(key); assertEquals("Backing cache value was not correct", expectedValue, actualValue); + + // Clear the backing cache to ensure that subsequent tests don't run into existing data + backingCache.clear(); } private static final String COMMON_KEY = "A"; + private static final MutableLong VALUE_ONE_A = new MutableLong(1L); + private static final MutableLong VALUE_ONE_B = new MutableLong(1L); + private static final MutableLong VALUE_TWO_A = new MutableLong(2L); /** *
    *
  • Add to the transaction cache
    • @@ -556,17 +622,26 @@ public class CacheTest extends TestCase { public Object execute() throws Throwable { - transactionalCache.put(COMMON_KEY, "AAA"); - backingCache.put(COMMON_KEY, "aaa"); + transactionalCache.put(COMMON_KEY, VALUE_ONE_A); + backingCache.put(COMMON_KEY, VALUE_ONE_B); return null; } }; + transactionalCache.setAllowEqualsChecks(false); transactionalCache.setMutable(true); - executeAndCheck(callback, false, COMMON_KEY, null); - executeAndCheck(callback, true, COMMON_KEY, "aaa"); + executeAndCheck(callback, false, COMMON_KEY, null); // Mutable: Pessimistic removal + executeAndCheck(callback, true, COMMON_KEY, null); // Mutable: Pessimistic removal transactionalCache.setMutable(false); - executeAndCheck(callback, false, COMMON_KEY, "AAA"); - executeAndCheck(callback, true, COMMON_KEY, "AAA"); + executeAndCheck(callback, false, COMMON_KEY, VALUE_ONE_B); // Immutable: Assume backing cache is correct + executeAndCheck(callback, true, COMMON_KEY, VALUE_ONE_B); // Immutable: Assume backing cache is correct + + transactionalCache.setAllowEqualsChecks(true); + transactionalCache.setMutable(true); + executeAndCheck(callback, false, COMMON_KEY, VALUE_ONE_B); // Mutable: Shared cache value checked + executeAndCheck(callback, true, COMMON_KEY, VALUE_ONE_B); // Mutable: Shared cache value checked + transactionalCache.setMutable(false); + executeAndCheck(callback, false, COMMON_KEY, VALUE_ONE_B); // Immutable: Assume backing cache is correct + executeAndCheck(callback, true, COMMON_KEY, VALUE_ONE_B); // Immutable: Assume backing cache is correct } /** *
      @@ -577,22 +652,30 @@ public class CacheTest extends TestCase */ 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); + transactionalCache.put(COMMON_KEY, VALUE_ONE_A); + backingCache.put(COMMON_KEY, VALUE_ONE_A); return null; } }; + transactionalCache.setAllowEqualsChecks(false); transactionalCache.setMutable(true); - executeAndCheck(callback, false, COMMON_KEY, "AAA"); - executeAndCheck(callback, true, COMMON_KEY, "AAA"); + executeAndCheck(callback, false, COMMON_KEY, VALUE_ONE_A); // Mutable: Object handle is match + executeAndCheck(callback, true, COMMON_KEY, VALUE_ONE_A); // Mutable: Object handle is match transactionalCache.setMutable(false); - executeAndCheck(callback, false, COMMON_KEY, commonValue); - executeAndCheck(callback, true, COMMON_KEY, commonValue); + executeAndCheck(callback, false, COMMON_KEY, VALUE_ONE_A); // Immutable: Object handle is match + executeAndCheck(callback, true, COMMON_KEY, VALUE_ONE_A); // Immutable: Object handle is match + + transactionalCache.setAllowEqualsChecks(true); + transactionalCache.setMutable(true); + executeAndCheck(callback, false, COMMON_KEY, VALUE_ONE_A); // Mutable: Object handle is match + executeAndCheck(callback, true, COMMON_KEY, VALUE_ONE_A); // Mutable: Object handle is match + transactionalCache.setMutable(false); + executeAndCheck(callback, false, COMMON_KEY, VALUE_ONE_A); // Immutable: Object handle is match + executeAndCheck(callback, true, COMMON_KEY, VALUE_ONE_A); // Immutable: Object handle is match } /** *
        @@ -607,17 +690,26 @@ public class CacheTest extends TestCase { public Object execute() throws Throwable { - transactionalCache.put(COMMON_KEY, "AAA"); + transactionalCache.put(COMMON_KEY, VALUE_ONE_A); backingCache.clear(); return null; } }; + transactionalCache.setAllowEqualsChecks(false); transactionalCache.setMutable(true); - executeAndCheck(callback, false, COMMON_KEY, null); - executeAndCheck(callback, true, COMMON_KEY, "AAA"); + executeAndCheck(callback, false, COMMON_KEY, VALUE_ONE_A); // Mutable: Add back to backing cache + executeAndCheck(callback, true, COMMON_KEY, VALUE_ONE_A); // Mutable: Add back to backing cache transactionalCache.setMutable(false); - executeAndCheck(callback, false, COMMON_KEY, "AAA"); - executeAndCheck(callback, true, COMMON_KEY, "AAA"); + executeAndCheck(callback, false, COMMON_KEY, VALUE_ONE_A); // Immutable: Add back to backing cache + executeAndCheck(callback, true, COMMON_KEY, VALUE_ONE_A); // Immutable: Add back to backing cache + + transactionalCache.setAllowEqualsChecks(true); + transactionalCache.setMutable(true); + executeAndCheck(callback, false, COMMON_KEY, VALUE_ONE_A); // Mutable: Add back to backing cache + executeAndCheck(callback, true, COMMON_KEY, VALUE_ONE_A); // Mutable: Add back to backing cache + transactionalCache.setMutable(false); + executeAndCheck(callback, false, COMMON_KEY, VALUE_ONE_A); // Immutable: Add back to backing cache + executeAndCheck(callback, true, COMMON_KEY, VALUE_ONE_A); // Immutable: Add back to backing cache } /** *
          @@ -633,18 +725,27 @@ public class CacheTest extends TestCase { public Object execute() throws Throwable { - backingCache.put(COMMON_KEY, "aaa1"); - transactionalCache.put(COMMON_KEY, "AAA"); - backingCache.put(COMMON_KEY, "aaa2"); + backingCache.put(COMMON_KEY, VALUE_ONE_A); + transactionalCache.put(COMMON_KEY, VALUE_ONE_B); + backingCache.put(COMMON_KEY, VALUE_TWO_A); return null; } }; + transactionalCache.setAllowEqualsChecks(false); transactionalCache.setMutable(true); - executeAndCheck(callback, false, COMMON_KEY, null); - executeAndCheck(callback, true, COMMON_KEY, "aaa2"); + executeAndCheck(callback, false, COMMON_KEY, null); // Mutable: Pessimistic removal + executeAndCheck(callback, true, COMMON_KEY, null); // Mutable: Pessimistic removal transactionalCache.setMutable(false); - executeAndCheck(callback, false, COMMON_KEY, "AAA"); - executeAndCheck(callback, true, COMMON_KEY, "AAA"); + executeAndCheck(callback, false, COMMON_KEY, VALUE_TWO_A); // Immutable: Assume backing cache is correct + executeAndCheck(callback, true, COMMON_KEY, VALUE_TWO_A); // Immutable: Assume backing cache is correct + + transactionalCache.setAllowEqualsChecks(true); + transactionalCache.setMutable(true); + executeAndCheck(callback, false, COMMON_KEY, null); // Mutable: Shared cache value checked failed + executeAndCheck(callback, true, COMMON_KEY, null); // Mutable: Shared cache value checked failed + transactionalCache.setMutable(false); + executeAndCheck(callback, false, COMMON_KEY, VALUE_TWO_A); // Immutable: Assume backing cache is correct + executeAndCheck(callback, true, COMMON_KEY, VALUE_TWO_A); // Immutable: Assume backing cache is correct } /** *
            @@ -660,18 +761,27 @@ public class CacheTest extends TestCase { public Object execute() throws Throwable { - backingCache.put(COMMON_KEY, "aaa1"); - transactionalCache.put(COMMON_KEY, "AAA"); + backingCache.put(COMMON_KEY, VALUE_ONE_A); + transactionalCache.put(COMMON_KEY, VALUE_ONE_B); backingCache.put(COMMON_KEY, null); return null; } }; + transactionalCache.setAllowEqualsChecks(false); transactionalCache.setMutable(true); - executeAndCheck(callback, false, COMMON_KEY, null); - executeAndCheck(callback, true, COMMON_KEY, null); + executeAndCheck(callback, false, COMMON_KEY, null); // Mutable: Pessimistic removal + executeAndCheck(callback, true, COMMON_KEY, null); // Mutable: Pessimistic removal transactionalCache.setMutable(false); - executeAndCheck(callback, false, COMMON_KEY, "AAA"); - executeAndCheck(callback, true, COMMON_KEY, "AAA"); + executeAndCheck(callback, false, COMMON_KEY, VALUE_ONE_B); // Immutable: Add back to backing cache + executeAndCheck(callback, true, COMMON_KEY, VALUE_ONE_B); // Immutable: Add back to backing cache + + transactionalCache.setAllowEqualsChecks(true); + transactionalCache.setMutable(true); + executeAndCheck(callback, false, COMMON_KEY, null); // Mutable: Pessimistic removal + executeAndCheck(callback, true, COMMON_KEY, null); // Mutable: Pessimistic removal + transactionalCache.setMutable(false); + executeAndCheck(callback, false, COMMON_KEY, VALUE_ONE_B); // Immutable: Add back to backing cache + executeAndCheck(callback, true, COMMON_KEY, VALUE_ONE_B); // Immutable: Add back to backing cache } /** *
              @@ -687,18 +797,27 @@ public class CacheTest extends TestCase { public Object execute() throws Throwable { - backingCache.put(COMMON_KEY, "aaa1"); + backingCache.put(COMMON_KEY, VALUE_ONE_A); transactionalCache.put(COMMON_KEY, null); - backingCache.put(COMMON_KEY, "aaa2"); + backingCache.put(COMMON_KEY, VALUE_ONE_B); return null; } }; + transactionalCache.setAllowEqualsChecks(false); transactionalCache.setMutable(true); - executeAndCheck(callback, false, COMMON_KEY, null); - executeAndCheck(callback, true, COMMON_KEY, "aaa2"); + executeAndCheck(callback, false, COMMON_KEY, null); // Mutable: Pessimistic removal + executeAndCheck(callback, true, COMMON_KEY, null); // Mutable: Pessimistic removal transactionalCache.setMutable(false); - executeAndCheck(callback, false, COMMON_KEY, null); - executeAndCheck(callback, true, COMMON_KEY, null); + executeAndCheck(callback, false, COMMON_KEY, VALUE_ONE_B); // Immutable: Assume backing cache is correct + executeAndCheck(callback, true, COMMON_KEY, VALUE_ONE_B); // Immutable: Assume backing cache is correct + + transactionalCache.setAllowEqualsChecks(true); + transactionalCache.setMutable(true); + executeAndCheck(callback, false, COMMON_KEY, null); // Mutable: Pessimistic removal + executeAndCheck(callback, true, COMMON_KEY, null); // Mutable: Pessimistic removal + transactionalCache.setMutable(false); + executeAndCheck(callback, false, COMMON_KEY, VALUE_ONE_B); // Immutable: Assume backing cache is correct + executeAndCheck(callback, true, COMMON_KEY, VALUE_ONE_B); // Immutable: Assume backing cache is correct } /** *
                @@ -714,18 +833,27 @@ public class CacheTest extends TestCase { public Object execute() throws Throwable { - backingCache.put(COMMON_KEY, "aaa1"); - transactionalCache.put(COMMON_KEY, "AAA"); + backingCache.put(COMMON_KEY, VALUE_ONE_A); + transactionalCache.put(COMMON_KEY, VALUE_ONE_B); backingCache.remove(COMMON_KEY); return null; } }; + transactionalCache.setAllowEqualsChecks(false); transactionalCache.setMutable(true); - executeAndCheck(callback, false, COMMON_KEY, null); - executeAndCheck(callback, true, COMMON_KEY, null); + executeAndCheck(callback, false, COMMON_KEY, null); // Mutable: Pessimistic removal + executeAndCheck(callback, true, COMMON_KEY, null); // Mutable: Pessimistic removal transactionalCache.setMutable(false); - executeAndCheck(callback, false, COMMON_KEY, "AAA"); - executeAndCheck(callback, true, COMMON_KEY, "AAA"); + executeAndCheck(callback, false, COMMON_KEY, VALUE_ONE_B); // Immutable: Add back to backing cache + executeAndCheck(callback, true, COMMON_KEY, VALUE_ONE_B); // Immutable: Add back to backing cache + + transactionalCache.setAllowEqualsChecks(true); + transactionalCache.setMutable(true); + executeAndCheck(callback, false, COMMON_KEY, null); // Mutable: Pessimistic removal + executeAndCheck(callback, true, COMMON_KEY, null); // Mutable: Pessimistic removal + transactionalCache.setMutable(false); + executeAndCheck(callback, false, COMMON_KEY, VALUE_ONE_B); // Immutable: Add back to backing cache + executeAndCheck(callback, true, COMMON_KEY, VALUE_ONE_B); // Immutable: Add back to backing cache } /** *
                  @@ -741,18 +869,27 @@ public class CacheTest extends TestCase { public Object execute() throws Throwable { - backingCache.put(COMMON_KEY, "aaa1"); - transactionalCache.put(COMMON_KEY, "AAA"); + backingCache.put(COMMON_KEY, VALUE_ONE_A); + transactionalCache.put(COMMON_KEY, VALUE_ONE_B); backingCache.clear(); return null; } }; + transactionalCache.setAllowEqualsChecks(false); transactionalCache.setMutable(true); - executeAndCheck(callback, false, COMMON_KEY, null); - executeAndCheck(callback, true, COMMON_KEY, null); + executeAndCheck(callback, false, COMMON_KEY, null); // Mutable: Pessimistic removal + executeAndCheck(callback, true, COMMON_KEY, null); // Mutable: Pessimistic removal transactionalCache.setMutable(false); - executeAndCheck(callback, false, COMMON_KEY, "AAA"); - executeAndCheck(callback, true, COMMON_KEY, "AAA"); + executeAndCheck(callback, false, COMMON_KEY, VALUE_ONE_B); // Immutable: Add back to backing cache + executeAndCheck(callback, true, COMMON_KEY, VALUE_ONE_B); // Immutable: Add back to backing cache + + transactionalCache.setAllowEqualsChecks(true); + transactionalCache.setMutable(true); + executeAndCheck(callback, false, COMMON_KEY, null); // Mutable: Pessimistic removal + executeAndCheck(callback, true, COMMON_KEY, null); // Mutable: Pessimistic removal + transactionalCache.setMutable(false); + executeAndCheck(callback, false, COMMON_KEY, VALUE_ONE_B); // Immutable: Add back to backing cache + executeAndCheck(callback, true, COMMON_KEY, VALUE_ONE_B); // Immutable: Add back to backing cache } /** *
                    @@ -770,16 +907,25 @@ public class CacheTest extends TestCase { backingCache.remove(COMMON_KEY); transactionalCache.remove(COMMON_KEY); - backingCache.put(COMMON_KEY, "aaa2"); + backingCache.put(COMMON_KEY, VALUE_ONE_B); return null; } }; + transactionalCache.setAllowEqualsChecks(false); transactionalCache.setMutable(true); - executeAndCheck(callback, false, COMMON_KEY, null); - executeAndCheck(callback, true, COMMON_KEY, null); + executeAndCheck(callback, false, COMMON_KEY, null); // Mutable: Pessimistic removal + executeAndCheck(callback, true, COMMON_KEY, null); // Mutable: Pessimistic removal transactionalCache.setMutable(false); - executeAndCheck(callback, false, COMMON_KEY, null); - executeAndCheck(callback, true, COMMON_KEY, null); + executeAndCheck(callback, false, COMMON_KEY, null); // Immutable: Remove from backing cache + executeAndCheck(callback, true, COMMON_KEY, null); // Immutable: Remove from backing cache + + transactionalCache.setAllowEqualsChecks(true); + transactionalCache.setMutable(true); + executeAndCheck(callback, false, COMMON_KEY, null); // Mutable: Pessimistic removal + executeAndCheck(callback, true, COMMON_KEY, null); // Mutable: Pessimistic removal + transactionalCache.setMutable(false); + executeAndCheck(callback, false, COMMON_KEY, null); // Immutable: Remove from backing cache + executeAndCheck(callback, true, COMMON_KEY, null); // Immutable: Remove from backing cache } /** *
                      @@ -796,17 +942,26 @@ public class CacheTest extends TestCase public Object execute() throws Throwable { backingCache.remove(COMMON_KEY); - transactionalCache.put(COMMON_KEY, "aaa2-x"); - backingCache.put(COMMON_KEY, "aaa2"); + transactionalCache.put(COMMON_KEY, VALUE_ONE_A); + backingCache.put(COMMON_KEY, VALUE_ONE_B); return null; } }; + transactionalCache.setAllowEqualsChecks(false); transactionalCache.setMutable(true); - executeAndCheck(callback, false, COMMON_KEY, null); - executeAndCheck(callback, true, COMMON_KEY, "aaa2"); // Doesn't write through + executeAndCheck(callback, false, COMMON_KEY, null); // Mutable: Pessimistic removal + executeAndCheck(callback, true, COMMON_KEY, null); // Mutable: Pessimistic removal transactionalCache.setMutable(false); - executeAndCheck(callback, false, COMMON_KEY, "aaa2-x"); // Always overwrites - executeAndCheck(callback, true, COMMON_KEY, "aaa2-x"); // Always overwrites + executeAndCheck(callback, false, COMMON_KEY, VALUE_ONE_B); // Immutable: Assume backing cache is correct + executeAndCheck(callback, true, COMMON_KEY, VALUE_ONE_B); // Immutable: Assume backing cache is correct + + transactionalCache.setAllowEqualsChecks(true); + transactionalCache.setMutable(true); + executeAndCheck(callback, false, COMMON_KEY, VALUE_ONE_B); // Mutable: Shared cache value checked + executeAndCheck(callback, true, COMMON_KEY, VALUE_ONE_B); // Mutable: Shared cache value checked + transactionalCache.setMutable(false); + executeAndCheck(callback, false, COMMON_KEY, VALUE_ONE_B); // Immutable: Assume backing cache is correct + executeAndCheck(callback, true, COMMON_KEY, VALUE_ONE_B); // Immutable: Assume backing cache is correct } /** *
                        @@ -822,18 +977,27 @@ public class CacheTest extends TestCase { public Object execute() throws Throwable { - backingCache.put(COMMON_KEY, "aaa1"); + backingCache.put(COMMON_KEY, VALUE_ONE_A); transactionalCache.remove(COMMON_KEY); - backingCache.put(COMMON_KEY, "aaa2"); + backingCache.put(COMMON_KEY, VALUE_ONE_B); return null; } }; + transactionalCache.setAllowEqualsChecks(false); transactionalCache.setMutable(true); - executeAndCheck(callback, false, COMMON_KEY, null); - executeAndCheck(callback, true, COMMON_KEY, null); + executeAndCheck(callback, false, COMMON_KEY, null); // Mutable: Pessimistic removal + executeAndCheck(callback, true, COMMON_KEY, null); // Mutable: Pessimistic removal transactionalCache.setMutable(false); - executeAndCheck(callback, false, COMMON_KEY, null); - executeAndCheck(callback, true, COMMON_KEY, null); + executeAndCheck(callback, false, COMMON_KEY, null); // Immutable: Remove from backing cache + executeAndCheck(callback, true, COMMON_KEY, null); // Immutable: Remove from backing cache + + transactionalCache.setAllowEqualsChecks(true); + transactionalCache.setMutable(true); + executeAndCheck(callback, false, COMMON_KEY, null); // Mutable: Pessimistic removal + executeAndCheck(callback, true, COMMON_KEY, null); // Mutable: Pessimistic removal + transactionalCache.setMutable(false); + executeAndCheck(callback, false, COMMON_KEY, null); // Immutable: Remove from backing cache + executeAndCheck(callback, true, COMMON_KEY, null); // Immutable: Remove from backing cache } /** *
                          @@ -849,18 +1013,27 @@ public class CacheTest extends TestCase { public Object execute() throws Throwable { - backingCache.put(COMMON_KEY, "aaa1"); + backingCache.put(COMMON_KEY, VALUE_ONE_A); transactionalCache.remove(COMMON_KEY); backingCache.remove(COMMON_KEY); return null; } }; + transactionalCache.setAllowEqualsChecks(false); transactionalCache.setMutable(true); - executeAndCheck(callback, false, COMMON_KEY, null); - executeAndCheck(callback, true, COMMON_KEY, null); + executeAndCheck(callback, false, COMMON_KEY, null); // Mutable: Remove from backing cache + executeAndCheck(callback, true, COMMON_KEY, null); // Mutable: Remove from backing cache transactionalCache.setMutable(false); - executeAndCheck(callback, false, COMMON_KEY, null); - executeAndCheck(callback, true, COMMON_KEY, null); + executeAndCheck(callback, false, COMMON_KEY, null); // Immutable: Remove from backing cache + executeAndCheck(callback, true, COMMON_KEY, null); // Immutable: Remove from backing cache + + transactionalCache.setAllowEqualsChecks(true); + transactionalCache.setMutable(true); + executeAndCheck(callback, false, COMMON_KEY, null); // Mutable: Remove from backing cache + executeAndCheck(callback, true, COMMON_KEY, null); // Mutable: Remove from backing cache + transactionalCache.setMutable(false); + executeAndCheck(callback, false, COMMON_KEY, null); // Immutable: Remove from backing cache + executeAndCheck(callback, true, COMMON_KEY, null); // Immutable: Remove from backing cache } /** *
                            @@ -876,17 +1049,26 @@ public class CacheTest extends TestCase { public Object execute() throws Throwable { - backingCache.put(COMMON_KEY, "aaa1"); + backingCache.put(COMMON_KEY, VALUE_ONE_A); transactionalCache.remove(COMMON_KEY); backingCache.clear(); return null; } }; + transactionalCache.setAllowEqualsChecks(false); transactionalCache.setMutable(true); - executeAndCheck(callback, false, COMMON_KEY, null); - executeAndCheck(callback, true, COMMON_KEY, null); + executeAndCheck(callback, false, COMMON_KEY, null); // Mutable: Nothing to do + executeAndCheck(callback, true, COMMON_KEY, null); // Mutable: Nothing to do transactionalCache.setMutable(false); - executeAndCheck(callback, false, COMMON_KEY, null); - executeAndCheck(callback, true, COMMON_KEY, null); + executeAndCheck(callback, false, COMMON_KEY, null); // Immutable: Nothing to do + executeAndCheck(callback, true, COMMON_KEY, null); // Immutable: Nothing to do + + transactionalCache.setAllowEqualsChecks(true); + transactionalCache.setMutable(true); + executeAndCheck(callback, false, COMMON_KEY, null); // Mutable: Nothing to do + executeAndCheck(callback, true, COMMON_KEY, null); // Mutable: Nothing to do + transactionalCache.setMutable(false); + executeAndCheck(callback, false, COMMON_KEY, null); // Immutable: Nothing to do + executeAndCheck(callback, true, COMMON_KEY, null); // Immutable: Nothing to do } } diff --git a/source/java/org/alfresco/repo/cache/NullCache.java b/source/java/org/alfresco/repo/cache/NullCache.java index 5a22b9f8a6..89eef2999b 100644 --- a/source/java/org/alfresco/repo/cache/NullCache.java +++ b/source/java/org/alfresco/repo/cache/NullCache.java @@ -33,6 +33,18 @@ import java.util.Collections; */ public class NullCache implements SimpleCache { + /** Singleton for retrieval via {@link #getInstance() } */ + private static final NullCache INSTANCE = new NullCache(); + + /** + * @return Returns a singleton that can be used in any way - all operations are stateless + */ + @SuppressWarnings("unchecked") + public static final NullCache getInstance() + { + return (NullCache) INSTANCE; + } + public NullCache() { } diff --git a/source/java/org/alfresco/repo/cache/TransactionalCache.java b/source/java/org/alfresco/repo/cache/TransactionalCache.java index 7b9b4c2be8..cf184105fd 100644 --- a/source/java/org/alfresco/repo/cache/TransactionalCache.java +++ b/source/java/org/alfresco/repo/cache/TransactionalCache.java @@ -72,7 +72,7 @@ import org.springframework.beans.factory.InitializingBean; public class TransactionalCache implements SimpleCache, TransactionListener, InitializingBean { - private static final String RESOURCE_KEY_TXN_DATA = "TransactionalCache.TxnData"; + private static final String RESOURCE_KEY_TXN_DATA = "TransactionalCache.TxnData"; private Log logger; private boolean isDebugEnabled; @@ -85,6 +85,8 @@ public class TransactionalCache private SimpleCache sharedCache; /** can the cached values be modified */ private boolean isMutable; + /** can values be compared using full equality checking */ + private boolean allowEqualsChecks; /** the maximum number of elements to be contained in the cache */ private int maxCacheSize = 500; /** a unique string identifying this instance when binding resources */ @@ -99,6 +101,7 @@ public class TransactionalCache isDebugEnabled = logger.isDebugEnabled(); disableSharedCache = false; isMutable = true; + allowEqualsChecks = false; } /** @@ -163,6 +166,21 @@ public class TransactionalCache this.isMutable = isMutable; } + /** + * Allow equality checking of values before they are written to the shared cache on + * commit. This allows some caches to bypass unnecessary cache updates when the + * values remain unchanged. Typically, this setting should be applied only to mutable + * caches and only where the values being stored have a fast and reliable equality check. + * + * @param allowEqualsChecks true if value comparisons can be made between values + * stored in the transactional cache and those stored in the + * shared cache + */ + public void setAllowEqualsChecks(boolean allowEqualsChecks) + { + this.allowEqualsChecks = allowEqualsChecks; + } + /** * Set the maximum number of elements to store in the update and remove caches. * The maximum number of elements stored in the transaction will be twice the @@ -171,7 +189,7 @@ public class TransactionalCache * The removed list will overflow to disk in order to ensure that deletions are * not lost. * - * @param maxCacheSize + * @param maxCacheSize maximum number of items to be held in-transaction */ public void setMaxCacheSize(int maxCacheSize) { @@ -179,9 +197,7 @@ public class TransactionalCache } /** - * Set the name that identifies this cache from other instances. This is optional. - * - * @param name + * Set the name that identifies this cache from other instances. */ public void setName(String name) { @@ -205,7 +221,7 @@ public class TransactionalCache // Assign a 'null' cache if write-through is disabled if (disableSharedCache) { - sharedCache = new NullCache(); + sharedCache = NullCache.getInstance(); } } @@ -232,6 +248,23 @@ public class TransactionalCache return data; } + /** + * Transaction-long setting to force all the share cache to be bypassed for the current transaction. + *

                            + * This setting is like having a {@link NullCache null} {@link #setSharedCache(SimpleCache) shared cache}, + * but only lasts for the transaction. + *

                            + * Use this when a read transaction must see consistent and current data i.e. go to the database. + * While this is active, write operations will also not be committed to the shared cache. + * + * @param noSharedCacheRead true to avoid reading from the shared cache for the transaction + */ + public void setDisableSharedCacheReadForTransaction(boolean noSharedCacheRead) + { + TransactionData txnData = getTransactionData(); + txnData.noSharedCacheRead = noSharedCacheRead; + } + /** * Checks the transactional removed and updated caches before checking the shared cache. */ @@ -347,6 +380,11 @@ public class TransactionalCache // Can't store values in the current txn any more ignoreSharedCache = true; } + else if (txnData.noSharedCacheRead) + { + // Explicitly told to ignore shared cache + ignoreSharedCache = true; + } else { // There is no in-txn entry for the key @@ -356,8 +394,6 @@ public class TransactionalCache txnData.updatedItemsCache.put(key, bucket); return value; } - // check if the cleared flag has been set - cleared flag means ignore shared as unreliable - ignoreSharedCache = txnData.isClearOn; } } // no value found - must we ignore the shared cache? @@ -440,7 +476,7 @@ public class TransactionalCache txnData.haveIssuedFullWarning = true; } } - Object existingValueObj = sharedCache.get(key); + Object existingValueObj = txnData.noSharedCacheRead ? null : sharedCache.get(key); CacheBucket bucket = null; if (existingValueObj == null) { @@ -650,7 +686,9 @@ public class TransactionalCache { K key = entry.getKey(); CacheBucket bucket = entry.getValue(); - bucket.doPreCommit(sharedCache, key, this.isMutable, txnData.isReadOnly); + bucket.doPreCommit( + sharedCache, + key, this.isMutable, this.allowEqualsChecks, txnData.isReadOnly); } if (isDebugEnabled) { @@ -709,7 +747,9 @@ public class TransactionalCache { K key = entry.getKey(); CacheBucket bucket = entry.getValue(); - bucket.doPostCommit(sharedCache, key, this.isMutable, txnData.isReadOnly); + bucket.doPostCommit( + sharedCache, + key, this.isMutable, this.allowEqualsChecks, txnData.isReadOnly); } if (isDebugEnabled) { @@ -800,7 +840,7 @@ public class TransactionalCache public void doPreCommit( SimpleCache sharedCache, Serializable key, - boolean mutable, boolean readOnly); + boolean mutable, boolean allowEqualsCheck, boolean readOnly); /** * Flush the current bucket to the shared cache as far as possible. * @@ -810,7 +850,7 @@ public class TransactionalCache public void doPostCommit( SimpleCache sharedCache, Serializable key, - boolean mutable, boolean readOnly); + boolean mutable, boolean allowEqualsCheck, boolean readOnly); } /** @@ -834,41 +874,42 @@ public class TransactionalCache public void doPreCommit( SimpleCache sharedCache, Serializable key, - boolean mutable, boolean readOnly) + boolean mutable, boolean allowEqualsCheck, boolean readOnly) { } public void doPostCommit( SimpleCache sharedCache, Serializable key, - boolean mutable, boolean readOnly) + boolean mutable, boolean allowEqualsCheck, boolean readOnly) { Object sharedObj = sharedCache.get(key); - if (!mutable) + if (sharedObj == null) { - // The value can't change so we can write through on the assumption - // that the value is always correct + // Nothing has changed, write it through sharedCache.put(key, value); } - else if (readOnly) + else if (!mutable) { - // Only add if nothing else has been added in the interim - if (sharedObj == null) - { - sharedCache.put(key, value); - } + // Someone else put the object there + // The assumption is that the value will be correct because the values are immutable + // Don't write it unnecessarily. + } + else if (sharedObj == value) + { + // Someone else put exactly the same value into the cache + // Don't write it unnecessarily. + } + else if (allowEqualsCheck && EqualsHelper.nullSafeEquals(value, sharedObj)) + { + // Someone else added a value but we have validated that it is the same + // as the new one that we where going to add. + // Don't write it unnecessarily. } else { - // Mutable, read-write - if (sharedObj == null) - { - sharedCache.put(key, value); - } - else - { - // Remove new value in the cache - sharedCache.remove(key); - } + // The shared value moved on in a way that was not possible to + // validate. We pessimistically remove the entry. + sharedCache.remove(key); } } } @@ -896,43 +937,52 @@ public class TransactionalCache public void doPreCommit( SimpleCache sharedCache, Serializable key, - boolean mutable, boolean readOnly) + boolean mutable, boolean allowEqualsCheck, boolean readOnly) { } public void doPostCommit( SimpleCache sharedCache, Serializable key, - boolean mutable, boolean readOnly) + boolean mutable, boolean allowEqualsCheck, boolean readOnly) { Object sharedObj = sharedCache.get(key); - if (!mutable) + if (sharedObj == null) { - // Not normally required as mutable objects don't change, - // but we can write it straight through as it should represent - // unchanging values - sharedCache.put(key, value); - } - else if (readOnly) - { - // Only add if value has not changed in the interim - if (sharedObj == originalValue) - { - sharedCache.put(key, value); - } - } - else - { - // Mutable, read-write - if (sharedObj == originalValue) + // Someone removed the value + if (!mutable) { + // We can assume that our value is correct because it's immutable sharedCache.put(key, value); } else { - // The value changed - sharedCache.remove(key); + // The value is mutable, so we must behave pessimistically } } + else if (!mutable) + { + // Someone else has already updated the value. + // This is not normally seen for immutable values. The assumption is that the values + // are equal. + // Don't write it unnecessarily. + } + else if (sharedObj == originalValue) + { + // Nothing has changed, write it through + sharedCache.put(key, value); + } + else if (allowEqualsCheck && EqualsHelper.nullSafeEquals(value, sharedObj)) + { + // Someone else updated the value but we have validated that it is the same + // as the one that we where going to update. + // Don't write it unnecessarily. + } + else + { + // The shared value moved on in a way that was not possible to + // validate. We pessimistically remove the entry. + sharedCache.remove(key); + } } } @@ -956,13 +1006,13 @@ public class TransactionalCache public void doPreCommit( SimpleCache sharedCache, Serializable key, - boolean mutable, boolean readOnly) + boolean mutable, boolean allowEqualsCheck, boolean readOnly) { } public void doPostCommit( SimpleCache sharedCache, Serializable key, - boolean mutable, boolean readOnly) + boolean mutable, boolean allowEqualsCheck, boolean readOnly) { } } @@ -976,6 +1026,7 @@ public class TransactionalCache private boolean isClearOn; private boolean isClosed; private boolean isReadOnly; + private boolean noSharedCacheRead; } /** diff --git a/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java b/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java index 40b5549093..b23d4cd761 100644 --- a/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java +++ b/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java @@ -44,6 +44,7 @@ import org.alfresco.ibatis.RetryingCallbackHelper.RetryingCallback; import org.alfresco.model.ContentModel; import org.alfresco.repo.cache.NullCache; import org.alfresco.repo.cache.SimpleCache; +import org.alfresco.repo.cache.TransactionalCache; import org.alfresco.repo.cache.lookup.EntityLookupCache; import org.alfresco.repo.cache.lookup.EntityLookupCache.EntityLookupCallbackDAOAdaptor; import org.alfresco.repo.domain.contentdata.ContentDataDAO; @@ -152,6 +153,10 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO * VALUE KEY: The Node's NodeRef
                            */ private EntityLookupCache nodesCache; + /** + * Backing transactional cache to allow read-through requests to be honoured + */ + private TransactionalCache nodesTransactionalCache; /** * Cache for the QName values:
                            * KEY: NodeVersionKey
                            @@ -311,7 +316,10 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO cache, CACHE_REGION_NODES, new NodesCacheCallbackDAO()); - + if (cache instanceof TransactionalCache) + { + this.nodesTransactionalCache = (TransactionalCache) cache; + } } /** @@ -629,9 +637,17 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO return txn; } - public Long getCurrentTransactionId() + public Long getCurrentTransactionId(boolean ensureNew) { - TransactionEntity txn = AlfrescoTransactionSupport.getResource(KEY_TRANSACTION); + TransactionEntity txn; + if (ensureNew) + { + txn = getCurrentTransaction(); + } + else + { + txn = AlfrescoTransactionSupport.getResource(KEY_TRANSACTION); + } return txn == null ? null : txn.getId(); } @@ -815,8 +831,8 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO /** * Callback to cache nodes by ID and {@link NodeRef}. When looking up objects based on the - * value key, only the referencing properties need be populated. ONLY live nodes are - * cached. + * value key, only the referencing properties need be populated. ALL nodes are cached, + * not just live nodes. * * @see NodeEntity * @@ -838,7 +854,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO */ public Pair findByKey(Long nodeId) { - NodeEntity node = selectNodeById(nodeId, Boolean.FALSE); + NodeEntity node = selectNodeById(nodeId, null); if (node != null) { // Lock it to prevent 'accidental' modification @@ -867,7 +883,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO public Pair findByValue(Node node) { NodeRef nodeRef = node.getNodeRef(); - node = selectNodeByNodeRef(nodeRef, Boolean.FALSE); + node = selectNodeByNodeRef(nodeRef, null); if (node != null) { // Lock it to prevent 'accidental' modification @@ -897,7 +913,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO @Override public boolean isInCurrentTxn(Long nodeId) { - Long currentTxnId = getCurrentTransactionId(); + Long currentTxnId = getCurrentTransactionId(false); if (currentTxnId == null) { // No transactional changes have been made to any nodes, therefore the node cannot @@ -909,59 +925,19 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO return nodeTxnId.equals(currentTxnId); } - // TODO: Restore to simple version - // TODO: Add read-through option for caches public Status getNodeRefStatus(NodeRef nodeRef) { - Node node = null; - - // Stage 1: check the cache without reading through - Long nodeId = nodesCache.getKey(nodeRef); - if (nodeId != null) - { - node = nodesCache.getValue(nodeId); - // If the node isn't for the current transaction, we are probably reindexing. So invalidate the cache, - // forcing a read through and a repeatable read on this noderef - if (node == null || AlfrescoTransactionSupport.getTransactionReadState() != TxnReadState.TXN_READ_WRITE - || !getCurrentTransaction().getId().equals(node.getTransaction().getId()) - || !node.getNodeRef().equals(nodeRef)) - { - invalidateNodeCaches(nodeId); - node = null; - } - } - - // Stage 2, read through to the database, caching results if appropriate - if (node == null) - { - Node nodeEntity = new NodeEntity(nodeRef); - // Explicitly remove this noderef from the cache, forcing a 'repeatable read' on this noderef from now on. - nodesCache.removeByValue(nodeEntity); - Pair pair = nodesCache.getByValue(nodeEntity); - if (pair == null) - { - // It's not there, so select ignoring the 'deleted' flag - node = selectNodeByNodeRef(nodeRef, null); - if (node != null) - { - // Invalidate anything cached for this node ID, just in case it has moved store, etc. - invalidateNodeCaches(node.getId()); - } - } - else - { - // We have successfully populated the cache - node = pair.getSecond(); - } - } - - if (node == null) + Node node = new NodeEntity(nodeRef); + Pair nodePair = nodesCache.getByValue(node); + // The nodesCache gets both live and deleted nodes. + if (nodePair == null) { return null; } - - Transaction txn = node.getTransaction(); - return new NodeRef.Status(nodeRef, txn.getChangeTxnId(), txn.getId(), node.getDeleted()); + else + { + return nodePair.getSecond().getNodeStatus(); + } } public Pair getNodePair(NodeRef nodeRef) @@ -989,6 +965,8 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO Pair pair = nodesCache.getByKey(nodeId); if (pair == null || pair.getSecond().getDeleted()) { + // Force a removal from the cache + nodesCache.removeByKey(nodeId); // Go back to the database and get what is there NodeEntity dbNode = selectNodeById(nodeId, null); if (pair == null) @@ -3679,6 +3657,16 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO * Bulk caching */ + @Override + public void setCheckNodeConsistency() + { + if (nodesTransactionalCache != null) + { + nodesTransactionalCache.setDisableSharedCacheReadForTransaction(true); + } + } + + @Override public void cacheNodesById(List nodeIds) { /* diff --git a/source/java/org/alfresco/repo/domain/node/NodeDAO.java b/source/java/org/alfresco/repo/domain/node/NodeDAO.java index e198d6d8bf..b7087b39b9 100644 --- a/source/java/org/alfresco/repo/domain/node/NodeDAO.java +++ b/source/java/org/alfresco/repo/domain/node/NodeDAO.java @@ -68,12 +68,13 @@ public interface NodeDAO extends NodeBulkLoader */ /** - * + * @param forceNew true to ensure that a new transaction entry is created + * if the current transaction does not have one. * @return Returns the ID of the current transaction entry or null if * there have not been any modifications to nodes registered in the - * transaction + * transaction and forceNew is false */ - public Long getCurrentTransactionId(); + public Long getCurrentTransactionId(boolean ensureNew); /* * Store diff --git a/source/java/org/alfresco/repo/domain/node/NodeDAOTest.java b/source/java/org/alfresco/repo/domain/node/NodeDAOTest.java index e56ac893c4..19e2a96299 100644 --- a/source/java/org/alfresco/repo/domain/node/NodeDAOTest.java +++ b/source/java/org/alfresco/repo/domain/node/NodeDAOTest.java @@ -65,11 +65,12 @@ public class NodeDAOTest extends TestCase public void testTransaction() throws Throwable { + final boolean[] newTxn = new boolean[] {false}; RetryingTransactionCallback getTxnIdCallback = new RetryingTransactionCallback() { public Long execute() throws Throwable { - return nodeDAO.getCurrentTransactionId(); + return nodeDAO.getCurrentTransactionId(newTxn[0]); } }; // No txn @@ -87,6 +88,10 @@ public class NodeDAOTest extends TestCase // First success Long txnId1 = txnHelper.doInTransaction(getTxnIdCallback); assertNull("No Txn ID should be present in untouched txn", txnId1); + // Second success + newTxn[0] = true; + Long txnId2 = txnHelper.doInTransaction(getTxnIdCallback); + assertNotNull("Txn ID should be present by forcing it", txnId2); } public void testGetNodesWithAspects() throws Throwable diff --git a/source/java/org/alfresco/repo/domain/node/NodeEntity.java b/source/java/org/alfresco/repo/domain/node/NodeEntity.java index 03cc48ce31..3bbfdc9840 100644 --- a/source/java/org/alfresco/repo/domain/node/NodeEntity.java +++ b/source/java/org/alfresco/repo/domain/node/NodeEntity.java @@ -80,6 +80,25 @@ public class NodeEntity implements Node, PermissionCheckValue this.auditableProperties = node.getAuditableProperties(); } + @Override + public int hashCode() + { + final int prime = 31; + int result = 1; + result = prime * result + ((id == null) ? 0 : id.hashCode()); + result = prime * result + ((version == null) ? 0 : version.hashCode()); + return result; + } + + @Override + public boolean equals(Object obj) + { + if (obj == null) return false; + if (!(obj instanceof NodeEntity)) return false; + NodeEntity that = (NodeEntity) obj; + return this.id.equals(that.id) && this.version.equals(that.version); + } + @Override public String toString() { diff --git a/source/java/org/alfresco/repo/domain/node/NodeVersionKey.java b/source/java/org/alfresco/repo/domain/node/NodeVersionKey.java index 1668b17e34..7646910d60 100644 --- a/source/java/org/alfresco/repo/domain/node/NodeVersionKey.java +++ b/source/java/org/alfresco/repo/domain/node/NodeVersionKey.java @@ -63,7 +63,7 @@ public class NodeVersionKey implements Serializable @Override public int hashCode() { - return nodeId.hashCode() + version.hashCode(); + return nodeId.hashCode() + version.hashCode()*37; } @Override diff --git a/source/java/org/alfresco/repo/node/ConcurrentNodeServiceTest.java b/source/java/org/alfresco/repo/node/ConcurrentNodeServiceTest.java index aa8a693952..aeab9db27d 100644 --- a/source/java/org/alfresco/repo/node/ConcurrentNodeServiceTest.java +++ b/source/java/org/alfresco/repo/node/ConcurrentNodeServiceTest.java @@ -29,6 +29,7 @@ import junit.framework.TestCase; import org.alfresco.repo.dictionary.DictionaryDAO; import org.alfresco.repo.dictionary.M2Model; +import org.alfresco.repo.domain.node.NodeDAO; import org.alfresco.repo.security.authentication.AuthenticationComponent; import org.alfresco.repo.security.authentication.AuthenticationUtil; import org.alfresco.repo.transaction.RetryingTransactionHelper; @@ -65,6 +66,7 @@ public class ConcurrentNodeServiceTest extends TestCase static ApplicationContext ctx = ApplicationContextHelper.getApplicationContext(); private NodeService nodeService; + private NodeDAO nodeDAO; private TransactionService transactionService; private NodeRef rootNodeRef; private AuthenticationComponent authenticationComponent; @@ -91,6 +93,7 @@ public class ConcurrentNodeServiceTest extends TestCase ServiceRegistry serviceRegistry = (ServiceRegistry) ctx.getBean(ServiceRegistry.SERVICE_REGISTRY); nodeService = serviceRegistry.getNodeService(); + nodeDAO = (NodeDAO) ctx.getBean("nodeDAO"); transactionService = serviceRegistry.getTransactionService(); authenticationComponent = (AuthenticationComponent) ctx.getBean("authenticationComponent"); @@ -182,6 +185,7 @@ public class ConcurrentNodeServiceTest extends TestCase @Override public Integer execute() throws Throwable { + nodeDAO.setCheckNodeConsistency(); // Grab the current value int current = 0; Object obj = (Object) nodeService.getProperty(rootNodeRef, property); @@ -248,7 +252,7 @@ public class ConcurrentNodeServiceTest extends TestCase { errors.add("\n Prop " + properties[i] + " : " + value); } - if (!value.equals(new Integer(loops))) + else if (!value.equals(new Integer(loops))) { errors.add("\n Prop " + properties[i] + " : " + value); } diff --git a/source/java/org/alfresco/repo/node/NodeBulkLoader.java b/source/java/org/alfresco/repo/node/NodeBulkLoader.java index 2fbe90264f..a63a0f1272 100644 --- a/source/java/org/alfresco/repo/node/NodeBulkLoader.java +++ b/source/java/org/alfresco/repo/node/NodeBulkLoader.java @@ -32,6 +32,12 @@ import org.alfresco.service.cmr.repository.NodeRef; */ public interface NodeBulkLoader { + /** + * Transaction-scope setting to make the Node loader to guarantee the validity of all + * caches: some cache data will be reloaded; some cache data will be considered safe. + */ + public void setCheckNodeConsistency(); + /** * Pre-cache data relevant to the given nodes. There is no need to split the collection * up before calling this method; it is up to the implementations to ensure that batching diff --git a/source/java/org/alfresco/repo/node/db/DbNodeServiceImpl.java b/source/java/org/alfresco/repo/node/db/DbNodeServiceImpl.java index a536ee885a..3e119d98d2 100644 --- a/source/java/org/alfresco/repo/node/db/DbNodeServiceImpl.java +++ b/source/java/org/alfresco/repo/node/db/DbNodeServiceImpl.java @@ -2767,7 +2767,7 @@ public class DbNodeServiceImpl extends AbstractNodeServiceImpl "Touched cm:modified date for node " + parentNodeId + " (" + modifiedDate + ")" + (useCurrentTxn ? " in txn " : " in new txn ") + - nodeDAO.getCurrentTransactionId()); + nodeDAO.getCurrentTransactionId(false)); } } catch (Throwable e) diff --git a/source/java/org/alfresco/repo/search/impl/lucene/ADMLuceneIndexerAndSearcherFactory.java b/source/java/org/alfresco/repo/search/impl/lucene/ADMLuceneIndexerAndSearcherFactory.java index d7ac74c972..cc813b6553 100644 --- a/source/java/org/alfresco/repo/search/impl/lucene/ADMLuceneIndexerAndSearcherFactory.java +++ b/source/java/org/alfresco/repo/search/impl/lucene/ADMLuceneIndexerAndSearcherFactory.java @@ -34,26 +34,18 @@ import org.alfresco.service.transaction.TransactionService; /** * Factory for ADM indxers and searchers * @author andyh - * */ public class ADMLuceneIndexerAndSearcherFactory extends AbstractLuceneIndexerAndSearcherFactory implements SupportsBackgroundIndexing { protected DictionaryService dictionaryService; - private NamespaceService nameSpaceService; - protected NodeService nodeService; - protected FullTextSearchIndexer fullTextSearchIndexer; - protected ContentService contentService; - protected TransactionService transactionService; /** * Set the dictinary service - * - * @param dictionaryService */ public void setDictionaryService(DictionaryService dictionaryService) { @@ -62,7 +54,6 @@ public class ADMLuceneIndexerAndSearcherFactory extends AbstractLuceneIndexerAnd /** * Set the name space service - * @param nameSpaceService */ public void setNameSpaceService(NamespaceService nameSpaceService) { @@ -71,7 +62,6 @@ public class ADMLuceneIndexerAndSearcherFactory extends AbstractLuceneIndexerAnd /** * Set the node service - * @param nodeService */ public void setNodeService(NodeService nodeService) { @@ -85,13 +75,15 @@ public class ADMLuceneIndexerAndSearcherFactory extends AbstractLuceneIndexerAnd /** * Set the content service - * @param contentService */ public void setContentService(ContentService contentService) { this.contentService = contentService; } + /** + * Set the transaction service + */ public void setTransactionService(TransactionService transactionService) { this.transactionService = transactionService; @@ -103,6 +95,7 @@ public class ADMLuceneIndexerAndSearcherFactory extends AbstractLuceneIndexerAnd ADMLuceneIndexerImpl indexer = ADMLuceneIndexerImpl.getUpdateIndexer(storeRef, deltaId, this); indexer.setNodeService(nodeService); + indexer.setBulkLoader(getBulkLoader()); indexer.setTenantService(tenantService); indexer.setDictionaryService(dictionaryService); // indexer.setLuceneIndexLock(luceneIndexLock); diff --git a/source/java/org/alfresco/repo/search/impl/lucene/ADMLuceneIndexerImpl.java b/source/java/org/alfresco/repo/search/impl/lucene/ADMLuceneIndexerImpl.java index 77fd3372ad..cf26e509a4 100644 --- a/source/java/org/alfresco/repo/search/impl/lucene/ADMLuceneIndexerImpl.java +++ b/source/java/org/alfresco/repo/search/impl/lucene/ADMLuceneIndexerImpl.java @@ -642,7 +642,7 @@ public class ADMLuceneIndexerImpl extends AbstractLuceneIndexerImpl imp final IndexReader mainReader) { final NodeRef nodeRef = new NodeRef(stringNodeRef); - final NodeRef.Status nodeStatus = nodeService.getNodeStatus(nodeRef); // DH: Let me know if this field gets dropped (performance) + final NodeRef.Status nodeStatus = nodeService.getNodeStatus(nodeRef); final List docs = new LinkedList(); if (nodeStatus == null) { diff --git a/source/java/org/alfresco/repo/search/impl/lucene/ADMLuceneUnIndexedIndexAndSearcherFactory.java b/source/java/org/alfresco/repo/search/impl/lucene/ADMLuceneUnIndexedIndexAndSearcherFactory.java index 9d81fe9601..0432c71f0c 100644 --- a/source/java/org/alfresco/repo/search/impl/lucene/ADMLuceneUnIndexedIndexAndSearcherFactory.java +++ b/source/java/org/alfresco/repo/search/impl/lucene/ADMLuceneUnIndexedIndexAndSearcherFactory.java @@ -28,6 +28,7 @@ public class ADMLuceneUnIndexedIndexAndSearcherFactory extends ADMLuceneIndexerA { ADMLuceneNoActionIndexerImpl indexer = ADMLuceneIndexerImpl.getNoActionIndexer(storeRef, deltaId, this); indexer.setNodeService(nodeService); + indexer.setBulkLoader(getBulkLoader()); indexer.setTenantService(tenantService); indexer.setDictionaryService(dictionaryService); // indexer.setLuceneIndexLock(luceneIndexLock); diff --git a/source/java/org/alfresco/repo/search/impl/lucene/AbstractLuceneIndexerImpl.java b/source/java/org/alfresco/repo/search/impl/lucene/AbstractLuceneIndexerImpl.java index 4ca27b46ba..61139ed7a4 100644 --- a/source/java/org/alfresco/repo/search/impl/lucene/AbstractLuceneIndexerImpl.java +++ b/source/java/org/alfresco/repo/search/impl/lucene/AbstractLuceneIndexerImpl.java @@ -30,6 +30,7 @@ import java.util.Set; import javax.transaction.Status; import javax.transaction.xa.XAResource; +import org.alfresco.repo.node.NodeBulkLoader; import org.alfresco.repo.search.Indexer; import org.alfresco.repo.search.IndexerException; import org.alfresco.repo.search.impl.lucene.index.TransactionStatus; @@ -102,6 +103,7 @@ public abstract class AbstractLuceneIndexerImpl extends AbstractLuceneBase im private boolean isReadThrough; protected TransactionService transactionService; + protected NodeBulkLoader bulkLoader; public void setReadThrough(boolean isReadThrough) { @@ -113,6 +115,14 @@ public abstract class AbstractLuceneIndexerImpl extends AbstractLuceneBase im this.transactionService = transactionService; } + /** + * @param bulkLoader object to provide node loading options + */ + public void setBulkLoader(NodeBulkLoader bulkLoader) + { + this.bulkLoader = bulkLoader; + } + protected static class Command { S ref; @@ -695,6 +705,11 @@ public abstract class AbstractLuceneIndexerImpl extends AbstractLuceneBase im @Override public T2 execute() throws Throwable { + // Request clean node data + if (bulkLoader != null) + { + bulkLoader.setCheckNodeConsistency(); + } try { return callback.execute(); diff --git a/source/java/org/alfresco/repo/solr/SOLRTrackingComponent.java b/source/java/org/alfresco/repo/solr/SOLRTrackingComponent.java index cc418f383a..27114936f5 100644 --- a/source/java/org/alfresco/repo/solr/SOLRTrackingComponent.java +++ b/source/java/org/alfresco/repo/solr/SOLRTrackingComponent.java @@ -20,7 +20,6 @@ package org.alfresco.repo.solr; import java.util.List; import java.util.Map; -import java.util.Set; import org.alfresco.repo.domain.node.Node; import org.alfresco.service.namespace.QName; diff --git a/source/java/org/alfresco/repo/solr/SOLRTrackingComponentImpl.java b/source/java/org/alfresco/repo/solr/SOLRTrackingComponentImpl.java index 9ff793fcf4..88e583ad2d 100644 --- a/source/java/org/alfresco/repo/solr/SOLRTrackingComponentImpl.java +++ b/source/java/org/alfresco/repo/solr/SOLRTrackingComponentImpl.java @@ -510,6 +510,9 @@ public class SOLRTrackingComponentImpl implements SOLRTrackingComponent return; } + // Ensure that we get fresh node references + nodeDAO.setCheckNodeConsistency(); + NodeMetaDataQueryRowHandler rowHandler = new NodeMetaDataQueryRowHandler(callback); boolean includeType = (resultFilter == null ? true : resultFilter.getIncludeType()); boolean includeProperties = (resultFilter == null ? true : resultFilter.getIncludeProperties()); @@ -868,9 +871,6 @@ public class SOLRTrackingComponentImpl implements SOLRTrackingComponent } } - /* (non-Javadoc) - * @see org.alfresco.repo.solr.SOLRTrackingComponent#getLastTransactionTimestamp() - */ @Override public Long getMaxTxnCommitTime() { diff --git a/source/java/org/alfresco/repo/solr/SOLRTrackingComponentTest.java b/source/java/org/alfresco/repo/solr/SOLRTrackingComponentTest.java index 39c8294f75..3559e0aeb6 100644 --- a/source/java/org/alfresco/repo/solr/SOLRTrackingComponentTest.java +++ b/source/java/org/alfresco/repo/solr/SOLRTrackingComponentTest.java @@ -576,12 +576,20 @@ public class SOLRTrackingComponentTest extends TestCase logger.debug("Got " + bt.getActualNodeCount() + " nodes in " + (endTime - startTime) + " ms"); } - private void getNodeMetaData(NodeMetaDataParameters params, MetaDataResultsFilter filter, SOLRTest bt) + private void getNodeMetaData(final NodeMetaDataParameters params, final MetaDataResultsFilter filter, final SOLRTest bt) { bt.clearNodesMetaData(); long startTime = System.currentTimeMillis(); - solrTrackingComponent.getNodesMetadata(params, filter, bt); + txnHelper.doInTransaction(new RetryingTransactionCallback() + { + @Override + public Void execute() throws Throwable + { + solrTrackingComponent.getNodesMetadata(params, filter, bt); + return null; + } + }, true, true); long endTime = System.currentTimeMillis(); bt.runNodeMetaDataChecks(params.getMaxResults());