[jbosscache-commits] JBoss Cache SVN: r5388 - in core/trunk/src/main/java/org/jboss/cache: interceptors and 1 other directories.

jbosscache-commits at lists.jboss.org jbosscache-commits at lists.jboss.org
Wed Mar 5 11:55:13 EST 2008


Author: manik.surtani at jboss.com
Date: 2008-03-05 11:55:13 -0500 (Wed, 05 Mar 2008)
New Revision: 5388

Modified:
   core/trunk/src/main/java/org/jboss/cache/CacheImpl.java
   core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticCreateIfNotExistsInterceptor.java
   core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticInterceptor.java
   core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticNodeInterceptor.java
   core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticValidatorInterceptor.java
   core/trunk/src/main/java/org/jboss/cache/interceptors/PessimisticLockInterceptor.java
   core/trunk/src/main/java/org/jboss/cache/optimistic/WorkspaceNode.java
   core/trunk/src/main/java/org/jboss/cache/optimistic/WorkspaceNodeImpl.java
Log:
JBCACHE-1296 -  Deleting and reading parent node in tx causes deleted children to survive

Modified: core/trunk/src/main/java/org/jboss/cache/CacheImpl.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/CacheImpl.java	2008-03-05 16:54:24 UTC (rev 5387)
+++ core/trunk/src/main/java/org/jboss/cache/CacheImpl.java	2008-03-05 16:55:13 UTC (rev 5388)
@@ -1291,7 +1291,7 @@
       else
       {
          found = n.isValid() && !n.isDeleted();
-         n.markAsDeleted(true);
+         n.markAsDeleted(true, true);
       }
 
       if (eviction && parent_node != null)

Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticCreateIfNotExistsInterceptor.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticCreateIfNotExistsInterceptor.java	2008-03-05 16:54:24 UTC (rev 5387)
+++ core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticCreateIfNotExistsInterceptor.java	2008-03-05 16:55:13 UTC (rev 5388)
@@ -195,12 +195,7 @@
             {
                // exists in workspace and has just been created.
                currentNode = peekInWorkspace.getNode();
-               if (peekInWorkspace.isDeleted())
-               {
-                  peekInWorkspace.markAsDeleted(false);
-                  // add in parent again
-                  workspaceNode.addChild(peekInWorkspace);
-               }
+               if (peekInWorkspace.isDeleted()) undeleteWorkspaceNode(peekInWorkspace, workspaceNode);
             }
          }
 
@@ -249,7 +244,7 @@
             // node does exist but might not be in the workspace
             workspaceNode = workspace.getNode(currentNode.getFqn());
             // wrap it up so we can put it in later if we need to
-            if (workspaceNode == null || workspaceNode.isDeleted())
+            if (workspaceNode == null)
             {
                if (trace)
                   log.trace("Child node " + currentNode.getFqn() + " doesn't exist in workspace or has been deleted.  Adding to workspace in gtx " + gtx);
@@ -272,6 +267,11 @@
                   log.trace("setting versioning of " + workspaceNode.getFqn() + " to be " + (workspaceNode.isVersioningImplicit() ? "implicit" : "explicit"));
                workspace.addNode(workspaceNode);
             }
+            else if (workspaceNode.isDeleted())
+            {
+               if (trace) log.trace("Found node but it is deleted in this workspace.  Needs resurrecting.");
+               undeleteWorkspaceNode(workspaceNode, workspace);
+            }
             else
             {
                if (trace) log.trace("Found child node in the workspace: " + currentNode);

Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticInterceptor.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticInterceptor.java	2008-03-05 16:54:24 UTC (rev 5387)
+++ core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticInterceptor.java	2008-03-05 16:55:13 UTC (rev 5388)
@@ -12,6 +12,7 @@
 import org.jboss.cache.NodeSPI;
 import org.jboss.cache.factories.annotations.Inject;
 import org.jboss.cache.optimistic.TransactionWorkspace;
+import org.jboss.cache.optimistic.WorkspaceNode;
 import org.jboss.cache.transaction.GlobalTransaction;
 import org.jboss.cache.transaction.OptimisticTransactionEntry;
 import org.jboss.cache.transaction.TransactionTable;
@@ -82,5 +83,23 @@
       return gtx;
    }
 
+   protected void undeleteWorkspaceNode(WorkspaceNode nodeToUndelete, TransactionWorkspace workspace)
+   {
+      undeleteWorkspaceNode(nodeToUndelete, workspace.getNode(nodeToUndelete.getFqn().getParent()));
+   }
 
+   /**
+    * Undeletes a node that already exists in the workspace, by setting appropriate flags and re-adding to parent's child map.
+    *
+    * @param nodeToUndelete WorkspaceNode to undelete
+    */
+   protected void undeleteWorkspaceNode(WorkspaceNode nodeToUndelete, WorkspaceNode parent)
+   {
+      nodeToUndelete.markAsDeleted(false);
+      // add in parent again
+      parent.addChild(nodeToUndelete);
+      nodeToUndelete.markAsResurrected(true);
+   }
+
+
 }

Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticNodeInterceptor.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticNodeInterceptor.java	2008-03-05 16:54:24 UTC (rev 5387)
+++ core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticNodeInterceptor.java	2008-03-05 16:55:13 UTC (rev 5388)
@@ -335,9 +335,8 @@
       // pre-notify
       if (notify) notifier.notifyNodeRemoved(workspaceNode.getFqn(), true, workspaceNode.getData(), ctx);
 
-      parentNode.removeChild(workspaceNode.getFqn().getLastElement());
-
       Fqn nodeFqn = workspaceNode.getFqn();
+      parentNode.removeChild(nodeFqn.getLastElement());
 
       SortedMap<Fqn, WorkspaceNode> tailMap = workspace.getNodesAfter(workspaceNode.getFqn());
 
@@ -544,10 +543,7 @@
          if (trace) log.trace("Node " + fqn + " has been deleted in the workspace.");
          if (undeleteIfNecessary)
          {
-            workspaceNode.markAsDeleted(false);
-            // re-add to parent
-            WorkspaceNode parent = fetchWorkspaceNode(ctx, fqn.getParent(), workspace, true, includeInvalidNodes);
-            parent.addChild(workspaceNode);
+            undeleteWorkspaceNode(workspaceNode, fetchWorkspaceNode(ctx, fqn.getParent(), workspace, true, includeInvalidNodes));
          }
          else
          {

Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticValidatorInterceptor.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticValidatorInterceptor.java	2008-03-05 16:54:24 UTC (rev 5387)
+++ core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticValidatorInterceptor.java	2008-03-05 16:55:13 UTC (rev 5388)
@@ -192,17 +192,34 @@
          else
          {
             boolean updateVersion = false;
-            if (workspaceNode.isChildrenModified())
+            if (workspaceNode.isChildrenModified() || workspaceNode.isResurrected()) // if it is newly created make sure we remove all underlying children that may exist, to solve a remove-and-create-in-tx condition
             {
                if (trace) log.trace("Updating children since node has modified children");
                // merge children.
                List<Set<Fqn>> deltas = workspaceNode.getMergedChildren();
 
                if (trace) log.trace("Applying children deltas to parent node " + underlyingNode.getFqn());
+
+               if (workspaceNode.isResurrected())
+               {
+                  // mark it as invalid so any direct references are marked as such
+                  Map childNode = underlyingNode.getChildrenMapDirect();
+                  if (childNode != null)
+                  {
+                     for (Object o : childNode.values())
+                     {
+                        NodeSPI cn = (NodeSPI) o;
+                        cn.setValid(false, true);
+                        if (!useTombstones) underlyingNode.removeChildDirect(cn.getFqn().getLastElement());
+                     }
+                  }
+               }
+
                for (Fqn child : deltas.get(0))
                {
-//                  underlyingNode.addChildDirect(workspaceNode.getChild(child.getLastElement()));
-                  underlyingNode.addChildDirect(workspace.getNode(child).getNode());
+                  NodeSPI childNode = workspace.getNode(child).getNode();
+                  underlyingNode.addChildDirect(childNode);
+                  childNode.setValid(true, false);
                }
 
                for (Fqn child : deltas.get(1))
@@ -211,14 +228,11 @@
                   NodeSPI childNode = underlyingNode.getChildDirect(child.getLastElement());
                   if (childNode != null) childNode.setValid(false, true);
 
-                  if (!useTombstones)
-                  {
-                     // don't retain the tombstone
-                     underlyingNode.removeChildDirect(child.getLastElement());
-                  }
+                  if (!useTombstones) underlyingNode.removeChildDirect(child.getLastElement());
                }
 
                updateVersion = underlyingNode.isLockForChildInsertRemove();
+
                // do we need to notify listeners of a modification??  If all we've done is added children then don't
                // notify.
             }

Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/PessimisticLockInterceptor.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/interceptors/PessimisticLockInterceptor.java	2008-03-05 16:54:24 UTC (rev 5387)
+++ core/trunk/src/main/java/org/jboss/cache/interceptors/PessimisticLockInterceptor.java	2008-03-05 16:55:13 UTC (rev 5388)
@@ -274,9 +274,10 @@
       // we need to mark new nodes created as deleted since they are only created to form a path to the node being removed, to
       // create a lock.
       boolean created = acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.WRITE, true, false, true, true, createdNodes, true);
+      TransactionEntry entry = null;
       if (ctx.getGlobalTransaction() != null)
       {
-         TransactionEntry entry = tx_table.get(ctx.getGlobalTransaction());
+         entry = tx_table.get(ctx.getGlobalTransaction());
          entry.addRemovedNode(fqn);
          for (NodeSPI nodeSPI : createdNodes)
          {
@@ -284,10 +285,11 @@
             nodeSPI.markAsDeleted(true);
          }
       }
-      acquireLocksOnChildren(peekNode(ctx, fqn, false, false, false), NodeLock.LockType.WRITE, ctx);
+      acquireLocksOnChildren(peekNode(ctx, fqn, false, false, false), NodeLock.LockType.WRITE, ctx, entry, true);
+
       if (!createdNodes.isEmpty())
       {
-         if (trace) log.trace("There were new nodes created, skiping notification on delete");
+         if (trace) log.trace("There were new nodes created, skipping notification on delete");
          Object[] args = ctx.getMethodCall().getArgs();
          if (trace)
             log.trace("Changing 'skipNotification' for method '_remove' from " + args[args.length - 1] + " to true");
@@ -530,12 +532,16 @@
       return created;
    }
 
+   private void acquireLocksOnChildren(NodeSPI parentNode, NodeLock.LockType lockType, InvocationContext ctx) throws InterruptedException
+   {
+      acquireLocksOnChildren(parentNode, lockType, ctx, null, false);
+   }
 
    /**
     * Acquires nodes on the children of this node. nodes on the node itself are not aquired.
     * If the supplied parent node is null the method returns(no op).
     */
-   private void acquireLocksOnChildren(NodeSPI parentNode, NodeLock.LockType lockType, InvocationContext ctx)
+   private void acquireLocksOnChildren(NodeSPI parentNode, NodeLock.LockType lockType, InvocationContext ctx, TransactionEntry entry, boolean addChildrenToDeletedList)
          throws InterruptedException
    {
       if (parentNode == null)
@@ -552,6 +558,13 @@
          if (gtx != null)
          {
             cache.getTransactionTable().addLocks(gtx, acquiredLocks);
+            if (addChildrenToDeletedList)
+            {
+               for (NodeLock l : acquiredLocks)
+               {
+                  entry.addRemovedNode(l.getFqn());
+               }
+            }
          }
          else
          {

Modified: core/trunk/src/main/java/org/jboss/cache/optimistic/WorkspaceNode.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/optimistic/WorkspaceNode.java	2008-03-05 16:54:24 UTC (rev 5387)
+++ core/trunk/src/main/java/org/jboss/cache/optimistic/WorkspaceNode.java	2008-03-05 16:55:13 UTC (rev 5388)
@@ -164,7 +164,7 @@
     *
     * @param workspaceNode
     */
-   void addChild(WorkspaceNode<K,V> workspaceNode);
+   void addChild(WorkspaceNode<K, V> workspaceNode);
 
    /**
     * @return true if children have been added to or removed from this node.  Not the same as 'dirty'.
@@ -172,8 +172,19 @@
    boolean isChildrenModified();
 
    /**
-    *
     * @return true if the child map has been loaded from the underlying data structure into the workspace.
     */
    boolean isChildrenLoaded();
+
+   /**
+    * @return teur if this node has been resurrected, i.e., it was deleted and re-added within the same transaction
+    */
+   boolean isResurrected();
+
+   /**
+    * Marks a node as resurrected, i.e., deleted and created again within the same transaction
+    *
+    * @param resurrected
+    */
+   void markAsResurrected(boolean resurrected);
 }

Modified: core/trunk/src/main/java/org/jboss/cache/optimistic/WorkspaceNodeImpl.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/optimistic/WorkspaceNodeImpl.java	2008-03-05 16:54:24 UTC (rev 5387)
+++ core/trunk/src/main/java/org/jboss/cache/optimistic/WorkspaceNodeImpl.java	2008-03-05 16:55:13 UTC (rev 5388)
@@ -48,7 +48,18 @@
    private Set<Fqn> childrenRemoved;// = new HashSet<Fqn>();
    private Map<K, V> optimisticDataMap;
    private boolean versioningImplicit = true; // default
+   private boolean resurrected = false;
 
+   // the following boolean attributes are encoded as bits in a byte:
+   // modified: attr & 1 (2 ^ 0)
+   // created: attr & 2 (2 ^ 1)
+   // childrenModified: attr & 4 (2 ^ 2)
+   // versioningImplicit: attr & 8 (2 ^ 3)
+   // resurrected: attr & 16 (2 ^ 4)
+
+   // versioningImplicit is enabled by default.
+   //private byte attr = 8;
+
    /**
     * Constructs with a node and workspace.
     */
@@ -92,6 +103,27 @@
       return optimisticChildNodeMap != null;
    }
 
+   public boolean isResurrected()
+   {
+      return resurrected;
+   }
+
+   public void markAsResurrected(boolean resurrected)
+   {
+      this.resurrected = resurrected;
+   }
+
+   @Override
+   public void markAsDeleted(boolean marker, boolean recursive)
+   {
+      super.markAsDeleted(marker, recursive);
+      if (marker)
+      {
+         if (childrenAdded != null) childrenAdded.clear();
+         if (childrenRemoved != null) childrenRemoved.clear();
+      }
+   }
+
    /**
     * Returns true if this node is modified.
     */




More information about the jbosscache-commits mailing list