5:45 p.m.
Author: manik.surtani(a)jboss.com
Date: 2008-07-30 11:45:12 -0400 (Wed, 30 Jul 2008)
New Revision: 6443
Modified:
core/trunk/src/main/java/org/jboss/cache/LegacyRegionManagerImpl.java
core/trunk/src/main/java/org/jboss/cache/RegionManagerImpl.java
Log:
Use StripedLock instead of a Set to keep track of concurrent activations and
deactivations!!!
cached log trace level
Modified: core/trunk/src/main/java/org/jboss/cache/LegacyRegionManagerImpl.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/LegacyRegionManagerImpl.java 2008-07-30
15:43:05 UTC (rev 6442)
+++ core/trunk/src/main/java/org/jboss/cache/LegacyRegionManagerImpl.java 2008-07-30
15:45:12 UTC (rev 6443)
@@ -32,14 +32,6 @@
@Override
protected void inactivateRegion(Fqn fqn) throws CacheException
{
- if (isActivatingDeactivating(fqn))
- {
-// throw new CacheException("Region " + fqn + " is already being
activated/deactivated");
- log.warn("Region " + fqn + " is already being
activated/deactivated");
- return;
- }
-
-
NodeSPI parent = null;
NodeSPI subtreeRoot = null;
boolean parentLocked = false;
@@ -48,7 +40,7 @@
try
{
// Record that this fqn is in status change, so can't provide state
- activationChangeNodes.add(fqn);
+ lock(fqn);
if (!isInactive(fqn))
{
@@ -142,7 +134,7 @@
}
}
- activationChangeNodes.remove(fqn);
+ unlock(fqn);
}
}
Modified: core/trunk/src/main/java/org/jboss/cache/RegionManagerImpl.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/RegionManagerImpl.java 2008-07-30 15:43:05
UTC (rev 6442)
+++ core/trunk/src/main/java/org/jboss/cache/RegionManagerImpl.java 2008-07-30 15:45:12
UTC (rev 6443)
@@ -19,11 +19,12 @@
import org.jboss.cache.invocation.InvocationContext;
import org.jboss.cache.lock.LockManager;
import static org.jboss.cache.lock.LockType.WRITE;
+import org.jboss.cache.util.concurrent.locks.LockContainer;
+import org.jboss.cache.util.concurrent.locks.ReentrantLockContainer;
import org.jgroups.Address;
import java.util.ArrayList;
import java.util.Collections;
-import java.util.HashSet;
import java.util.List;
import java.util.Set;
@@ -45,19 +46,39 @@
private RegionRegistry regionsRegistry;
private boolean defaultInactive;
- protected final Log log = LogFactory.getLog(RegionManagerImpl.class);
+ protected static final Log log = LogFactory.getLog(RegionManagerImpl.class);
+ protected static final boolean trace = log.isTraceEnabled();
CacheSPI<?, ?> cache;
private boolean usingEvictions;
private EvictionConfig evictionConfig;
private final EvictionTimerTask evictionTimerTask = new EvictionTimerTask();
- protected final Set<Fqn> activationChangeNodes = Collections.synchronizedSet(new
HashSet<Fqn>());
+ private final LockContainer<Fqn> regionLocks = new
ReentrantLockContainer<Fqn>(4);
protected Configuration configuration;
protected RPCManager rpcManager;
protected LockManager lockManager;
protected BuddyFqnTransformer buddyFqnTransformer;
private boolean isUsingBR;
+ // -------- region lock helpers
+
+ protected final boolean isRegionLocked(Fqn fqn)
+ {
+ return regionLocks.isLocked(fqn);
+ }
+
+ protected final void lock(Fqn fqn)
+ {
+ regionLocks.getLock(fqn).lock();
+ if (trace) log.trace("LOCKED " + fqn, new Throwable());
+ }
+
+ protected final void unlock(Fqn fqn)
+ {
+ if (trace) log.trace("UNLOCKED " + fqn, new Throwable());
+ regionLocks.getLock(fqn).unlock();
+ }
+
@Inject
public void injectDependencies(CacheSPI cache, Configuration configuration, RPCManager
rpcManager, LockManager lockManager,
BuddyFqnTransformer transformer, RegionRegistry
regionsRegistry)
@@ -109,7 +130,7 @@
protected void destroy()
{
regionsRegistry.clear();
- activationChangeNodes.clear();
+ regionLocks.reset();
}
public boolean isUsingEvictions()
@@ -169,7 +190,7 @@
fqn = buddyFqnTransformer.getActualFqn(fqn);
}
- if (log.isTraceEnabled()) log.trace("Contents of RegionsRegistry: " +
regionsRegistry);
+ if (trace) log.trace("Contents of RegionsRegistry: " + regionsRegistry);
Fqn fqnToUse = fqn;
if (DEFAULT_REGION.equals(fqnToUse)) fqnToUse = Fqn.ROOT;
// first see if a region for this specific Fqn exists
@@ -210,7 +231,7 @@
if (regionsRegistry.containsKey(nextFqn))
{
Region r = regionsRegistry.get(nextFqn);
- if (log.isTraceEnabled()) log.trace("Trying next region " + nextFqn
+ " and got " + r);
+ if (trace) log.trace("Trying next region " + nextFqn + " and
got " + r);
// this is a very poor way of telling whether a region is a marshalling one
or an eviction one. :-(
// mandates that class loaders be registered for marshalling regions.
@@ -271,7 +292,7 @@
{
try
{
- if (log.isTraceEnabled()) log.trace("Activating region " + fqn);
+ if (trace) log.trace("Activating region " + fqn);
Region r = getRegion(fqn, false);
if (r != null)
{
@@ -339,11 +360,6 @@
// Check whether the node already exists and has data
Node subtreeRoot = cache.getNode(fqn); // NOTE this used to be a peek!
- if (isActivatingDeactivating(fqn))
- {
- throw new CacheException("Region " + fqn + " is already being
activated/deactivated");
- }
-
if (log.isDebugEnabled())
{
log.debug("activating " + fqn);
@@ -354,7 +370,7 @@
// Add this fqn to the set of those we are activating
// so calls to _getState for the fqn can return quickly
- activationChangeNodes.add(fqn);
+ lock(fqn);
BuddyManager buddyManager = cache.getBuddyManager();
// Request partial state from the cluster and integrate it
@@ -366,8 +382,9 @@
{
// We'll update this node with the state we receive
// need to obtain all necessary locks.
+ Node root = cache.getRoot();
cache.getInvocationContext().getOptionOverrides().setCacheModeLocal(true);
- subtreeRoot = cache.getRoot().addChild(fqn);
+ subtreeRoot = root.addChild(fqn);
cache.getInvocationContext().getOptionOverrides().setCacheModeLocal(false);
}
@@ -440,7 +457,7 @@
}
finally
{
- activationChangeNodes.remove(fqn);
+ unlock(fqn);
}
}
@@ -466,13 +483,6 @@
*/
protected void inactivateRegion(Fqn fqn) throws CacheException
{
- if (isActivatingDeactivating(fqn))
- {
- log.warn("Region " + fqn + " is already being
activated/deactivated. Not doing anything.");
- return;
- }
-// throw new CacheException("Region " + fqn + " is already being
activated/deactivated");
-
NodeSPI parent = null;
NodeSPI subtreeRoot = null;
boolean parentLocked = false;
@@ -483,7 +493,7 @@
try
{
// Record that this fqn is in status change, so can't provide state
- activationChangeNodes.add(fqn);
+ lock(fqn);
if (!isInactive(fqn))
{
@@ -579,24 +589,10 @@
// If necessary, release locks
if (ctx != null) lockManager.unlock(ctx);
- activationChangeNodes.remove(fqn);
+ unlock(fqn);
}
}
- /**
- * <p/>
- * This is legacy code and should not be called directly. This is a private method
for now and will be refactored out.
- * You should be using {@link #activate(Fqn)} and {@link #deactivate(Fqn)}
- * <p/>
- *
- * @param fqn fqn of the region
- * @return true if the region defined by the fqn is in the process of
activating/deactivating
- */
- protected boolean isActivatingDeactivating(Fqn fqn)
- {
- return activationChangeNodes.contains(fqn);
- }
-
public boolean hasRegion(Fqn fqn, Region.Type type)
{
Region r = regionsRegistry.get(fqn);
@@ -728,7 +724,7 @@
for (EvictionRegionConfig erc : ercs)
{
Fqn fqn = erc.getRegionFqn();
- if (log.isTraceEnabled()) log.trace("Creating eviction region " +
fqn);
+ if (trace) log.trace("Creating eviction region " + fqn);
if (fqn.equals(DEFAULT_REGION))
{
@@ -736,7 +732,7 @@
{
throw new ConfigurationException("A default region for evictions has
already been set for this cache");
}
- if (log.isTraceEnabled()) log.trace("Applying settings for " +
DEFAULT_REGION + " to Fqn.ROOT");
+ if (trace) log.trace("Applying settings for " + DEFAULT_REGION +
" to Fqn.ROOT");
fqn = Fqn.ROOT;
setDefault = true;
}