[jbosscache-commits] JBoss Cache SVN: r6363 - in core/trunk/src: main/java/org/jboss/cache/interceptors and 7 other directories.
jbosscache-commits at lists.jboss.org
jbosscache-commits at lists.jboss.org
Tue Jul 22 19:13:28 EDT 2008
Author: manik.surtani at jboss.com
Date: 2008-07-22 19:13:28 -0400 (Tue, 22 Jul 2008)
New Revision: 6363
Modified:
core/trunk/src/main/java/org/jboss/cache/AbstractNodeFactory.java
core/trunk/src/main/java/org/jboss/cache/InternalNode.java
core/trunk/src/main/java/org/jboss/cache/NodeFactory.java
core/trunk/src/main/java/org/jboss/cache/NodeSPI.java
core/trunk/src/main/java/org/jboss/cache/PessimisticNodeFactory.java
core/trunk/src/main/java/org/jboss/cache/PessimisticUnversionedNode.java
core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java
core/trunk/src/main/java/org/jboss/cache/interceptors/MVCCLockingInterceptor.java
core/trunk/src/main/java/org/jboss/cache/invocation/NodeInvocationDelegate.java
core/trunk/src/main/java/org/jboss/cache/mvcc/MVCCNodeFactory.java
core/trunk/src/main/java/org/jboss/cache/mvcc/MVCCNodeHelper.java
core/trunk/src/main/java/org/jboss/cache/mvcc/NodeReference.java
core/trunk/src/main/java/org/jboss/cache/optimistic/OptimisticNodeFactory.java
core/trunk/src/test/java/org/jboss/cache/api/mvcc/LockTestBase.java
core/trunk/src/test/java/org/jboss/cache/api/mvcc/read_committed/ReadCommittedLockParentTest.java
core/trunk/src/test/java/org/jboss/cache/api/mvcc/repeatable_read/RepeatableReadLockParentTest.java
core/trunk/src/test/java/org/jboss/cache/lock/AbstractLockManagerRecordingTest.java
core/trunk/src/test/java/org/jboss/cache/lock/NodeBasedLockManagerRecordingTest.java
Log:
Improved MVCC locking code
Modified: core/trunk/src/main/java/org/jboss/cache/AbstractNodeFactory.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/AbstractNodeFactory.java 2008-07-22 22:23:42 UTC (rev 6362)
+++ core/trunk/src/main/java/org/jboss/cache/AbstractNodeFactory.java 2008-07-22 23:13:28 UTC (rev 6363)
@@ -11,9 +11,9 @@
import org.jboss.cache.factories.ComponentRegistry;
import org.jboss.cache.factories.annotations.Inject;
import org.jboss.cache.interceptors.InterceptorChain;
+import org.jboss.cache.invocation.InvocationContext;
import org.jboss.cache.invocation.InvocationContextContainer;
import org.jboss.cache.invocation.NodeInvocationDelegate;
-import org.jboss.cache.lock.LockStrategyFactory;
import org.jboss.cache.mvcc.ReadCommittedNode;
import org.jboss.cache.optimistic.TransactionWorkspace;
import org.jboss.cache.optimistic.WorkspaceNode;
@@ -33,27 +33,27 @@
protected InvocationContextContainer invocationContextContainer;
protected InterceptorChain interceptorChain;
protected CommandsFactory commandsFactory;
- protected LockStrategyFactory lockStrategyFactory;
protected ComponentRegistry componentRegistry;
+ protected DataContainer dataContainer;
@Inject
- private void injectComponentRegistry(ComponentRegistry componentRegistry)
+ private void injectComponentRegistry(ComponentRegistry componentRegistry, DataContainer dataContainer)
{
this.componentRegistry = componentRegistry;
+ this.dataContainer = dataContainer;
}
@Inject
public void injectDependencies(CacheSPI<K, V> cache, Configuration configuration,
InvocationContextContainer invocationContextContainer,
- InterceptorChain interceptorChain, CommandsFactory commandsFactory, LockStrategyFactory lockStrategyFactory)
+ InterceptorChain interceptorChain, CommandsFactory commandsFactory)
{
this.cache = cache;
this.configuration = configuration;
this.invocationContextContainer = invocationContextContainer;
this.interceptorChain = interceptorChain;
this.commandsFactory = commandsFactory;
- this.lockStrategyFactory = lockStrategyFactory;
}
private NodeSPI<K, V> initializeNodeInvocationDelegate(UnversionedNode<K, V> internal)
@@ -120,4 +120,9 @@
nid.injectDependencies(cache);
return nid;
}
+
+ public NodeSPI<K, V> createAndRegister(Fqn fqn, NodeSPI<K, V> parent, InvocationContext ctx, boolean attachToParent)
+ {
+ throw new UnsupportedOperationException("Unsupported in this implementation (" + getClass().getSimpleName() + ")!");
+ }
}
Modified: core/trunk/src/main/java/org/jboss/cache/InternalNode.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/InternalNode.java 2008-07-22 22:23:42 UTC (rev 6362)
+++ core/trunk/src/main/java/org/jboss/cache/InternalNode.java 2008-07-22 23:13:28 UTC (rev 6363)
@@ -65,7 +65,11 @@
Set<NodeSPI<K, V>> getChildren(boolean includeMarkedForRemoval);
- NodeSPI<K, V> getOrCreateChild(Object childName, GlobalTransaction gtx, boolean notify);
+ /**
+ * @deprecated should use the {@link org.jboss.cache.NodeFactory} instead.
+ */
+ @Deprecated
+ NodeSPI<K, V> getOrCreateChild(Object childName, GlobalTransaction gtx);
Set<Object> getChildrenNames();
Modified: core/trunk/src/main/java/org/jboss/cache/NodeFactory.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/NodeFactory.java 2008-07-22 22:23:42 UTC (rev 6362)
+++ core/trunk/src/main/java/org/jboss/cache/NodeFactory.java 2008-07-22 23:13:28 UTC (rev 6363)
@@ -1,5 +1,6 @@
package org.jboss.cache;
+import org.jboss.cache.invocation.InvocationContext;
import org.jboss.cache.mvcc.ReadCommittedNode;
import org.jboss.cache.optimistic.TransactionWorkspace;
import org.jboss.cache.optimistic.WorkspaceNode;
@@ -63,6 +64,21 @@
*/
NodeSPI<K, V> createNode(Object childName, NodeSPI<K, V> parent);
+ /**
+ * Creates a new node, registers an undo operation in the context, and optionally attaches the node to its parent.
+ * <p/>
+ * The assumption here is that any locks are acquired to prevent concurrent creation of the same node. Implementations
+ * of the NodeFactory should not attempt to synchronize or guard against concurrent creation.
+ * <p/>
+ *
+ * @param fqn fqn of node to create. Must not be null or root.
+ * @param parent parent to attach to. Must not be null, even if attachToParent is false.
+ * @param ctx invocation context to register with. Must not be null.
+ * @param attachToParent if true, the node is registered in the parent's child map. If false, it is not.
+ * @return a new node, or the existing node if one existed.
+ */
+ NodeSPI<K, V> createAndRegister(Fqn fqn, NodeSPI<K, V> parent, InvocationContext ctx, boolean attachToParent);
+
NodeSPI<K, V> createRootNode();
NodeSPI<K, V> createNodeInvocationDelegate(InternalNode<K, V> internalNode, boolean wrapWithNodeReference);
Modified: core/trunk/src/main/java/org/jboss/cache/NodeSPI.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/NodeSPI.java 2008-07-22 22:23:42 UTC (rev 6362)
+++ core/trunk/src/main/java/org/jboss/cache/NodeSPI.java 2008-07-22 23:13:28 UTC (rev 6363)
@@ -101,7 +101,9 @@
* @param name name of child to create
* @param tx transaction under which to create child
* @return newly created node
+ * @deprecated should use the {@link org.jboss.cache.NodeFactory} instead.
*/
+ @Deprecated
NodeSPI<K, V> getOrCreateChild(Object name, GlobalTransaction tx);
/**
Modified: core/trunk/src/main/java/org/jboss/cache/PessimisticNodeFactory.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/PessimisticNodeFactory.java 2008-07-22 22:23:42 UTC (rev 6362)
+++ core/trunk/src/main/java/org/jboss/cache/PessimisticNodeFactory.java 2008-07-22 23:13:28 UTC (rev 6363)
@@ -1,5 +1,8 @@
package org.jboss.cache;
+import org.jboss.cache.factories.annotations.Inject;
+import org.jboss.cache.lock.LockStrategyFactory;
+
import java.util.Map;
/**
@@ -10,11 +13,20 @@
*/
public class PessimisticNodeFactory<K, V> extends AbstractNodeFactory<K, V>
{
+ private LockStrategyFactory lockStrategyFactory;
+
+ @Inject
+ private void injectLockStrategyFactory(LockStrategyFactory lockStrategyFactory)
+ {
+ this.lockStrategyFactory = lockStrategyFactory;
+ }
+
@Override
protected UnversionedNode<K, V> createInternalNode(Object childName, Fqn fqn, NodeSPI<K, V> parent, Map<K, V> data)
{
PessimisticUnversionedNode<K, V> internal = new PessimisticUnversionedNode<K, V>(childName, fqn, data, cache);
- internal.injectDependencies(cache, commandsFactory, lockStrategyFactory, this);
+ internal.injectDependencies(cache, commandsFactory, this);
+ internal.injectLockStrategyFactory(lockStrategyFactory);
return internal;
}
}
Modified: core/trunk/src/main/java/org/jboss/cache/PessimisticUnversionedNode.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/PessimisticUnversionedNode.java 2008-07-22 22:23:42 UTC (rev 6362)
+++ core/trunk/src/main/java/org/jboss/cache/PessimisticUnversionedNode.java 2008-07-22 23:13:28 UTC (rev 6363)
@@ -3,6 +3,7 @@
import org.jboss.cache.commands.write.CreateNodeCommand;
import org.jboss.cache.invocation.InvocationContext;
import org.jboss.cache.lock.IdentityLock;
+import org.jboss.cache.lock.LockStrategyFactory;
import org.jboss.cache.transaction.GlobalTransaction;
import java.util.HashMap;
@@ -22,10 +23,11 @@
* locking is not needed.
*/
protected transient IdentityLock lock = null;
+ protected LockStrategyFactory lockStrategyFactory;
public PessimisticUnversionedNode(Object name, Fqn fqn, Map<K, V> data, CacheSPI<K, V> cache)
{
- super(fqn, cache);
+ super(fqn, cache, cache.getConfiguration().isLockParentForChildInsertRemove());
if (!fqn.isRoot() && !name.equals(fqn.getLastElement()))
throw new IllegalArgumentException("Child " + name + " must be last part of " + fqn);
@@ -38,6 +40,11 @@
// ------ lock-per-node paradigm
+ public void injectLockStrategyFactory(LockStrategyFactory lockStrategyFactory)
+ {
+ this.lockStrategyFactory = lockStrategyFactory;
+ }
+
protected synchronized void initLock()
{
if (lock == null)
@@ -77,6 +84,7 @@
{
PessimisticUnversionedNode<K, V> n = new PessimisticUnversionedNode<K, V>(fqn.getLastElement(), fqn, data, cache);
copyInternals(n);
+ n.lockStrategyFactory = lockStrategyFactory;
return n;
}
Modified: core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java 2008-07-22 22:23:42 UTC (rev 6362)
+++ core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java 2008-07-22 23:13:28 UTC (rev 6363)
@@ -13,7 +13,6 @@
import org.jboss.cache.commands.write.CreateNodeCommand;
import org.jboss.cache.invocation.InvocationContext;
import org.jboss.cache.lock.IdentityLock;
-import org.jboss.cache.lock.LockStrategyFactory;
import org.jboss.cache.marshall.MarshalledValue;
import org.jboss.cache.optimistic.DataVersion;
import org.jboss.cache.transaction.GlobalTransaction;
@@ -45,17 +44,16 @@
/**
* A reference of the CacheImpl instance.
*/
- transient CacheSPI<K, V> cache;
+// transient CacheSPI<K, V> cache;
/**
* Map of general data keys to values.
*/
protected HashMap<K, V> data;
-
protected NodeSPI<K, V> delegate;
CommandsFactory commandsFactory;
- protected LockStrategyFactory lockStrategyFactory;
protected NodeFactory<K, V> nodeFactory;
+ protected CacheSPI<K, V> cache;
/**
* Constructs a new node with an FQN of Root.
@@ -77,19 +75,19 @@
initFlags();
}
- public UnversionedNode(Fqn fqn, CacheSPI<K, V> cache)
+ public UnversionedNode(Fqn fqn, CacheSPI<K, V> cache, boolean lockForChildInsertRemove)
{
initFlags();
this.cache = cache;
+ setLockForChildInsertRemove(lockForChildInsertRemove);
this.fqn = fqn;
- init();
// if this is a root node, create the child map.
if (fqn.isRoot()) children = new ConcurrentHashMap<Object, Node<K, V>>(64, .5f, 16);
}
- public UnversionedNode(Fqn fqn, CacheSPI<K, V> cache, Map<K, V> data)
+ public UnversionedNode(Fqn fqn, CacheSPI<K, V> cache, boolean lockForChildInsertRemove, Map<K, V> data)
{
- this(fqn, cache);
+ this(fqn, cache, lockForChildInsertRemove);
if (data != null) this.data = new HashMap<K, V>(data);
}
@@ -113,25 +111,14 @@
this.delegate = delegate;
}
- public void injectDependencies(CacheSPI<K, V> spi, CommandsFactory commandsFactory, LockStrategyFactory lockStrategyFactory, NodeFactory<K, V> nodeFactory)
+ public void injectDependencies(CacheSPI<K, V> spi, CommandsFactory commandsFactory, NodeFactory<K, V> nodeFactory)
{
this.cache = spi;
this.commandsFactory = commandsFactory;
- this.lockStrategyFactory = lockStrategyFactory;
this.nodeFactory = nodeFactory;
- init();
}
/**
- * Initializes with a name and FQN and cache.
- */
- private void init()
- {
- if (cache != null && cache.getConfiguration() != null)
- setLockForChildInsertRemove(cache.getConfiguration().isLockParentForChildInsertRemove());
- }
-
- /**
* Returns a parent by checking the TreeMap by name.
*/
public NodeSPI<K, V> getParent()
@@ -202,9 +189,9 @@
return data.put(key, value);
}
- public NodeSPI<K, V> getOrCreateChild(Object childName, GlobalTransaction gtx, boolean notify)
+ public NodeSPI<K, V> getOrCreateChild(Object childName, GlobalTransaction gtx)
{
- return getOrCreateChild(childName, gtx, true, notify);
+ return getOrCreateChild(childName, gtx, true, true);
}
protected NodeSPI<K, V> getOrCreateChild(Object childName, GlobalTransaction gtx, boolean createIfNotExists, boolean notify)
@@ -666,7 +653,7 @@
@SuppressWarnings("unchecked")
public InternalNode<K, V> copy()
{
- UnversionedNode<K, V> n = new UnversionedNode<K, V>(fqn, cache);
+ UnversionedNode<K, V> n = new UnversionedNode<K, V>(fqn, cache, isFlagSet(LOCK_FOR_CHILD_INSERT_REMOVE));
if (data != null) n.data = (HashMap<K, V>) data.clone();
copyInternals(n);
return n;
@@ -675,12 +662,11 @@
protected void copyInternals(UnversionedNode n)
{
// direct reference to child map
- n.children = children;
+ n.children = children();
n.commandsFactory = commandsFactory;
n.delegate = delegate;
n.nodeFactory = nodeFactory;
n.flags = flags;
- n.lockStrategyFactory = lockStrategyFactory;
}
public void setInternalState(Map<K, V> state)
Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/MVCCLockingInterceptor.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/interceptors/MVCCLockingInterceptor.java 2008-07-22 22:23:42 UTC (rev 6362)
+++ core/trunk/src/main/java/org/jboss/cache/interceptors/MVCCLockingInterceptor.java 2008-07-22 23:13:28 UTC (rev 6363)
@@ -112,47 +112,59 @@
ctx.getOptionOverrides().setLockAcquisitionTimeout(0);
if (command.isRecursive())
{
- List<Fqn> fqnsToEvict;
- if (command.getFqn().isRoot())
+ handleRecursiveEvict(ctx, command);
+ }
+ else
+ {
+ handleNonrecursiveEvict(ctx, command);
+ }
+
+ return invokeNextInterceptor(ctx, command);
+ }
+
+ @SuppressWarnings("unchecked")
+ private void handleRecursiveEvict(InvocationContext ctx, EvictCommand command) throws InterruptedException
+ {
+ List<Fqn> fqnsToEvict;
+ if (command.getFqn().isRoot())
+ {
+ // special treatment - deal with all of root's kids instead.
+ Map<Object, NodeSPI> children = dataContainer.peek(Fqn.ROOT).getChildrenMapDirect();
+ if (children != null && !children.isEmpty())
{
- // special treatment - deal with all of root's kids instead.
- Map<Object, NodeSPI> children = dataContainer.peek(Fqn.ROOT).getChildrenMapDirect();
- if (children != null && !children.isEmpty())
- {
- fqnsToEvict = new LinkedList<Fqn>();
- for (NodeSPI child : children.values())
- fqnsToEvict.addAll(helper.wrapNodesForWriting(ctx, child.getFqn()));
- }
- else
- {
- fqnsToEvict = Collections.emptyList();
- }
+ fqnsToEvict = new LinkedList<Fqn>();
+ for (NodeSPI child : children.values())
+ fqnsToEvict.addAll(helper.wrapNodesForWriting(ctx, child.getFqn()));
}
else
{
- fqnsToEvict = helper.wrapNodesForWriting(ctx, command.getFqn());
+ fqnsToEvict = Collections.emptyList();
}
-
- command.setNodesToEvict(fqnsToEvict);
}
else
{
- if (command.getFqn().isRoot())
+ fqnsToEvict = helper.wrapNodesForWriting(ctx, command.getFqn());
+ }
+
+ command.setNodesToEvict(fqnsToEvict);
+ }
+
+ @SuppressWarnings("unchecked")
+ private void handleNonrecursiveEvict(InvocationContext ctx, EvictCommand command) throws InterruptedException
+ {
+ if (command.getFqn().isRoot())
+ {
+ Map<Object, NodeSPI> children = dataContainer.peek(Fqn.ROOT).getChildrenMapDirect();
+ if (children != null && !children.isEmpty())
{
- Map<Object, NodeSPI> children = dataContainer.peek(Fqn.ROOT).getChildrenMapDirect();
- if (children != null && !children.isEmpty())
- {
- for (NodeSPI child : children.values())
- helper.wrapNodeForWriting(ctx, child.getFqn(), true, false, false, true, true);
- }
+ for (NodeSPI child : children.values())
+ helper.wrapNodeForWriting(ctx, child.getFqn(), true, false, false, true, true);
}
- else
- {
- helper.wrapNodeForWriting(ctx, command.getFqn(), true, false, false, true, true);
- }
}
-
- return invokeNextInterceptor(ctx, command);
+ else
+ {
+ helper.wrapNodeForWriting(ctx, command.getFqn(), true, false, false, true, true);
+ }
}
@Override
@@ -332,7 +344,7 @@
else
{
if (trace)
- log.trace("Nothing to do since there is a transaction in scope. Deferring writing changes. Ctx=" + ctx);
+ log.trace("Nothing to do since there is a transaction in scope.");
}
}
Modified: core/trunk/src/main/java/org/jboss/cache/invocation/NodeInvocationDelegate.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/invocation/NodeInvocationDelegate.java 2008-07-22 22:23:42 UTC (rev 6362)
+++ core/trunk/src/main/java/org/jboss/cache/invocation/NodeInvocationDelegate.java 2008-07-22 23:13:28 UTC (rev 6363)
@@ -81,7 +81,7 @@
public NodeSPI<K, V> getOrCreateChild(Object name, GlobalTransaction tx)
{
- return node.getOrCreateChild(name, tx, true);
+ return node.getOrCreateChild(name, tx);
}
@SuppressWarnings("deprecation")
Modified: core/trunk/src/main/java/org/jboss/cache/mvcc/MVCCNodeFactory.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/mvcc/MVCCNodeFactory.java 2008-07-22 22:23:42 UTC (rev 6362)
+++ core/trunk/src/main/java/org/jboss/cache/mvcc/MVCCNodeFactory.java 2008-07-22 23:13:28 UTC (rev 6363)
@@ -1,12 +1,17 @@
package org.jboss.cache.mvcc;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
import org.jboss.cache.AbstractNodeFactory;
import org.jboss.cache.Fqn;
import org.jboss.cache.InternalNode;
import org.jboss.cache.NodeSPI;
import org.jboss.cache.UnversionedNode;
+import org.jboss.cache.commands.write.CreateNodeCommand;
import org.jboss.cache.factories.annotations.Start;
+import org.jboss.cache.invocation.InvocationContext;
import org.jboss.cache.lock.IsolationLevel;
+import org.jboss.cache.transaction.TransactionContext;
import java.util.Map;
@@ -20,6 +25,9 @@
{
private boolean useRepeatableRead;
private static final NullMarkerNode NULL_MARKER = new NullMarkerNode(null);
+ private static final Log log = LogFactory.getLog(MVCCNodeFactory.class);
+ private static final boolean trace = log.isTraceEnabled();
+ private boolean lockChildForInsertRemove;
/**
* Initialises the node factory with the configuration from the cache.
@@ -28,6 +36,7 @@
public void init()
{
useRepeatableRead = configuration.getIsolationLevel() == IsolationLevel.REPEATABLE_READ;
+ lockChildForInsertRemove = configuration.isLockParentForChildInsertRemove();
}
/**
@@ -51,7 +60,7 @@
private NodeSPI<K, V> initializeNodeInvocationDelegate(UnversionedNode<K, V> internal)
{
- internal.injectDependencies(cache, commandsFactory, lockStrategyFactory, this);
+ internal.injectDependencies(cache, commandsFactory, this);
// always assume that new nodes do not have data loaded
internal.setDataLoaded(false);
@@ -65,14 +74,14 @@
@Override
public NodeSPI<K, V> createNode(Fqn fqn, NodeSPI<K, V> parent, Map<K, V> data)
{
- UnversionedNode<K, V> internal = new UnversionedNode<K, V>(fqn, cache, data);
+ UnversionedNode<K, V> internal = new UnversionedNode<K, V>(fqn, cache, lockChildForInsertRemove, data);
return initializeNodeInvocationDelegate(internal);
}
@Override
public NodeSPI<K, V> createNode(Fqn fqn, NodeSPI<K, V> parent)
{
- UnversionedNode<K, V> internal = new UnversionedNode<K, V>(fqn, cache);
+ UnversionedNode<K, V> internal = new UnversionedNode<K, V>(fqn, cache, lockChildForInsertRemove);
return initializeNodeInvocationDelegate(internal);
}
@@ -98,4 +107,35 @@
if (wrapWithNodeReference) internalNode = new NodeReference<K, V>(internalNode);
return super.createNodeInvocationDelegate(internalNode, false);
}
+
+ @Override
+ public NodeSPI<K, V> createAndRegister(Fqn fqn, NodeSPI<K, V> parent, InvocationContext ctx, boolean attachToParent)
+ {
+ NodeSPI<K, V> child;
+ if (fqn == null) throw new IllegalArgumentException("null child fqn");
+
+ child = dataContainer.peek(fqn, false);
+
+ if (child == null)
+ {
+ cache.getNotifier().notifyNodeCreated(fqn, true, ctx);
+ NodeSPI<K, V> newChild = createNode(fqn, parent);
+ if (attachToParent) parent.addChildDirect(newChild);
+ // addChild actually succeeded!
+ child = newChild;
+
+ if (trace) log.trace("Created new child with fqn [" + fqn + "]");
+
+ TransactionContext tctx = null;
+ if ((tctx = ctx.getTransactionContext()) != null)
+ {
+ CreateNodeCommand createNodeCommand = commandsFactory.buildCreateNodeCommand(fqn);
+ tctx.addModification(createNodeCommand);
+ }
+
+ // notify if we actually created a new child
+ cache.getNotifier().notifyNodeCreated(fqn, false, ctx);
+ }
+ return child;
+ }
}
Modified: core/trunk/src/main/java/org/jboss/cache/mvcc/MVCCNodeHelper.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/mvcc/MVCCNodeHelper.java 2008-07-22 22:23:42 UTC (rev 6362)
+++ core/trunk/src/main/java/org/jboss/cache/mvcc/MVCCNodeHelper.java 2008-07-22 23:13:28 UTC (rev 6363)
@@ -170,7 +170,7 @@
{
ReadCommittedNode n = (ReadCommittedNode) context.lookUpNode(fqn);
if (createIfAbsent && n != null && n.isNullNode()) n = null;
- if (n != null)
+ if (n != null) // exists in context! Just acquire lock if needed, and wrap.
{
// acquire lock if needed
if (lockForWriting && acquireLock(context, fqn))
@@ -185,7 +185,6 @@
n.markAsDeleted(false);
n.setValid(true, false);
}
-
}
else
{
@@ -193,6 +192,7 @@
InternalNode in = dataContainer.peekInternalNode(fqn, includeInvalidNodes);
if (in != null)
{
+ // exists in cache! Just acquire lock if needed, and wrap.
// do we need a lock?
boolean needToCopy = false;
if (lockForWriting && acquireLock(context, fqn))
@@ -216,13 +216,14 @@
parentRCN.markForUpdate(context, dataContainer, nodeFactory, writeSkewCheck);
}
- // now to lock and create the node.
+ // now to lock and create the node. Lock first to prevent concurrent creation!
acquireLock(context, fqn);
+// NodeSPI temp = nodeFactory.createAndRegister(fqn, null, context, false);
NodeSPI temp = parent.getOrCreateChild(fqn.getLastElement(), context.getGlobalTransaction());
- // TODO: warning, hack! There is a race condition here. Add a way to create nodes without attaching to a parent.
parent.removeChildDirect(fqn.getLastElement());
+
in = ((NodeInvocationDelegate) temp).getDelegationTarget();
n = nodeFactory.createWrappedNode(in);
n.setCreated(true);
Modified: core/trunk/src/main/java/org/jboss/cache/mvcc/NodeReference.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/mvcc/NodeReference.java 2008-07-22 22:23:42 UTC (rev 6362)
+++ core/trunk/src/main/java/org/jboss/cache/mvcc/NodeReference.java 2008-07-22 23:13:28 UTC (rev 6363)
@@ -90,9 +90,9 @@
return delegate.put(key, value);
}
- public NodeSPI<K, V> getOrCreateChild(Object child_name, GlobalTransaction gtx, boolean notify)
+ public NodeSPI<K, V> getOrCreateChild(Object child_name, GlobalTransaction gtx)
{
- return delegate.getOrCreateChild(child_name, gtx, notify);
+ return delegate.getOrCreateChild(child_name, gtx);
}
public V remove(K key)
Modified: core/trunk/src/main/java/org/jboss/cache/optimistic/OptimisticNodeFactory.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/optimistic/OptimisticNodeFactory.java 2008-07-22 22:23:42 UTC (rev 6362)
+++ core/trunk/src/main/java/org/jboss/cache/optimistic/OptimisticNodeFactory.java 2008-07-22 23:13:28 UTC (rev 6363)
@@ -5,6 +5,8 @@
import org.jboss.cache.NodeSPI;
import org.jboss.cache.UnversionedNode;
import org.jboss.cache.VersionedNode;
+import org.jboss.cache.factories.annotations.Inject;
+import org.jboss.cache.lock.LockStrategyFactory;
import java.util.Map;
@@ -16,11 +18,20 @@
*/
public class OptimisticNodeFactory<K, V> extends AbstractNodeFactory<K, V>
{
+ private LockStrategyFactory lockStrategyFactory;
+
+ @Inject
+ private void injectLockStrategyFactory(LockStrategyFactory lockStrategyFactory)
+ {
+ this.lockStrategyFactory = lockStrategyFactory;
+ }
+
@Override
protected UnversionedNode<K, V> createInternalNode(Object childName, Fqn fqn, NodeSPI<K, V> parent, Map<K, V> data)
{
VersionedNode<K, V> internal = new VersionedNode<K, V>(fqn, parent, data, cache);
- internal.injectDependencies(cache, commandsFactory, lockStrategyFactory, this);
+ internal.injectDependencies(cache, commandsFactory, this);
+ internal.injectLockStrategyFactory(lockStrategyFactory);
return internal;
}
Modified: core/trunk/src/test/java/org/jboss/cache/api/mvcc/LockTestBase.java
===================================================================
--- core/trunk/src/test/java/org/jboss/cache/api/mvcc/LockTestBase.java 2008-07-22 22:23:42 UTC (rev 6362)
+++ core/trunk/src/test/java/org/jboss/cache/api/mvcc/LockTestBase.java 2008-07-22 23:13:28 UTC (rev 6363)
@@ -569,4 +569,59 @@
assert "v2".equals(cache.get(AB, "k"));
assert "v".equals(cache.get(ABC, "k"));
}
+
+ public void testOverwritingOnInsert2() throws Exception
+ {
+ cache.put(AB, "k", "v");
+
+ tm.begin();
+ cache.put(AB, "k", "v2");
+ assert "v2".equals(cache.get(AB, "k"));
+ assert null == cache.get(ABC, "k");
+ Transaction t1 = tm.suspend();
+
+ tm.begin();
+ cache.put(ABC, "k", "v");
+ assert "v".equals(cache.get(ABC, "k"));
+ assert "v".equals(cache.get(AB, "k"));
+ Transaction t2 = tm.suspend();
+
+ tm.resume(t1);
+ t1.commit();
+
+ assert "v2".equals(cache.get(AB, "k"));
+ assert null == cache.get(ABC, "k");
+
+ tm.resume(t2);
+ t2.commit();
+
+ assert "v2".equals(cache.get(AB, "k"));
+ assert "v".equals(cache.get(ABC, "k"));
+ }
+
+ public void testOverwritingOnInsert3() throws Exception
+ {
+ cache.put(AB, "k", "v");
+
+ tm.begin();
+ cache.put(AB, "k", "v2");
+ assert "v2".equals(cache.get(AB, "k"));
+ assert null == cache.get(ABC, "k");
+ Transaction t1 = tm.suspend();
+
+ tm.begin();
+ cache.put(ABC, "k", "v");
+ assert "v".equals(cache.get(ABC, "k"));
+ assert "v".equals(cache.get(AB, "k"));
+ tm.commit();
+
+ assert "v".equals(cache.get(ABC, "k"));
+ assert "v".equals(cache.get(AB, "k"));
+
+ tm.resume(t1);
+ t1.commit();
+
+ assert "v2".equals(cache.get(AB, "k"));
+ assert "v".equals(cache.get(ABC, "k"));
+ }
}
Modified: core/trunk/src/test/java/org/jboss/cache/api/mvcc/read_committed/ReadCommittedLockParentTest.java
===================================================================
--- core/trunk/src/test/java/org/jboss/cache/api/mvcc/read_committed/ReadCommittedLockParentTest.java 2008-07-22 22:23:42 UTC (rev 6362)
+++ core/trunk/src/test/java/org/jboss/cache/api/mvcc/read_committed/ReadCommittedLockParentTest.java 2008-07-22 23:13:28 UTC (rev 6363)
@@ -15,4 +15,16 @@
{
// no op since a locked parent makes tis test irrelevant.
}
+
+ @Override
+ public void testOverwritingOnInsert2()
+ {
+ // no op since a locked parent makes tis test irrelevant.
+ }
+
+ @Override
+ public void testOverwritingOnInsert3()
+ {
+ // no op since a locked parent makes tis test irrelevant.
+ }
}
Modified: core/trunk/src/test/java/org/jboss/cache/api/mvcc/repeatable_read/RepeatableReadLockParentTest.java
===================================================================
--- core/trunk/src/test/java/org/jboss/cache/api/mvcc/repeatable_read/RepeatableReadLockParentTest.java 2008-07-22 22:23:42 UTC (rev 6362)
+++ core/trunk/src/test/java/org/jboss/cache/api/mvcc/repeatable_read/RepeatableReadLockParentTest.java 2008-07-22 23:13:28 UTC (rev 6363)
@@ -15,4 +15,18 @@
{
// no op since a locked parent makes tis test irrelevant.
}
+
+ @Override
+ public void testOverwritingOnInsert2()
+ {
+ // no op since a locked parent makes tis test irrelevant.
+ }
+
+ @Override
+ public void testOverwritingOnInsert3()
+ {
+ // no op since a locked parent makes tis test irrelevant.
+ }
}
+
+
Modified: core/trunk/src/test/java/org/jboss/cache/lock/AbstractLockManagerRecordingTest.java
===================================================================
--- core/trunk/src/test/java/org/jboss/cache/lock/AbstractLockManagerRecordingTest.java 2008-07-22 22:23:42 UTC (rev 6362)
+++ core/trunk/src/test/java/org/jboss/cache/lock/AbstractLockManagerRecordingTest.java 2008-07-22 23:13:28 UTC (rev 6363)
@@ -67,7 +67,7 @@
protected NodeSPI createNode(Fqn fqn)
{
UnversionedNode un = new UnversionedNode(fqn);
- un.injectDependencies(null, null, new LockStrategyFactory(), null);
+ un.injectDependencies(null, null);
return new NodeInvocationDelegate(un);
}
}
Modified: core/trunk/src/test/java/org/jboss/cache/lock/NodeBasedLockManagerRecordingTest.java
===================================================================
--- core/trunk/src/test/java/org/jboss/cache/lock/NodeBasedLockManagerRecordingTest.java 2008-07-22 22:23:42 UTC (rev 6362)
+++ core/trunk/src/test/java/org/jboss/cache/lock/NodeBasedLockManagerRecordingTest.java 2008-07-22 23:13:28 UTC (rev 6363)
@@ -31,7 +31,7 @@
protected NodeSPI createNode(Fqn fqn)
{
PessimisticUnversionedNode un = new PessimisticUnversionedNode(fqn.getLastElement(), fqn, null, null);
- un.injectDependencies(null, null, new LockStrategyFactory(), null);
+ un.injectDependencies(null, null);
return new NodeInvocationDelegate(un);
}
}
More information about the jbosscache-commits
mailing list