[jboss-cvs] JBossCache/src/org/jboss/cache/interceptors ...

Manik Surtani manik at jboss.org
Wed Mar 7 13:01:04 EST 2007


  User: msurtani
  Date: 07/03/07 13:01:04

  Modified:    src/org/jboss/cache/interceptors  
                        OptimisticLockingInterceptor.java
                        OptimisticValidatorInterceptor.java
  Log:
  JBCACHE-961 - more sophisticated merging of child maps in workspace nodes.
  
  Revision  Changes    Path
  1.27      +18 -26    JBossCache/src/org/jboss/cache/interceptors/OptimisticLockingInterceptor.java
  
  (In the diff below, changes in quantity of whitespace are not shown.)
  
  Index: OptimisticLockingInterceptor.java
  ===================================================================
  RCS file: /cvsroot/jboss/JBossCache/src/org/jboss/cache/interceptors/OptimisticLockingInterceptor.java,v
  retrieving revision 1.26
  retrieving revision 1.27
  diff -u -b -r1.26 -r1.27
  --- OptimisticLockingInterceptor.java	7 Feb 2007 22:06:40 -0000	1.26
  +++ OptimisticLockingInterceptor.java	7 Mar 2007 18:01:04 -0000	1.27
  @@ -19,8 +19,6 @@
   import org.jboss.cache.transaction.TransactionEntry;
   
   import java.lang.reflect.Method;
  -import java.util.Collection;
  -import java.util.Iterator;
   
   /**
    * Locks nodes during transaction boundaries
  @@ -40,7 +38,7 @@
   
      public Object invoke(MethodCall m) throws Throwable
      {
  -      // bypass for buddy group org metod calls.
  +      // bypass for buddy group org method calls.
         if (MethodDeclarations.isBuddyGroupOrganisationMethod(m.getMethodId())) return super.invoke(m);
   
         InvocationContext ctx = cache.getInvocationContext();
  @@ -50,23 +48,14 @@
         // bail out if _lock() is being called on the tree cache... this should never be called with o/l enabled.
         if (m.getMethodId() == MethodDeclarations.lockMethodLocal_id)
         {
  -         log.warn("OptimisticLockingInterceptor intercepted a call to CacheImpl._lock().  " +
  -                 "This should NEVER be called if optimistic locking is used!!  " +
  -                 "Not allowing this call to proceed further down the chain.");
  -         return retval;
  +         throw new CacheException("_lock() passed up the interceptor stack when Optimistic Locking is used.  This is NOT supported.");
         }
   
  -      if (ctx.getTransaction() != null)
  +      if (ctx.getTransaction() != null && ctx.getGlobalTransaction() != null)
         {
            GlobalTransaction gtx = ctx.getGlobalTransaction();
   
  -         if (gtx == null)
  -         {
  -            throw new Exception("failed to get global transaction");
  -         }
            //we are interested in the prepare/commit/rollback
  -
  -         //methods we are interested in are prepare/commit
            //this is irrespective of whether we are local or remote
            switch (m.getMethodId())
            {
  @@ -103,24 +92,26 @@
                  try
                  {
                     retval = super.invoke(m);
  -                  unlock(gtx);
                  }
                  catch (Throwable t)
                  {
  -                  log.debug("exception encountered on " + meth + " running unlock ", t);
  +                  log.debug("exception encountered on " + meth, t);
  +                  throw t;
  +               }
  +               finally
  +               {
                     try
                     {
                        unlock(gtx);
                     }
  -                  catch (Throwable ct)
  +                  catch (Exception e)
                     {
  -                     log.fatal("Failed to unlock on " + meth, t);
  +                     log.fatal("Failed to unlock on " + meth, e);
                     }
  -                  throw t;
                  }
                  break;
               default:
  -               //we do not care
  +               //we do not care, just pass up the chain.
                  retval = super.invoke(m);
                  break;
            }
  @@ -128,7 +119,7 @@
         }
         else
         {
  -         throw new CacheException("not in a transaction");
  +         throw new CacheException("Not in a transaction");
         }
   
         return retval;
  @@ -137,21 +128,22 @@
      private Object lockNodes(GlobalTransaction gtx) throws Exception
      {
         TransactionWorkspace workspace = getTransactionWorkspace(gtx);
  -      log.debug("locking nodes");
  +      log.debug("Locking nodes in transaction workspace, presumably for a prepare()");
   
         // should be an ordered list
  -      for (WorkspaceNode workspaceNode : workspace.getNodes().values()) {
  +      for (WorkspaceNode workspaceNode : workspace.getNodes().values())
  +      {
            
            NodeSPI node = workspaceNode.getNode();
            boolean acquired = node.getLock().acquire(gtx, lockAcquisitionTimeout, NodeLock.LockType.WRITE);
            if (acquired)
            {
  -            if (log.isTraceEnabled()) log.trace("acquired lock on node " + node.getFqn());
  +            if (log.isTraceEnabled()) log.trace("Acquired lock on node " + node.getFqn());
               cache.getTransactionTable().addLock(gtx, node.getLock());
            }
            else
            {
  -            throw new CacheException("unable to acquire lock on node " + node.getFqn());
  +            throw new CacheException("Unable to acquire lock on node " + node.getFqn());
            }
   
         }
  
  
  
  1.58      +29 -12    JBossCache/src/org/jboss/cache/interceptors/OptimisticValidatorInterceptor.java
  
  (In the diff below, changes in quantity of whitespace are not shown.)
  
  Index: OptimisticValidatorInterceptor.java
  ===================================================================
  RCS file: /cvsroot/jboss/JBossCache/src/org/jboss/cache/interceptors/OptimisticValidatorInterceptor.java,v
  retrieving revision 1.57
  retrieving revision 1.58
  diff -u -b -r1.57 -r1.58
  --- OptimisticValidatorInterceptor.java	7 Feb 2007 22:06:40 -0000	1.57
  +++ OptimisticValidatorInterceptor.java	7 Mar 2007 18:01:04 -0000	1.58
  @@ -20,7 +20,9 @@
   
   import javax.transaction.Transaction;
   import java.util.Collection;
  +import java.util.List;
   import java.util.Map;
  +import java.util.Set;
   
   /**
    * Validates the data in the transaction workspace against data in the actual
  @@ -135,10 +137,6 @@
         }
      }
   
  -   private boolean checkNotInitialRootVersion(NodeSPI n)
  -   {
  -      return !n.getFqn().isRoot() || !(n.getVersion() instanceof DefaultDataVersion) || n.getVersion() != DefaultDataVersion.ZERO;
  -   }
   
      private void commit(GlobalTransaction gtx)
      {
  @@ -185,23 +183,42 @@
            }
            else
            {
  -            // "Will somebody please think of the children!!"
  -            // if (wrappedNode.hasCreatedOrRemovedChildren() handleChildNodes(wrappedNode);
  -            if (wrappedNode.isDirty())
  -            {
                  NodeSPI current = wrappedNode.getNode();
  -               Map mergedChildren = wrappedNode.getMergedChildren();
  +            boolean updateVersion = false;
  +            if (wrappedNode.isChildrenModified())
  +            {
  +               log.trace("Updating children since node has modified children");
  +               // merge children.
  +               List<Set<NodeSPI>> deltas = wrappedNode.getMergedChildren();
  +
  +               if (trace) log.trace("Applying children deltas to parent node " + current.getFqn());
  +               for (NodeSPI child : deltas.get(0))
  +               {
  +                  current.addChildDirect(child);
  +               }
   
  -               // this could be done better to account for more subtle merges
  -               current.setChildrenMapDirect(mergedChildren);
  +               for (NodeSPI child : deltas.get(1))
  +               {
  +                  current.removeChildDirect(child.getFqn());
  +               }
  +
  +               updateVersion = cache.getConfiguration().isLockParentForChildInsertRemove();
  +            }
   
  +            if (wrappedNode.isDirty())
  +            {
                  // do we need to notify listeners of a modification??  If all we've done is added children then don't
                  // notify.
  +               log.trace("Merging data since node is dirty");
                  Map mergedData = wrappedNode.getMergedData();
   
                  current.clearDataDirect();
                  current.putAllDirect(mergedData);
  +               updateVersion = true;
  +            }
   
  +            if (updateVersion)
  +            {
                  if (wrappedNode.isVersioningImplicit())
                  {
                     if (trace) log.trace("Versioning is implicit; incrementing.");
  @@ -221,7 +238,7 @@
               {
                  if (trace)
                  {
  -                  log.trace("Merging node " + wrappedNode.getFqn() + " not necessary since the node is not dirty");
  +                  log.trace("Version update on " + wrappedNode.getFqn() + " not necessary since the node is not dirty or LockParentForChildInsertRemove is set to false");
                  }
               }
            }
  
  
  



More information about the jboss-cvs-commits mailing list