Author: manik.surtani(a)jboss.com Date: 2008-01-03 08:20:31 -0500 (Thu, 03 Jan 2008) New Revision: 4966 Modified: core/trunk/src/main/java/org/jboss/cache/loader/ClusteredCacheLoader.java core/trunk/src/test/java/org/jboss/cache/loader/ClusteredCacheLoaderRegionBasedTest.java Log: Fixed CCL making remote calls before the cache is started Modified: core/trunk/src/main/java/org/jboss/cache/loader/ClusteredCacheLoader.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/loader/ClusteredCacheLoader.java 2008-01-03 12:42:32 UTC (rev 4965) +++ core/trunk/src/main/java/org/jboss/cache/loader/ClusteredCacheLoader.java 2008-01-03 13:20:31 UTC (rev 4966) @@ -9,6 +9,7 @@ import net.jcip.annotations.ThreadSafe; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.jboss.cache.CacheStatus; import org.jboss.cache.Fqn; import org.jboss.cache.Modification; import org.jboss.cache.NodeSPI; @@ -48,6 +49,17 @@ private ClusteredCacheLoaderConfig config; /** + * A test to check whether the cache is in it's started state. If not, calls should not be made as the channel may + * not have properly started, blocks due to state transfers may be in progress, etc. + * + * @return true if the cache is in it's STARTED state. + */ + protected boolean isCacheReady() + { + return cache.getCacheStatus() == CacheStatus.STARTED; + } + + /** * Sets the configuration. * A property <code>timeout</code> is used as the timeout value. */ @@ -70,7 +82,7 @@ public Set getChildrenNames(Fqn fqn) throws Exception { - if (!cache.getInvocationContext().isOriginLocal()) return Collections.emptySet(); + if (!isCacheReady() || !cache.getInvocationContext().isOriginLocal()) return Collections.emptySet(); lock.acquireLock(fqn, true); try { @@ -143,7 +155,7 @@ protected Map get0(Fqn name) throws Exception { // DON'T make a remote call if this is a remote call in the first place - leads to deadlocks - JBCACHE-1103 - if (!cache.getInvocationContext().isOriginLocal()) return Collections.emptyMap(); + if (!isCacheReady() || !cache.getInvocationContext().isOriginLocal()) return Collections.emptyMap(); lock.acquireLock(name, true); try { @@ -160,7 +172,7 @@ public boolean exists(Fqn name) throws Exception { // DON'T make a remote call if this is a remote call in the first place - leads to deadlocks - JBCACHE-1103 - if (!cache.getInvocationContext().isOriginLocal()) return false; + if (!isCacheReady() || !cache.getInvocationContext().isOriginLocal()) return false; lock.acquireLock(name, false); try @@ -178,32 +190,26 @@ public Object put(Fqn name, Object key, Object value) throws Exception { - if (cache.getInvocationContext().isOriginLocal()) + // DON'T make a remote call if this is a remote call in the first place - leads to deadlocks - JBCACHE-1103 + if (!isCacheReady() || !cache.getInvocationContext().isOriginLocal()) return null; + lock.acquireLock(name, true); + try { - lock.acquireLock(name, true); - try + NodeSPI n = cache.peek(name, false); + if (n == null) { - NodeSPI n = cache.peek(name, false); - if (n == null) - { - MethodCall call = MethodCallFactory.create(MethodDeclarations.getKeyValueMethodLocal, name, key, true); - return callRemote(call); - } - else - { - // dont bother with a remote call - return n.getDirect(key); - } + MethodCall call = MethodCallFactory.create(MethodDeclarations.getKeyValueMethodLocal, name, key, true); + return callRemote(call); } - finally + else { - lock.releaseLock(name); + // dont bother with a remote call + return n.getDirect(key); } } - else + finally { - log.trace("Call originated remotely. Not bothering to try and do a clustered get() for this put(). Returning null."); - return null; + lock.releaseLock(name); } } @@ -228,32 +234,26 @@ */ public Object remove(Fqn name, Object key) throws Exception { - if (cache.getInvocationContext().isOriginLocal()) + // DON'T make a remote call if this is a remote call in the first place - leads to deadlocks - JBCACHE-1103 + if (!isCacheReady() || !cache.getInvocationContext().isOriginLocal()) return false; + lock.acquireLock(name, true); + try { - lock.acquireLock(name, true); - try + NodeSPI n = cache.peek(name, true); + if (n == null) { - NodeSPI n = cache.peek(name, true); - if (n == null) - { - MethodCall call = MethodCallFactory.create(MethodDeclarations.getKeyValueMethodLocal, name, key, true); - return callRemote(call); - } - else - { - // dont bother with a remote call - return n.getDirect(key); - } + MethodCall call = MethodCallFactory.create(MethodDeclarations.getKeyValueMethodLocal, name, key, true); + return callRemote(call); } - finally + else { - lock.releaseLock(name); + // dont bother with a remote call + return n.getDirect(key); } } - else + finally { - log.trace("Call originated remotely. Not bothering to try and do a clustered get() for this remove(). Returning null."); - return null; + lock.releaseLock(name); } } Modified: core/trunk/src/test/java/org/jboss/cache/loader/ClusteredCacheLoaderRegionBasedTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/loader/ClusteredCacheLoaderRegionBasedTest.java 2008-01-03 12:42:32 UTC (rev 4965) +++ core/trunk/src/test/java/org/jboss/cache/loader/ClusteredCacheLoaderRegionBasedTest.java 2008-01-03 13:20:31 UTC (rev 4966) @@ -1,5 +1,8 @@ package org.jboss.cache.loader; +import org.testng.annotations.Test; + +@Test(groups = {"functional"}) public class ClusteredCacheLoaderRegionBasedTest extends ClusteredCacheLoaderTest { public ClusteredCacheLoaderRegionBasedTest()