4:45 a.m.
Author: manik.surtani(a)jboss.com
Date: 2009-03-18 05:45:27 -0400 (Wed, 18 Mar 2009)
New Revision: 7908
Added:
core/trunk/src/main/java/org/jboss/cache/mvcc/NullMarkerNodeForRemoval.java
Modified:
core/trunk/src/main/java/org/jboss/cache/AbstractNodeFactory.java
core/trunk/src/main/java/org/jboss/cache/NodeFactory.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/NullMarkerNode.java
core/trunk/src/main/java/org/jboss/cache/mvcc/RepeatableReadNode.java
core/trunk/src/test/java/org/jboss/cache/api/CacheAPITest.java
core/trunk/src/test/java/org/jboss/cache/api/mvcc/repeatable_read/CacheAPIMVCCTest.java
Log:
JBCACHE-1493 - REPEATABLE_READ inconsistent with write skew disabled
Modified: core/trunk/src/main/java/org/jboss/cache/AbstractNodeFactory.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/AbstractNodeFactory.java 2009-03-18 08:43:29
UTC (rev 7907)
+++ core/trunk/src/main/java/org/jboss/cache/AbstractNodeFactory.java 2009-03-18 09:45:27
UTC (rev 7908)
@@ -125,6 +125,11 @@
throw new UnsupportedOperationException("Unsupported in this implementation
(" + getClass().getSimpleName() + ")!");
}
+ public ReadCommittedNode createWrappedNodeForRemoval(Fqn fqn, InternalNode<K, V>
node, InternalNode<K, V> parent)
+ {
+ throw new UnsupportedOperationException("Unsupported in this implementation
(" + getClass().getSimpleName() + ")!");
+ }
+
public NodeSPI<K, V> createRootNode()
{
return createNode(Fqn.ROOT, null);
Modified: core/trunk/src/main/java/org/jboss/cache/NodeFactory.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/NodeFactory.java 2009-03-18 08:43:29 UTC (rev
7907)
+++ core/trunk/src/main/java/org/jboss/cache/NodeFactory.java 2009-03-18 09:45:27 UTC (rev
7908)
@@ -38,6 +38,8 @@
{
ReadCommittedNode createWrappedNode(InternalNode<K, V> node, InternalNode<K,
V> parent);
+ ReadCommittedNode createWrappedNodeForRemoval(Fqn fqn, InternalNode<K, V> node,
InternalNode<K, V> parent);
+
WorkspaceNode<K, V> createWrappedNode(NodeSPI<K, V> dataNode,
TransactionWorkspace workspace);
/**
Modified: core/trunk/src/main/java/org/jboss/cache/mvcc/MVCCNodeFactory.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/mvcc/MVCCNodeFactory.java 2009-03-18 08:43:29
UTC (rev 7907)
+++ core/trunk/src/main/java/org/jboss/cache/mvcc/MVCCNodeFactory.java 2009-03-18 09:45:27
UTC (rev 7908)
@@ -43,7 +43,6 @@
public class MVCCNodeFactory<K, V> extends AbstractNodeFactory<K, V>
{
private boolean useRepeatableRead;
- private static final NullMarkerNode NULL_MARKER = new NullMarkerNode();
private static final Log log = LogFactory.getLog(MVCCNodeFactory.class);
private static final boolean trace = log.isTraceEnabled();
private boolean lockChildForInsertRemove;
@@ -68,11 +67,47 @@
* @return a ReadCommittedNode
*/
@Override
- @SuppressWarnings("unchecked")
public ReadCommittedNode createWrappedNode(InternalNode<K, V> node,
InternalNode<K, V> parent)
{
- if (node == null) return useRepeatableRead ? NULL_MARKER : null;
- ReadCommittedNode rcn = useRepeatableRead ? new RepeatableReadNode(node, parent) :
new ReadCommittedNode(node, parent);
+ return createWrappedNode(null, node, parent, false);
+ }
+
+ @Override
+ public ReadCommittedNode createWrappedNodeForRemoval(Fqn fqn, InternalNode<K, V>
node, InternalNode<K, V> parent)
+ {
+ return createWrappedNode(fqn, node, parent, true);
+ }
+
+ @SuppressWarnings("unchecked")
+ private ReadCommittedNode createWrappedNode(Fqn fqn, InternalNode<K, V> node,
InternalNode<K, V> parent, boolean forRemoval)
+ {
+ ReadCommittedNode rcn;
+
+ if (node == null)
+ {
+ if (useRepeatableRead)
+ {
+ if (forRemoval)
+ {
+ // create but do not return this just yet as it needs to be initialized
+ rcn = new NullMarkerNodeForRemoval(parent, fqn);
+ }
+ else
+ {
+ return NullMarkerNode.getInstance();
+ }
+ }
+ else
+ {
+ // if we are using read-committed, just return a null
+ return null;
+ }
+ }
+ else
+ {
+ rcn = useRepeatableRead ? new RepeatableReadNode(node, parent) : new
ReadCommittedNode(node, parent);
+ }
+
rcn.initialize(configuration, invocationContextContainer, componentRegistry,
interceptorChain);
rcn.injectDependencies(cache);
return rcn;
Modified: core/trunk/src/main/java/org/jboss/cache/mvcc/MVCCNodeHelper.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/mvcc/MVCCNodeHelper.java 2009-03-18 08:43:29
UTC (rev 7907)
+++ core/trunk/src/main/java/org/jboss/cache/mvcc/MVCCNodeHelper.java 2009-03-18 09:45:27
UTC (rev 7908)
@@ -85,10 +85,8 @@
* Attempts to provide the context with a set of wrapped nodes based on the Collection
of fqns passed in. If the
* nodes already exist in the context then the node is not wrapped again.
* <p/>
- * {@link InternalNode}s are wrapped using {@link
org.jboss.cache.NodeFactory#createWrappedNode(org.jboss.cache.InternalNode,
- * org.jboss.cache.InternalNode)} and as such, null internal nodes are treated
according to isolation level used.
- * See {@link
org.jboss.cache.NodeFactory#createWrappedNode(org.jboss.cache.InternalNode,
- * org.jboss.cache.InternalNode)} for details on this behaviour.
+ * {@link InternalNode}s are wrapped using {@link
org.jboss.cache.NodeFactory#createWrappedNode(org.jboss.cache.InternalNode,
org.jboss.cache.InternalNode)} and as such, null internal nodes are treated according to
isolation level used.
+ * See {@link
org.jboss.cache.NodeFactory#createWrappedNode(org.jboss.cache.InternalNode,
org.jboss.cache.InternalNode)} for details on this behaviour.
* <p/>
* Note that if the context has the {@link
org.jboss.cache.config.Option#isForceWriteLock()} option set, then write
* locks are acquired and the node is copied.
@@ -404,9 +402,9 @@
@SuppressWarnings("unchecked")
private ReadCommittedNode wrapAndPutInContext(InvocationContext ctx, Fqn fqn, boolean
forUpdate) {
ReadCommittedNode node = (ReadCommittedNode) ctx.lookUpNode(fqn);
- if (node == null) {
+ if (node == null || node.isNullNode()) {
InternalNode[] nodes = dataContainer.peekInternalNodeAndDirectParent(fqn,
false);
- node = nodeFactory.createWrappedNode(nodes[0], nodes[1]);
+ node = nodeFactory.createWrappedNodeForRemoval(fqn, nodes[0], nodes[1]);
ctx.putLookedUpNode(fqn, node);
}
Modified: core/trunk/src/main/java/org/jboss/cache/mvcc/NullMarkerNode.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/mvcc/NullMarkerNode.java 2009-03-18 08:43:29
UTC (rev 7907)
+++ core/trunk/src/main/java/org/jboss/cache/mvcc/NullMarkerNode.java 2009-03-18 09:45:27
UTC (rev 7908)
@@ -30,41 +30,21 @@
* @author Manik Surtani (<a href="mailto:manik AT jboss DOT org">manik
AT jboss DOT org</a>)
* @since 3.0
*/
-public class NullMarkerNode extends RepeatableReadNode
+public class NullMarkerNode extends NullMarkerNodeForRemoval
{
- public NullMarkerNode()
+ private static final NullMarkerNode INSTANCE = new NullMarkerNode();
+
+ private NullMarkerNode()
{
super(null, null);
}
- /**
- * @return always returns true
- */
- @Override
- public boolean isNullNode()
+ public static NullMarkerNode getInstance()
{
- return true;
+ return INSTANCE;
}
/**
- * @return always returns true so that any get commands, upon getting this node, will
ignore the node as though it were removed.
- */
- @Override
- public boolean isDeleted()
- {
- return true;
- }
-
- /**
- * @return always returns true so that any get commands, upon getting this node, will
ignore the node as though it were invalid.
- */
- @Override
- public boolean isValid()
- {
- return false;
- }
-
- /**
* A no-op.
*/
@Override
Added: core/trunk/src/main/java/org/jboss/cache/mvcc/NullMarkerNodeForRemoval.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/mvcc/NullMarkerNodeForRemoval.java
(rev 0)
+++ core/trunk/src/main/java/org/jboss/cache/mvcc/NullMarkerNodeForRemoval.java 2009-03-18
09:45:27 UTC (rev 7908)
@@ -0,0 +1,90 @@
+package org.jboss.cache.mvcc;
+
+import org.jboss.cache.DataContainer;
+import org.jboss.cache.Fqn;
+import org.jboss.cache.InternalNode;
+import org.jboss.cache.InvocationContext;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * A specific type of null marker node, used for removal of nodes that don't exist
+ *
+ * @author Manik Surtani
+ * @since 3.1.0
+ */
+public class NullMarkerNodeForRemoval extends RepeatableReadNode
+{
+ private Fqn fqn;
+
+ public NullMarkerNodeForRemoval(InternalNode parent, Fqn fqn)
+ {
+ super(null, parent);
+ this.fqn = fqn;
+ }
+
+ @Override
+ public Fqn getFqn()
+ {
+ return fqn;
+ }
+
+ /**
+ * @return always returns true
+ */
+ @Override
+ public boolean isNullNode()
+ {
+ return true;
+ }
+
+ /**
+ * @return always returns true so that any get commands, upon getting this node, will
ignore the node as though it were invalid.
+ */
+ @Override
+ public boolean isValid()
+ {
+ return false;
+ }
+
+ /**
+ * @return always returns true so that any get commands, upon getting this node, will
ignore the node as though it were removed.
+ */
+ @Override
+ public boolean isDeleted()
+ {
+ return true;
+ }
+
+ @Override
+ protected void updateNode(Fqn fqn, InvocationContext ctx, DataContainer
dataContainer)
+ {
+ // no-op since the only updates that are allowed to happen here are the removal of
the node, which only affects the parent.
+ }
+
+ @Override
+ public Map getDataDirect()
+ {
+ return Collections.emptyMap();
+ }
+
+ @Override
+ public Set getChildrenNamesDirect()
+ {
+ return Collections.emptySet();
+ }
+
+ @Override
+ public Set getChildrenDirect()
+ {
+ return Collections.emptySet();
+ }
+
+ @Override
+ public void setValid(boolean valid, boolean recursive)
+ {
+ // no-op
+ }
+}
Modified: core/trunk/src/main/java/org/jboss/cache/mvcc/RepeatableReadNode.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/mvcc/RepeatableReadNode.java 2009-03-18
08:43:29 UTC (rev 7907)
+++ core/trunk/src/main/java/org/jboss/cache/mvcc/RepeatableReadNode.java 2009-03-18
09:45:27 UTC (rev 7908)
@@ -71,8 +71,13 @@
// make a backup copy
backup = node;
- node = backup.copy();
+ node = copyNode(backup);
}
+
+ private InternalNode copyNode(InternalNode nodeToCopy)
+ {
+ return nodeToCopy == null ? null : nodeToCopy.copy();
+ }
@Override
@SuppressWarnings("unchecked")
Modified: core/trunk/src/test/java/org/jboss/cache/api/CacheAPITest.java
===================================================================
--- core/trunk/src/test/java/org/jboss/cache/api/CacheAPITest.java 2009-03-18 08:43:29 UTC
(rev 7907)
+++ core/trunk/src/test/java/org/jboss/cache/api/CacheAPITest.java 2009-03-18 09:45:27 UTC
(rev 7908)
@@ -35,7 +35,7 @@
@Test(groups = {"functional", "pessimistic"}, sequential = true,
testName = "api.CacheAPITest")
public class CacheAPITest extends AbstractSingleCacheTest
{
- private CacheSPI<String, String> cache;
+ protected CacheSPI<String, String> cache;
private List<String> events;
public CacheSPI createCache()
Modified:
core/trunk/src/test/java/org/jboss/cache/api/mvcc/repeatable_read/CacheAPIMVCCTest.java
===================================================================
---
core/trunk/src/test/java/org/jboss/cache/api/mvcc/repeatable_read/CacheAPIMVCCTest.java 2009-03-18
08:43:29 UTC (rev 7907)
+++
core/trunk/src/test/java/org/jboss/cache/api/mvcc/repeatable_read/CacheAPIMVCCTest.java 2009-03-18
09:45:27 UTC (rev 7908)
@@ -6,7 +6,10 @@
import org.jboss.cache.lock.IsolationLevel;
import org.testng.annotations.Test;
+import javax.transaction.Transaction;
+import javax.transaction.TransactionManager;
+
/**
* MVCC version of {@link org.jboss.cache.api.CacheAPITest}
*/
@@ -25,4 +28,18 @@
{
return NodeLockingScheme.MVCC;
}
+
+ public void testWriteSkewOnRemovalOfNullNode() throws Exception
+ {
+ TransactionManager tm = cache.getTransactionManager();
+ tm.begin();
+ cache.getNode("/a");
+ Transaction tx = tm.suspend();
+ cache.put("/a", "k", "v2");
+ assert cache.get("/a", "k").equals("v2");
+ tm.resume(tx);
+ cache.removeNode("/a");
+ tx.commit();
+ assert cache.getNode("/a") == null; // this fails
+ }
}
\ No newline at end of file