From 834be2837a81ad9df93906cc8f3f3e0f5de92379 Mon Sep 17 00:00:00 2001 From: Brian Stansberry Date: Wed, 27 Feb 2008 17:47:36 +0000 Subject: [PATCH] Redo handling of invalidated region root nodes git-svn-id: https://svn.jboss.org/repos/hibernate/core/trunk@14368 1b8cb986-b30d-0410-93ca-fae66ebed9b2 --- .../cache/jbc2/BasicRegionAdapter.java | 136 +++++++++++------- ...OptimisticTransactionalAccessDelegate.java | 3 - .../access/TransactionalAccessDelegate.java | 20 ++- .../jbc2/query/QueryResultsRegionImpl.java | 2 - ...usteredConcurrentTimestampsRegionImpl.java | 2 - .../jbc2/timestamp/TimestampsRegionImpl.java | 2 - .../AbstractGeneralDataRegionTestCase.java | 57 ++++---- ...ollectionRegionAccessStrategyTestCase.java | 77 +++++----- ...actEntityRegionAccessStrategyTestCase.java | 78 +++++----- 9 files changed, 210 insertions(+), 167 deletions(-) diff --git a/cache-jbosscache2/src/main/java/org/hibernate/cache/jbc2/BasicRegionAdapter.java b/cache-jbosscache2/src/main/java/org/hibernate/cache/jbc2/BasicRegionAdapter.java index 2b67811336..842f085f67 100644 --- a/cache-jbosscache2/src/main/java/org/hibernate/cache/jbc2/BasicRegionAdapter.java +++ b/cache-jbosscache2/src/main/java/org/hibernate/cache/jbc2/BasicRegionAdapter.java @@ -31,6 +31,10 @@ import javax.transaction.SystemException; import javax.transaction.Transaction; import javax.transaction.TransactionManager; +import org.hibernate.cache.CacheException; +import org.hibernate.cache.Region; +import org.hibernate.cache.jbc2.util.CacheHelper; +import org.hibernate.cache.jbc2.util.NonLockingDataVersion; import org.jboss.cache.Cache; import org.jboss.cache.Fqn; import org.jboss.cache.Node; @@ -39,20 +43,10 @@ import org.jboss.cache.config.Configuration; import org.jboss.cache.config.Option; import org.jboss.cache.config.Configuration.NodeLockingScheme; import org.jboss.cache.notifications.annotation.CacheListener; -import org.jboss.cache.notifications.annotation.NodeCreated; -import org.jboss.cache.notifications.annotation.NodeRemoved; -import org.jboss.cache.notifications.event.NodeCreatedEvent; -import org.jboss.cache.notifications.event.NodeRemovedEvent; import org.jboss.cache.optimistic.DataVersion; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.hibernate.cache.CacheException; -import org.hibernate.cache.Region; -import org.hibernate.cache.jbc2.builder.JndiMultiplexingCacheInstanceManager; -import org.hibernate.cache.jbc2.util.CacheHelper; -import org.hibernate.cache.jbc2.util.NonLockingDataVersion; - /** * General support for writing {@link Region} implementations for JBoss Cache * 2.x. @@ -74,7 +68,7 @@ public abstract class BasicRegionAdapter implements Region { protected final Logger log; protected final Object regionRootMutex = new Object(); - protected RegionRootListener listener; +// protected RegionRootListener listener; public BasicRegionAdapter(Cache jbcCache, String regionName, String regionPrefix) { this.jbcCache = jbcCache; @@ -103,17 +97,31 @@ public abstract class BasicRegionAdapter implements Region { } } - // If we are using replication, we may remove the root node - // and then need to re-add it. In that case, the fact - // that it is resident will not replicate, so use a listener - // to set it as resident - if (CacheHelper.isClusteredReplication(cfg.getCacheMode()) - || CacheHelper.isClusteredInvalidation(cfg.getCacheMode())) { - listener = new RegionRootListener(); - jbcCache.addCacheListener(listener); - } +// // If we are using replication, we may remove the root node +// // and then need to re-add it. In that case, the fact +// // that it is resident will not replicate, so use a listener +// // to set it as resident +// if (CacheHelper.isClusteredReplication(cfg.getCacheMode()) +// || CacheHelper.isClusteredInvalidation(cfg.getCacheMode())) { +// listener = new RegionRootListener(); +// jbcCache.addCacheListener(listener); +// } - establishRegionRootNode(); + regionRoot = jbcCache.getRoot().getChild( regionFqn ); + if (regionRoot == null || !regionRoot.isValid()) { + // Establish the region root node with a non-locking data version + DataVersion version = optimistic ? NonLockingDataVersion.INSTANCE : null; + regionRoot = CacheHelper.addNode(jbcCache, regionFqn, true, true, version); + } + else if (optimistic && regionRoot instanceof NodeSPI) { + // FIXME Hacky workaround to JBCACHE-1202 + if ( !( ( ( NodeSPI ) regionRoot ).getVersion() instanceof NonLockingDataVersion ) ) { + ((NodeSPI) regionRoot).setVersion(NonLockingDataVersion.INSTANCE); + } + } + if (!regionRoot.isResident()) { + regionRoot.setResident(true); + } } catch (Exception e) { throw new CacheException(e.getMessage(), e); @@ -123,30 +131,48 @@ public abstract class BasicRegionAdapter implements Region { private void establishRegionRootNode() { synchronized (regionRootMutex) { - if (regionRoot != null && regionRoot.isValid()) + // If we've been blocking for the mutex, perhaps another + // thread has already reestablished the root. + // In case the node was reestablised via replication, confirm it's + // marked "resident" (a status which doesn't replicate) + if (regionRoot != null && regionRoot.isValid()) { return; + } + + // For pessimistic locking, we just want to toss out our ref + // to any old invalid root node and get the latest (may be null) + if (!optimistic) { + regionRoot = jbcCache.getRoot().getChild( regionFqn ); + return; + } + + // The rest only matters for optimistic locking, where we + // need to establish the proper data version on the region root + // Don't hold a transactional lock for this Transaction tx = suspend(); + Node newRoot = null; try { // Make sure the root node for the region exists and // has a DataVersion that never complains - regionRoot = jbcCache.getRoot().getChild( regionFqn ); - if (regionRoot == null) { + newRoot = jbcCache.getRoot().getChild( regionFqn ); + if (newRoot == null || !newRoot.isValid()) { // Establish the region root node with a non-locking data version DataVersion version = optimistic ? NonLockingDataVersion.INSTANCE : null; - regionRoot = CacheHelper.addNode(jbcCache, regionFqn, true, true, version); + newRoot = CacheHelper.addNode(jbcCache, regionFqn, true, true, version); } - else if (optimistic && regionRoot instanceof NodeSPI) { + else if (newRoot instanceof NodeSPI) { // FIXME Hacky workaround to JBCACHE-1202 - if ( !( ( ( NodeSPI ) regionRoot ).getVersion() instanceof NonLockingDataVersion ) ) { - ((NodeSPI) regionRoot).setVersion(NonLockingDataVersion.INSTANCE); + if ( !( ( ( NodeSPI ) newRoot ).getVersion() instanceof NonLockingDataVersion ) ) { + ((NodeSPI) newRoot).setVersion(NonLockingDataVersion.INSTANCE); } } - // Never evict this node - regionRoot.setResident(true); + // Never evict this node + newRoot.setResident(true); } finally { resume(tx); + regionRoot = newRoot; } } } @@ -164,20 +190,22 @@ public abstract class BasicRegionAdapter implements Region { } /** - * If the cache is configured for optimistic locking, checks for the - * validity of the root cache node for this region, - * creating a new one if it does not exist or is invalid. Suspends any + * Checks for the validity of the root cache node for this region, + * creating a new one if it does not exist or is invalid, and also + * ensuring that the root node is marked as resident. Suspends any * transaction while doing this to ensure no transactional locks are held * on the region root. * - * This is only needed for optimistic locking, as with optimistic the - * region root node has a special version that must be established. - * * TODO remove this once JBCACHE-1250 is resolved. */ public void ensureRegionRootExists() { - if (optimistic && (regionRoot == null || !regionRoot.isValid())) + + if (regionRoot == null || !regionRoot.isValid()) establishRegionRootNode(); + + // Fix up the resident flag + if (regionRoot != null && regionRoot.isValid() && !regionRoot.isResident()) + regionRoot.setResident(true); } public void destroy() throws CacheException { @@ -202,10 +230,10 @@ public abstract class BasicRegionAdapter implements Region { } catch (Exception e) { throw new CacheException(e); } - finally { - if (listener != null) - jbcCache.removeCacheListener(listener); - } +// finally { +// if (listener != null) +// jbcCache.removeCacheListener(listener); +// } } protected void deactivateLocalNode() { @@ -364,17 +392,17 @@ public abstract class BasicRegionAdapter implements Region { return escaped; } - @CacheListener - public class RegionRootListener { - - @NodeCreated - public void nodeCreated(NodeCreatedEvent event) { - if (!event.isPre() && event.getFqn().equals(getRegionFqn())) { - log.debug("Node created for " + getRegionFqn()); - Node regionRoot = jbcCache.getRoot().getChild(getRegionFqn()); - regionRoot.setResident(true); - } - } - - } +// @CacheListener +// public class RegionRootListener { +// +// @NodeCreated +// public void nodeCreated(NodeCreatedEvent event) { +// if (!event.isPre() && event.getFqn().equals(getRegionFqn())) { +// log.debug("Node created for " + getRegionFqn()); +// Node regionRoot = jbcCache.getRoot().getChild(getRegionFqn()); +// regionRoot.setResident(true); +// } +// } +// +// } } diff --git a/cache-jbosscache2/src/main/java/org/hibernate/cache/jbc2/access/OptimisticTransactionalAccessDelegate.java b/cache-jbosscache2/src/main/java/org/hibernate/cache/jbc2/access/OptimisticTransactionalAccessDelegate.java index a9b7c28371..76d0d3e053 100755 --- a/cache-jbosscache2/src/main/java/org/hibernate/cache/jbc2/access/OptimisticTransactionalAccessDelegate.java +++ b/cache-jbosscache2/src/main/java/org/hibernate/cache/jbc2/access/OptimisticTransactionalAccessDelegate.java @@ -169,9 +169,6 @@ public class OptimisticTransactionalAccessDelegate extends TransactionalAccessDe Option opt = NonLockingDataVersion.getInvocationOption(); CacheHelper.removeAll(cache, regionFqn, opt); - - // Restablish the region root node with a non-locking data version - CacheHelper.addNode(cache, regionFqn, false, true, NonLockingDataVersion.INSTANCE); } } diff --git a/cache-jbosscache2/src/main/java/org/hibernate/cache/jbc2/access/TransactionalAccessDelegate.java b/cache-jbosscache2/src/main/java/org/hibernate/cache/jbc2/access/TransactionalAccessDelegate.java index 753d2fef13..61e9e317ed 100755 --- a/cache-jbosscache2/src/main/java/org/hibernate/cache/jbc2/access/TransactionalAccessDelegate.java +++ b/cache-jbosscache2/src/main/java/org/hibernate/cache/jbc2/access/TransactionalAccessDelegate.java @@ -56,16 +56,23 @@ public class TransactionalAccessDelegate { } public Object get(Object key, long txTimestamp) throws CacheException { + + region.ensureRegionRootExists(); + return CacheHelper.get(cache, regionFqn, key); } public boolean putFromLoad(Object key, Object value, long txTimestamp, Object version) throws CacheException { + + region.ensureRegionRootExists(); return CacheHelper.putForExternalRead(cache, regionFqn, key, value); } public boolean putFromLoad(Object key, Object value, long txTimestamp, Object version, boolean minimalPutOverride) throws CacheException { + + region.ensureRegionRootExists(); // We ignore minimalPutOverride. JBossCache putForExternalRead is // already about as minimal as we can get; it will promptly return @@ -88,6 +95,8 @@ public class TransactionalAccessDelegate { } public boolean insert(Object key, Object value, Object version) throws CacheException { + + region.ensureRegionRootExists(); CacheHelper.put(cache, regionFqn, key, value); return true; @@ -99,6 +108,8 @@ public class TransactionalAccessDelegate { public boolean update(Object key, Object value, Object currentVersion, Object previousVersion) throws CacheException { + + region.ensureRegionRootExists(); CacheHelper.put(cache, regionFqn, key, value); return true; @@ -110,6 +121,8 @@ public class TransactionalAccessDelegate { } public void remove(Object key) throws CacheException { + + region.ensureRegionRootExists(); CacheHelper.remove(cache, regionFqn, key); } @@ -119,6 +132,9 @@ public class TransactionalAccessDelegate { } public void evict(Object key) throws CacheException { + + region.ensureRegionRootExists(); + CacheHelper.remove(cache, regionFqn, key); } @@ -127,8 +143,6 @@ public class TransactionalAccessDelegate { } private void evictOrRemoveAll() throws CacheException { - CacheHelper.removeAll(cache, regionFqn); - // Restore the region root node - CacheHelper.addNode(cache, regionFqn, false, true, null); + CacheHelper.removeAll(cache, regionFqn); } } diff --git a/cache-jbosscache2/src/main/java/org/hibernate/cache/jbc2/query/QueryResultsRegionImpl.java b/cache-jbosscache2/src/main/java/org/hibernate/cache/jbc2/query/QueryResultsRegionImpl.java index fd9f9291ee..e69e89f0af 100644 --- a/cache-jbosscache2/src/main/java/org/hibernate/cache/jbc2/query/QueryResultsRegionImpl.java +++ b/cache-jbosscache2/src/main/java/org/hibernate/cache/jbc2/query/QueryResultsRegionImpl.java @@ -89,8 +89,6 @@ public class QueryResultsRegionImpl extends TransactionalDataRegionAdapter imple if (localOnly) opt.setCacheModeLocal(true); CacheHelper.removeAll(getCacheInstance(), getRegionFqn(), opt); - // Restore the region root node - CacheHelper.addNode(getCacheInstance(), getRegionFqn(), false, true, null); } public Object get(Object key) throws CacheException { diff --git a/cache-jbosscache2/src/main/java/org/hibernate/cache/jbc2/timestamp/ClusteredConcurrentTimestampsRegionImpl.java b/cache-jbosscache2/src/main/java/org/hibernate/cache/jbc2/timestamp/ClusteredConcurrentTimestampsRegionImpl.java index a7e613c175..ef552f4133 100644 --- a/cache-jbosscache2/src/main/java/org/hibernate/cache/jbc2/timestamp/ClusteredConcurrentTimestampsRegionImpl.java +++ b/cache-jbosscache2/src/main/java/org/hibernate/cache/jbc2/timestamp/ClusteredConcurrentTimestampsRegionImpl.java @@ -93,8 +93,6 @@ public class ClusteredConcurrentTimestampsRegionImpl extends TransactionalDataRe public void evictAll() throws CacheException { Option opt = getNonLockingDataVersionOption(true); CacheHelper.removeAll(getCacheInstance(), getRegionFqn(), opt); - // Restore the region root node - CacheHelper.addNode(getCacheInstance(), getRegionFqn(), false, true, null); } public Object get(Object key) throws CacheException { diff --git a/cache-jbosscache2/src/main/java/org/hibernate/cache/jbc2/timestamp/TimestampsRegionImpl.java b/cache-jbosscache2/src/main/java/org/hibernate/cache/jbc2/timestamp/TimestampsRegionImpl.java index d4d017020d..30aa10638c 100644 --- a/cache-jbosscache2/src/main/java/org/hibernate/cache/jbc2/timestamp/TimestampsRegionImpl.java +++ b/cache-jbosscache2/src/main/java/org/hibernate/cache/jbc2/timestamp/TimestampsRegionImpl.java @@ -97,8 +97,6 @@ public class TimestampsRegionImpl extends TransactionalDataRegionAdapter impleme // TODO Is this a valid operation on a timestamps cache? Option opt = getNonLockingDataVersionOption(true); CacheHelper.removeAll(getCacheInstance(), getRegionFqn(), opt); - // Restore the region root node - CacheHelper.addNode(getCacheInstance(), getRegionFqn(), false, true, null); } public Object get(Object key) throws CacheException { diff --git a/cache-jbosscache2/src/test/java/org/hibernate/test/cache/jbc2/AbstractGeneralDataRegionTestCase.java b/cache-jbosscache2/src/test/java/org/hibernate/test/cache/jbc2/AbstractGeneralDataRegionTestCase.java index e8a70b341c..5b10410a60 100644 --- a/cache-jbosscache2/src/test/java/org/hibernate/test/cache/jbc2/AbstractGeneralDataRegionTestCase.java +++ b/cache-jbosscache2/src/test/java/org/hibernate/test/cache/jbc2/AbstractGeneralDataRegionTestCase.java @@ -212,42 +212,35 @@ public abstract class AbstractGeneralDataRegionTestCase extends AbstractRegionIm localRegion.evictAll(); + // This should re-establish the region root node in the optimistic case + assertNull(localRegion.get(KEY)); + regionRoot = localCache.getRoot().getChild(regionFqn); - assertFalse(regionRoot == null); - assertEquals(0, regionRoot.getChildrenNames().size()); - assertTrue(regionRoot.isResident()); - - if (CacheHelper.isClusteredInvalidation(remoteCache)) { - // With invalidation, a node that removes the region root cannot reestablish - // it on remote nodes, since the only message the propagates is "invalidate". - // So, we have to reestablish it ourselves - - // First, do a get to help test whether a get messes up the optimistic version - String msg = "Known issue JBCACHE-1251 -- problem reestablishing invalidated region root"; - try { - assertEquals(null, remoteRegion.get(KEY)); - } - catch (CacheException ce) { - log.error(msg, ce); - fail(msg + " -- cause: " + ce); - } - remoteRegion.put(KEY, VALUE1); - assertEquals(msg, VALUE1, remoteRegion.get(KEY)); - } - - regionRoot = remoteCache.getRoot().getChild(regionFqn); - assertFalse(regionRoot == null); - if (invalidation) { - // JBC seems broken: see http://www.jboss.com/index.html?module=bb&op=viewtopic&t=121408 - // FIXME replace with the following when JBCACHE-1199 and JBCACHE-1200 are done: - //assertFalse(regionRoot.isValid()); - checkNodeIsEmpty(regionRoot); + if (optimistic) { + assertFalse(regionRoot == null); + assertEquals(0, regionRoot.getChildrenNames().size()); + assertTrue(regionRoot.isValid()); + assertTrue(regionRoot.isResident()); } else { - // Same assertion, just different assertion msg + assertTrue("region root is removed", regionRoot == null || !regionRoot.isValid()); + } + + // Re-establishing the region root on the local node doesn't + // propagate it to other nodes. Do a get on the remote node to re-establish + // This only adds a node in the case of optimistic locking + assertEquals(null, remoteRegion.get(KEY)); + + regionRoot = remoteCache.getRoot().getChild(regionFqn); + if (optimistic) { + assertFalse(regionRoot == null); assertEquals(0, regionRoot.getChildrenNames().size()); - } - assertTrue(regionRoot.isResident()); + assertTrue(regionRoot.isValid()); + assertTrue(regionRoot.isResident()); + } + else { + assertTrue("region root is removed", regionRoot == null || !regionRoot.isValid()); + } assertEquals("local is clean", null, localRegion.get(KEY)); assertEquals("remote is clean", null, remoteRegion.get(KEY)); diff --git a/cache-jbosscache2/src/test/java/org/hibernate/test/cache/jbc2/collection/AbstractCollectionRegionAccessStrategyTestCase.java b/cache-jbosscache2/src/test/java/org/hibernate/test/cache/jbc2/collection/AbstractCollectionRegionAccessStrategyTestCase.java index c03d05c1f2..8b3c987422 100644 --- a/cache-jbosscache2/src/test/java/org/hibernate/test/cache/jbc2/collection/AbstractCollectionRegionAccessStrategyTestCase.java +++ b/cache-jbosscache2/src/test/java/org/hibernate/test/cache/jbc2/collection/AbstractCollectionRegionAccessStrategyTestCase.java @@ -460,8 +460,6 @@ public abstract class AbstractCollectionRegionAccessStrategyTestCase extends Abs // Wait for async propagation sleep(250); - - if (isUsingOptimisticLocking()) { regionRoot = localCache.getRoot().getChild(regionFqn); assertEquals(NonLockingDataVersion.class, ((NodeSPI) regionRoot).getVersion().getClass()); @@ -474,43 +472,54 @@ public abstract class AbstractCollectionRegionAccessStrategyTestCase extends Abs else localAccessStrategy.removeAll(); + // This should re-establish the region root node in the optimistic case + assertNull(localAccessStrategy.get(KEY, System.currentTimeMillis())); + regionRoot = localCache.getRoot().getChild(regionFqn); - assertFalse(regionRoot == null); - assertEquals(0, getValidChildrenCount(regionRoot)); - assertTrue(regionRoot.isResident()); - - if (isUsingInvalidation()) { - // With invalidation, a node that removes the region root cannot reestablish - // it on remote nodes, since the only message the propagates is "invalidate". - // So, we have to reestablish it ourselves - - // First, do a get to help test whether a get messes up the optimistic version - String msg = "Known issue JBCACHE-1251 -- problem reestablishing invalidated region root"; - try { - assertEquals(null, remoteAccessStrategy.get(KEY, System.currentTimeMillis())); - } - catch (CacheException ce) { - log.error(msg, ce); - fail(msg + " -- cause: " + ce); - } - remoteAccessStrategy.putFromLoad(KEY, VALUE1, System.currentTimeMillis(), new Integer(1)); - assertEquals(msg, VALUE1, remoteAccessStrategy.get(KEY, System.currentTimeMillis())); - } - - regionRoot = remoteCache.getRoot().getChild(regionFqn); - assertFalse(regionRoot == null); - if (isUsingInvalidation()) { - // Region root should have 1 child -- the one we added above - assertEquals(1, getValidChildrenCount(regionRoot)); + if (isUsingOptimisticLocking()) { + assertFalse(regionRoot == null); + assertEquals(0, getValidChildrenCount(regionRoot)); + assertTrue(regionRoot.isValid()); + assertTrue(regionRoot.isResident()); } else { - // Same assertion, just different assertion msg - assertEquals(0, getValidChildrenCount(regionRoot)); + assertTrue("region root is removed", regionRoot == null || !regionRoot.isValid()); + } + + // Re-establishing the region root on the local node doesn't + // propagate it to other nodes. Do a get on the remote node to re-establish + // This only adds a node in the case of optimistic locking + assertEquals(null, remoteAccessStrategy.get(KEY, System.currentTimeMillis())); + + regionRoot = remoteCache.getRoot().getChild(regionFqn); + if (isUsingOptimisticLocking()) { + assertFalse(regionRoot == null); + assertTrue(regionRoot.isValid()); + assertTrue(regionRoot.isResident()); + // Not invalidation, so we didn't insert a child above + assertEquals(0, getValidChildrenCount(regionRoot)); } - assertTrue(regionRoot.isResident()); + else { + assertTrue("region root is removed", regionRoot == null || !regionRoot.isValid()); + } - assertNull("local is clean", localAccessStrategy.get(KEY, System.currentTimeMillis())); - assertEquals("remote is correct", (isUsingInvalidation() ? VALUE1 : null), remoteAccessStrategy.get(KEY, System.currentTimeMillis())); + // Test whether the get above messes up the optimistic version + remoteAccessStrategy.putFromLoad(KEY, VALUE1, System.currentTimeMillis(), new Integer(1)); + assertEquals(VALUE1, remoteAccessStrategy.get(KEY, System.currentTimeMillis())); + + // Revalidate the region root + regionRoot = remoteCache.getRoot().getChild(regionFqn); + assertFalse(regionRoot == null); + assertTrue(regionRoot.isValid()); + assertTrue(regionRoot.isResident()); + // Region root should have 1 child -- the one we added above + assertEquals(1, getValidChildrenCount(regionRoot)); + + // Wait for async propagation of the putFromLoad + sleep(250); + + assertEquals("local is correct", (isUsingInvalidation() ? null : VALUE1), localAccessStrategy.get(KEY, System.currentTimeMillis())); + assertEquals("remote is correct", VALUE1, remoteAccessStrategy.get(KEY, System.currentTimeMillis())); } private int getValidChildrenCount(Node node) { diff --git a/cache-jbosscache2/src/test/java/org/hibernate/test/cache/jbc2/entity/AbstractEntityRegionAccessStrategyTestCase.java b/cache-jbosscache2/src/test/java/org/hibernate/test/cache/jbc2/entity/AbstractEntityRegionAccessStrategyTestCase.java index b3302b66c4..43b671de6e 100644 --- a/cache-jbosscache2/src/test/java/org/hibernate/test/cache/jbc2/entity/AbstractEntityRegionAccessStrategyTestCase.java +++ b/cache-jbosscache2/src/test/java/org/hibernate/test/cache/jbc2/entity/AbstractEntityRegionAccessStrategyTestCase.java @@ -24,7 +24,6 @@ package org.hibernate.test.cache.jbc2.entity; import java.util.Iterator; -import java.util.Set; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -34,7 +33,6 @@ import junit.framework.Test; import junit.framework.TestSuite; import org.hibernate.cache.CacheDataDescription; -import org.hibernate.cache.CacheException; import org.hibernate.cache.EntityRegion; import org.hibernate.cache.access.AccessType; import org.hibernate.cache.access.EntityRegionAccessStrategy; @@ -684,43 +682,53 @@ public abstract class AbstractEntityRegionAccessStrategyTestCase extends Abstrac else localAccessStrategy.removeAll(); - regionRoot = localCache.getRoot().getChild(regionFqn); - assertFalse(regionRoot == null); - assertEquals(0, getValidChildrenCount(regionRoot)); - assertTrue(regionRoot.isResident()); - - if (isUsingInvalidation()) { - // With invalidation, a node that removes the region root cannot reestablish - // it on remote nodes, since the only message the propagates is "invalidate". - // So, we have to reestablish it ourselves - - // First, do a get to help test whether a get messes up the optimistic version - String msg = "Known issue JBCACHE-1251 -- problem reestablishing invalidated region root"; - try { - assertEquals(null, remoteAccessStrategy.get(KEY, System.currentTimeMillis())); - } - catch (CacheException ce) { - log.error(msg, ce); - fail(msg + " -- cause: " + ce); - } - remoteAccessStrategy.putFromLoad(KEY, VALUE1, System.currentTimeMillis(), new Integer(1)); - assertEquals(msg, VALUE1, remoteAccessStrategy.get(KEY, System.currentTimeMillis())); - } + // This should re-establish the region root node in the optimistic case + assertNull(localAccessStrategy.get(KEY, System.currentTimeMillis())); - regionRoot = remoteCache.getRoot().getChild(regionFqn); - assertFalse(regionRoot == null); - if (isUsingInvalidation()) { - // Region root should have 1 child -- the one we added above - assertEquals(1, getValidChildrenCount(regionRoot)); + regionRoot = localCache.getRoot().getChild(regionFqn); + if (isUsingOptimisticLocking()) { + assertFalse(regionRoot == null); + assertEquals(0, getValidChildrenCount(regionRoot)); + assertTrue(regionRoot.isValid()); + assertTrue(regionRoot.isResident()); } else { - // Same assertion, just different assertion msg - assertEquals(0, getValidChildrenCount(regionRoot)); - } - assertTrue(regionRoot.isResident()); + assertTrue("region root is removed", regionRoot == null || !regionRoot.isValid()); + } + + // Re-establishing the region root on the local node doesn't + // propagate it to other nodes. Do a get on the remote node to re-establish + assertEquals(null, remoteAccessStrategy.get(KEY, System.currentTimeMillis())); + + regionRoot = remoteCache.getRoot().getChild(regionFqn); + if (isUsingOptimisticLocking()) { + assertFalse(regionRoot == null); + assertTrue(regionRoot.isValid()); + assertTrue(regionRoot.isResident()); + // Not invalidation, so we didn't insert a child above + assertEquals(0, getValidChildrenCount(regionRoot)); + } + else { + assertTrue("region root is removed", regionRoot == null || !regionRoot.isValid()); + } - assertNull("local is clean", localAccessStrategy.get(KEY, System.currentTimeMillis())); - assertEquals("remote is correct", (isUsingInvalidation() ? VALUE1 : null), remoteAccessStrategy.get(KEY, System.currentTimeMillis())); + // Test whether the get above messes up the optimistic version + remoteAccessStrategy.putFromLoad(KEY, VALUE1, System.currentTimeMillis(), new Integer(1)); + assertEquals(VALUE1, remoteAccessStrategy.get(KEY, System.currentTimeMillis())); + + // Revalidate the region root + regionRoot = remoteCache.getRoot().getChild(regionFqn); + assertFalse(regionRoot == null); + assertTrue(regionRoot.isValid()); + assertTrue(regionRoot.isResident()); + // Region root should have 1 child -- the one we added above + assertEquals(1, getValidChildrenCount(regionRoot)); + + // Wait for async propagation + sleep(250); + + assertEquals("local is correct", (isUsingInvalidation() ? null : VALUE1), localAccessStrategy.get(KEY, System.currentTimeMillis())); + assertEquals("remote is correct", VALUE1, remoteAccessStrategy.get(KEY, System.currentTimeMillis())); } private int getValidChildrenCount(Node node) {