[jbosscache-commits] JBoss Cache SVN: r7908 - in core/trunk/src: main/java/org/jboss/cache/mvcc and 2 other directories.

jbosscache-commits at lists.jboss.org jbosscache-commits at lists.jboss.org
Wed Mar 18 05:45:28 EDT 2009


Author: manik.surtani at 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




More information about the jbosscache-commits mailing list