[jbosscache-commits] JBoss Cache SVN: r5567 - in core/trunk/src: main/java/org/jboss/cache/commands/cachedata and 4 other directories.

jbosscache-commits at lists.jboss.org jbosscache-commits at lists.jboss.org
Mon Apr 14 22:09:13 EDT 2008


Author: manik.surtani at jboss.com
Date: 2008-04-14 22:09:13 -0400 (Mon, 14 Apr 2008)
New Revision: 5567

Modified:
   core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java
   core/trunk/src/main/java/org/jboss/cache/commands/cachedata/MoveCommand.java
   core/trunk/src/main/java/org/jboss/cache/commands/cachedata/PutDataMapCommand.java
   core/trunk/src/main/java/org/jboss/cache/commands/cachedata/PutKeyValueCommand.java
   core/trunk/src/main/java/org/jboss/cache/commands/cachedata/RemoveDataCommand.java
   core/trunk/src/main/java/org/jboss/cache/commands/cachedata/RemoveKeyCommand.java
   core/trunk/src/main/java/org/jboss/cache/commands/cachedata/RemoveNodeCommand.java
   core/trunk/src/main/java/org/jboss/cache/interceptors/BaseRpcInterceptor.java
   core/trunk/src/main/java/org/jboss/cache/interceptors/CallInterceptor.java
   core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticReplicationInterceptor.java
   core/trunk/src/main/java/org/jboss/cache/interceptors/ReplicationInterceptor.java
   core/trunk/src/main/java/org/jboss/cache/interceptors/TxInterceptor.java
   core/trunk/src/main/java/org/jboss/cache/lock/LockManager.java
   core/trunk/src/main/java/org/jboss/cache/transaction/TransactionEntry.java
   core/trunk/src/main/java/org/jboss/cache/transaction/TransactionTable.java
   core/trunk/src/test/java/org/jboss/cache/replicated/AsyncReplTest.java
   core/trunk/src/test/java/org/jboss/cache/replicated/SyncReplTxTest.java
Log:
Transaction rollback fixes

Modified: core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java	2008-04-15 00:34:48 UTC (rev 5566)
+++ core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java	2008-04-15 02:09:13 UTC (rev 5567)
@@ -10,7 +10,6 @@
 import org.apache.commons.logging.LogFactory;
 import static org.jboss.cache.AbstractNode.NodeFlags.*;
 import org.jboss.cache.commands.CommandsFactory;
-import org.jboss.cache.commands.cachedata.RemoveNodeCommand;
 import org.jboss.cache.factories.annotations.CacheInjectionMethods;
 import org.jboss.cache.factories.annotations.Inject;
 import org.jboss.cache.lock.IdentityLock;
@@ -272,11 +271,14 @@
                if (notify) cache.getNotifier().notifyNodeCreated(child_fqn, true, ctx);
                child = newChild;
                children.put(child_name, child);
-               if (gtx != null)
-               {
-                  RemoveNodeCommand undoOp = commandsFactory.buildRemoveNodeCommand(gtx, child_fqn, false, false, false);
-                  transactionTable.addUndoOperation(gtx, undoOp);
-               }
+
+               // why is this needed?
+//               if (gtx != null)
+//               {
+
+//                  RemoveNodeCommand undoOp = commandsFactory.buildRemoveNodeCommand(gtx, child_fqn, false, false, false);
+//                  transactionTable.addUndoOperation(gtx, undoOp);
+//               }
             }
          }
 

Modified: core/trunk/src/main/java/org/jboss/cache/commands/cachedata/MoveCommand.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/commands/cachedata/MoveCommand.java	2008-04-15 00:34:48 UTC (rev 5566)
+++ core/trunk/src/main/java/org/jboss/cache/commands/cachedata/MoveCommand.java	2008-04-15 02:09:13 UTC (rev 5567)
@@ -99,12 +99,6 @@
 
       if (!skipNotifications)
          notifier.notifyNodeMoved(nodeToMoveFqn, Fqn.fromRelativeElements(newParentFqn, nodeToMoveFqn.getLastElement()), false, ctx);
-
-      // now register an undo op
-      if (ctx.getTransaction() != null)
-      {
-         transactionTable.addUndoOperation(ctx.getGlobalTransaction(), this);
-      }
    }
 
    public Fqn getTo()

Modified: core/trunk/src/main/java/org/jboss/cache/commands/cachedata/PutDataMapCommand.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/commands/cachedata/PutDataMapCommand.java	2008-04-15 00:34:48 UTC (rev 5566)
+++ core/trunk/src/main/java/org/jboss/cache/commands/cachedata/PutDataMapCommand.java	2008-04-15 02:09:13 UTC (rev 5567)
@@ -66,14 +66,9 @@
       NodeSPI nodeSPI = cacheData.findNodeCheck(globalTransaction, fqn, false);
       oldData = nodeSPI.getDataDirect();
       notifier.notifyNodeModified(fqn, true, NodeModifiedEvent.ModificationType.PUT_MAP, oldData, ctx);
-      if (globalTransaction != null && createUndoOps)
-      {
-         transactionTable.addUndoOperation(globalTransaction, this);
-      }
-      if (eraseContents)
-      {
-         nodeSPI.clearDataDirect();
-      }
+
+      if (eraseContents) nodeSPI.clearDataDirect();
+
       nodeSPI.putAllDirect(data);
       notifier.notifyNodeModified(fqn, false, NodeModifiedEvent.ModificationType.PUT_MAP, nodeSPI.getDataDirect(), ctx);
       return null;

Modified: core/trunk/src/main/java/org/jboss/cache/commands/cachedata/PutKeyValueCommand.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/commands/cachedata/PutKeyValueCommand.java	2008-04-15 00:34:48 UTC (rev 5566)
+++ core/trunk/src/main/java/org/jboss/cache/commands/cachedata/PutKeyValueCommand.java	2008-04-15 02:09:13 UTC (rev 5567)
@@ -74,11 +74,7 @@
       Map rawData = n.getDataDirect();
       notifier.notifyNodeModified(fqn, true, NodeModifiedEvent.ModificationType.PUT_DATA, rawData, ctx);
       oldValue = n.putDirect(key, value);
-      // this modification) and put it into the TX's undo list.
-      if (globalTransaction != null && createUndoOps)
-      {
-         transactionTable.addUndoOperation(globalTransaction, this);
-      }
+
       Map newData = Collections.singletonMap(key, value);
       notifier.notifyNodeModified(fqn, false, NodeModifiedEvent.ModificationType.PUT_DATA, newData, ctx);
       return oldValue;

Modified: core/trunk/src/main/java/org/jboss/cache/commands/cachedata/RemoveDataCommand.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/commands/cachedata/RemoveDataCommand.java	2008-04-15 00:34:48 UTC (rev 5566)
+++ core/trunk/src/main/java/org/jboss/cache/commands/cachedata/RemoveDataCommand.java	2008-04-15 02:09:13 UTC (rev 5567)
@@ -76,18 +76,9 @@
 
       notifyAfter(data, ctx);
 
-      registerForRollback();
       return null;
    }
 
-   private void registerForRollback()
-   {
-      if (globalTransaction != null && createUndoops)
-      {
-         transactionTable.addUndoOperation(globalTransaction, this);
-      }
-   }
-
    private void prepareDataForRollback(Map data)
    {
       if (globalTransaction != null && createUndoops && !eviction && !data.isEmpty())

Modified: core/trunk/src/main/java/org/jboss/cache/commands/cachedata/RemoveKeyCommand.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/commands/cachedata/RemoveKeyCommand.java	2008-04-15 00:34:48 UTC (rev 5566)
+++ core/trunk/src/main/java/org/jboss/cache/commands/cachedata/RemoveKeyCommand.java	2008-04-15 02:09:13 UTC (rev 5567)
@@ -68,10 +68,6 @@
 
       this.oldValue = n.removeDirect(key);
 
-      if (globalTransaction != null && createUndoOps && oldValue != null)
-      {
-         transactionTable.addUndoOperation(globalTransaction, this);
-      }
       Map removedData = Collections.singletonMap(key, oldValue);
       notifier.notifyNodeModified(fqn, false, NodeModifiedEvent.ModificationType.REMOVE_DATA, removedData, ctx);
       return oldValue;

Modified: core/trunk/src/main/java/org/jboss/cache/commands/cachedata/RemoveNodeCommand.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/commands/cachedata/RemoveNodeCommand.java	2008-04-15 00:34:48 UTC (rev 5566)
+++ core/trunk/src/main/java/org/jboss/cache/commands/cachedata/RemoveNodeCommand.java	2008-04-15 02:09:13 UTC (rev 5567)
@@ -96,7 +96,6 @@
       if (globalTransaction != null && createUndoOps && !eviction && found)
       {
          parentFqn = parentNode.getFqn();
-         transactionTable.addUndoOperation(globalTransaction, this);
       }
 
       notifyAfterEviction(ctx);

Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/BaseRpcInterceptor.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/interceptors/BaseRpcInterceptor.java	2008-04-15 00:34:48 UTC (rev 5566)
+++ core/trunk/src/main/java/org/jboss/cache/interceptors/BaseRpcInterceptor.java	2008-04-15 02:09:13 UTC (rev 5567)
@@ -161,9 +161,7 @@
    protected boolean skipReplicationOfTransactionMethod(InvocationContext ctx)
    {
       GlobalTransaction gtx = ctx.getGlobalTransaction();
-      boolean isInitiatedHere = gtx != null && !gtx.isRemote();
-      if (trace) log.trace("isInitiatedHere? " + isInitiatedHere + "; gtx = " + gtx);
-      return !isTransactionalAndLocal(ctx);
+      return gtx == null || gtx.isRemote();
    }
 
    /**

Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/CallInterceptor.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/interceptors/CallInterceptor.java	2008-04-15 00:34:48 UTC (rev 5566)
+++ core/trunk/src/main/java/org/jboss/cache/interceptors/CallInterceptor.java	2008-04-15 02:09:13 UTC (rev 5567)
@@ -2,11 +2,11 @@
 
 import org.jboss.cache.InvocationContext;
 import org.jboss.cache.commands.CacheCommand;
-import org.jboss.cache.commands.functional.TxCacheCommand;
 import org.jboss.cache.commands.cachedata.PutDataMapCommand;
 import org.jboss.cache.commands.cachedata.PutKeyValueCommand;
 import org.jboss.cache.commands.cachedata.RemoveDataCommand;
 import org.jboss.cache.commands.cachedata.RemoveNodeCommand;
+import org.jboss.cache.commands.functional.TxCacheCommand;
 import org.jboss.cache.commands.tx.CommitCommand;
 import org.jboss.cache.commands.tx.OptimisticPrepareCommand;
 import org.jboss.cache.commands.tx.PrepareCommand;
@@ -51,28 +51,24 @@
    public Object handlePrepareCommand(InvocationContext ctx, PrepareCommand command) throws Throwable
    {
       if (trace) log.trace("Suppressing invocation of method handlePrepareCommand.");
-      Transaction tx = ctx.getTransaction();
       return null;
    }
 
    public Object handleOptimisticPrepareCommand(InvocationContext ctx, OptimisticPrepareCommand command) throws Throwable
    {
       if (trace) log.trace("Suppressing invocation of method handleOptimisticPrepareCommand.");
-      Transaction tx = ctx.getTransaction();
       return null;
    }
 
    public Object handleCommitCommand(InvocationContext ctx, CommitCommand command) throws Throwable
    {
       if (trace) log.trace("Suppressing invocation of method handleCommitCommand.");
-      Transaction tx = ctx.getTransaction();
       return null;
    }
 
    public Object handleRollbackCommand(InvocationContext ctx, RollbackCommand command) throws Throwable
    {
       if (trace) log.trace("Suppressing invocation of method handleRollbackCommand.");
-      Transaction tx = ctx.getTransaction();
       return null;
    }
 
@@ -104,22 +100,22 @@
 
    public Object handlePutDataMapCommand(InvocationContext ctx, PutDataMapCommand command) throws Throwable
    {
-      return handleAlterChacheMethod(ctx, command);
+      return handleAlterCacheMethod(ctx, command);
    }
 
    public Object handlePutKeyValueCommand(InvocationContext ctx, PutKeyValueCommand command) throws Throwable
    {
-      return handleAlterChacheMethod(ctx, command);
+      return handleAlterCacheMethod(ctx, command);
    }
 
    public Object handleRemoveNodeCommand(InvocationContext ctx, RemoveNodeCommand command) throws Throwable
    {
-      return handleAlterChacheMethod(ctx, command);
+      return handleAlterCacheMethod(ctx, command);
    }
 
    public Object handleRemoveDataCommand(InvocationContext ctx, RemoveDataCommand command) throws Throwable
    {
-      return handleAlterChacheMethod(ctx, command);
+      return handleAlterCacheMethod(ctx, command);
    }
 
    /**
@@ -128,7 +124,7 @@
     * in case a method has been invoked that the OptimisticNodeInterceptor knows nothing about, it will
     * filter down here.
     */
-   private Object handleAlterChacheMethod(InvocationContext ctx, TxCacheCommand command)
+   private Object handleAlterCacheMethod(InvocationContext ctx, TxCacheCommand command)
          throws Throwable
    {
       Object result = invokeCommand(ctx, command);
@@ -141,16 +137,21 @@
             {
                log.debug("didn't find GlobalTransaction for " + ctx.getTransaction() + "; won't add modification to transaction list");
             }
-         } else
+         }
+         else
          {
             Option o = ctx.getOptionOverrides();
             if (o != null && o.isCacheModeLocal())
             {
                log.debug("Not adding method to modification list since cache mode local is set.");
-            } else
+            }
+            else
             {
+               // TODO: Revisit this, this is a bug if a local rollback occurs!!
                transactionTable.addModification(gtx, command);
             }
+
+            // todo: consolidate cache loader and regular modification lists!!
             if (cacheLoaderManager != null)
                transactionTable.addCacheLoaderModification(gtx, command);
          }

Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticReplicationInterceptor.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticReplicationInterceptor.java	2008-04-15 00:34:48 UTC (rev 5566)
+++ core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticReplicationInterceptor.java	2008-04-15 02:09:13 UTC (rev 5567)
@@ -286,16 +286,19 @@
          this.workspace = workspace;
       }
 
+      @Override
       public Object handleDataGravitationCleanupCommand(InvocationContext ctx, DataGravitationCleanupCommand command) throws Throwable
       {
          return command;
       }
 
+      @Override
       public Object handleGravitateDataCommand(InvocationContext ctx, GravitateDataCommand command) throws Throwable
       {
          return command;
       }
 
+      @Override
       public Object handleEvictFqnCommand(InvocationContext ctx, EvictNodeCommand command) throws Throwable
       {
          EvictNodeCommand clone = commandsFactory.buildEvictFqnCommand(command.getFqn());
@@ -309,18 +312,21 @@
          return clone;
       }
 
+      @Override
       public Object handlePutDataMapCommand(InvocationContext ctx, PutDataMapCommand command) throws Throwable
       {
          PutDataMapCommand clone = commandsFactory.buildPutDataMapCommand(null, command.getFqn(), command.getData(), command.isCreateUndoOps(), command.isEraseContents());
          return setDataVersion(clone, clone.getFqn());
       }
 
+      @Override
       public Object handlePutKeyValueCommand(InvocationContext ctx, PutKeyValueCommand command) throws Throwable
       {
          PutKeyValueCommand clone = commandsFactory.buildPutKeyValueCommand(null, command.getFqn(), command.getKey(), command.getValue(), command.isCreateUndoOps(), command.isPutForExternalRead());
          return setDataVersion(clone, command.getFqn());
       }
 
+      @Override
       public Object handleRemoveNodeCommand(InvocationContext ctx, RemoveNodeCommand command) throws Throwable
       {
          RemoveNodeCommand clone = commandsFactory.buildRemoveNodeCommand(command.getGlobalTransaction(), command.getFqn(), command.isEviction(),
@@ -328,12 +334,14 @@
          return setDataVersion(clone, command.getFqn());
       }
 
+      @Override
       public Object handleRemoveKeyCommand(InvocationContext ctx, RemoveKeyCommand command) throws Throwable
       {
          RemoveKeyCommand clone = commandsFactory.buildRemoveKeyCommand(null, command.getFqn(), command.getKey(), command.isCreateUndoOps());
          return setDataVersion(clone, command.getFqn());
       }
 
+      @Override
       public Object handleRemoveDataCommand(InvocationContext ctx, RemoveDataCommand command) throws Throwable
       {
          RemoveDataCommand clone = commandsFactory.buildRemoveDataCommand(command.getGlobalTransaction(), command.getFqn(), command.isCreateUndoops(),
@@ -341,6 +349,7 @@
          return setDataVersion(clone, command.getFqn());
       }
 
+      @Override
       public Object handleDefault(InvocationContext ctx, CacheCommand command) throws Throwable
       {
          throw new CacheException("Not handling " + command + " commads!");

Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/ReplicationInterceptor.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/interceptors/ReplicationInterceptor.java	2008-04-15 00:34:48 UTC (rev 5566)
+++ core/trunk/src/main/java/org/jboss/cache/interceptors/ReplicationInterceptor.java	2008-04-15 02:09:13 UTC (rev 5567)
@@ -14,6 +14,7 @@
 import org.jboss.cache.commands.tx.RollbackCommand;
 import org.jboss.cache.config.Configuration;
 import org.jboss.cache.config.Option;
+import org.jboss.cache.factories.annotations.Inject;
 import org.jboss.cache.transaction.GlobalTransaction;
 import org.jboss.cache.transaction.TransactionTable;
 
@@ -29,6 +30,7 @@
 {
    private TransactionTable transactionTable;
 
+   @Inject
    public void setDependencies(TransactionTable txTable)
    {
       this.transactionTable = txTable;
@@ -45,6 +47,7 @@
       return false;
    }
 
+   @Override
    public Object handleCommitCommand(InvocationContext ctx, CommitCommand command) throws Throwable
    {
       if (!skipReplicationOfTransactionMethod(ctx))
@@ -52,6 +55,7 @@
       return invokeNextInterceptor(ctx, command);
    }
 
+   @Override
    public Object handlePrepareCommand(InvocationContext ctx, PrepareCommand command) throws Throwable
    {
       Object retVal = invokeNextInterceptor(ctx, command);
@@ -59,6 +63,7 @@
       return retVal;
    }
 
+   @Override
    public Object handleRollbackCommand(InvocationContext ctx, RollbackCommand command) throws Throwable
    {
       if (!skipReplicationOfTransactionMethod(ctx) && !ctx.isLocalRollbackOnly())
@@ -68,13 +73,17 @@
       return invokeNextInterceptor(ctx, command);
    }
 
+   @Override
    public Object handlePutKeyValueCommand(InvocationContext ctx, PutKeyValueCommand command) throws Throwable
    {
       if (skipReplication(ctx)) return invokeNextInterceptor(ctx, command);
       if (isTransactionalAndLocal(ctx))
       {
          Object returnValue = invokeNextInterceptor(ctx, command);
-         transactionTable.get(command.getGlobalTransaction()).setForceAsyncReplication(true);
+         if (command.isPutForExternalRead())
+         {
+            transactionTable.get(command.getGlobalTransaction()).setForceAsyncReplication(true);
+         }
          return returnValue;
       }
       else
@@ -83,31 +92,37 @@
       }
    }
 
+   @Override
    public Object handlePutDataMapCommand(InvocationContext ctx, PutDataMapCommand command) throws Throwable
    {
       return handleCrudMethod(ctx, command, false);
    }
 
+   @Override
    public Object handleDataGravitationCleanupCommand(InvocationContext ctx, DataGravitationCleanupCommand command) throws Throwable
    {
       return handleCrudMethod(ctx, command, false);
    }
 
+   @Override
    public Object handleMoveCommand(InvocationContext ctx, MoveCommand command) throws Throwable
    {
       return handleCrudMethod(ctx, command, false);
    }
 
+   @Override
    public Object handleRemoveNodeCommand(InvocationContext ctx, RemoveNodeCommand command) throws Throwable
    {
       return handleCrudMethod(ctx, command, false);
    }
 
+   @Override
    public Object handleRemoveKeyCommand(InvocationContext ctx, RemoveKeyCommand command) throws Throwable
    {
       return handleCrudMethod(ctx, command, false);
    }
 
+   @Override
    public Object handleRemoveDataCommand(InvocationContext ctx, RemoveDataCommand command) throws Throwable
    {
       return handleCrudMethod(ctx, command, false);

Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/TxInterceptor.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/interceptors/TxInterceptor.java	2008-04-15 00:34:48 UTC (rev 5566)
+++ core/trunk/src/main/java/org/jboss/cache/interceptors/TxInterceptor.java	2008-04-15 02:09:13 UTC (rev 5567)
@@ -10,9 +10,9 @@
 import org.jboss.cache.InvocationContext;
 import org.jboss.cache.RPCManager;
 import org.jboss.cache.ReplicationException;
-import org.jboss.cache.factories.annotations.Inject;
 import org.jboss.cache.commands.CacheCommand;
 import org.jboss.cache.commands.CommandsFactory;
+import org.jboss.cache.commands.functional.TxCacheCommand;
 import org.jboss.cache.commands.state.DataVersionCommand;
 import org.jboss.cache.commands.state.GlobalTransactionCommand;
 import org.jboss.cache.commands.tx.CommitCommand;
@@ -23,6 +23,7 @@
 import org.jboss.cache.commands.visitors.GlobalTransactionCommandsVisitor;
 import org.jboss.cache.config.Configuration;
 import org.jboss.cache.config.Option;
+import org.jboss.cache.factories.annotations.Inject;
 import org.jboss.cache.invocation.CacheLifecycleManager;
 import org.jboss.cache.invocation.CacheTransactionHelper;
 import org.jboss.cache.invocation.InvocationContextContainer;
@@ -32,7 +33,11 @@
 import org.jboss.cache.transaction.TransactionEntry;
 import org.jboss.cache.transaction.TxUtil;
 
-import javax.transaction.*;
+import javax.transaction.InvalidTransactionException;
+import javax.transaction.Status;
+import javax.transaction.Synchronization;
+import javax.transaction.SystemException;
+import javax.transaction.Transaction;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -75,7 +80,7 @@
 
 
    @Inject
-   public void intialize(Configuration configuration,  RPCManager rpcManager,
+   public void intialize(Configuration configuration, RPCManager rpcManager,
                          CacheTransactionHelper transactionHelper, Notifier notifier, InvocationContextContainer icc,
                          CacheLifecycleManager lifecycleManager, CommandsFactory factory)
    {
@@ -108,7 +113,8 @@
             result = handleRemotePrepare(ctx, command, command.isOnePhaseCommit());
             scrubTxsOnExit = true;
             incresePrepares();
-         } else
+         }
+         else
          {
             if (trace) log.trace("received my own message (discarding it)");
             result = null;
@@ -315,7 +321,8 @@
             {
                log.debug("Started new local TX as result of remote PREPARE: local TX=" + ltx + " (Status=" + ltx.getStatus() + "), global TX=" + gtx);
             }
-         } else
+         }
+         else
          {
             //this should be valid
             if (!ctx.isValidTransaction())
@@ -347,7 +354,8 @@
             log.debug("creating new tx entry");
             txTable.put(gtx, entry);
             if (trace) log.trace("TxTable contents: " + txTable);
-         } else
+         }
+         else
          {
             entry = txTable.get(gtx);
          }
@@ -359,7 +367,8 @@
          if (configuration.isNodeLockingOptimistic())
          {
             retval = handleOptimisticPrepare(ctx, gtx, ltx, (OptimisticPrepareCommand) command);
-         } else
+         }
+         else
          {
             retval = handlePessimisticPrepare(ctx, ltx, command);
          }
@@ -429,7 +438,8 @@
                   log.warn("Roll back failed encountered", th);
                }
                throw t;
-            } else
+            }
+            else
             {
                throw t;
             }
@@ -481,7 +491,8 @@
       if (gtx != null)
       {
          command = replaceGtx(command, gtx);
-      } else
+      }
+      else
       {
          // get the current globalTransaction from the txTable.
          gtx = txTable.get(tx);
@@ -532,16 +543,17 @@
             {
                if (trace)
                   log.trace("Using one-phase prepare.  Not propagating the prepare call up the stack until called to do so by the sync handler.");
-            } else
+            }
+            else
             {
                invokeNextInterceptor(ctx, command);
             }
 
             // JBCACHE-361 Confirm that the transaction is ACTIVE
-            if (ctx.isValidTransaction())
+            if (!ctx.isValidTransaction())
             {
                throw new ReplicationException("prepare() failed -- " +
-                     "local transaction status is not STATUS_ACTIVE;" +
+                     "local transaction status is not valid;" +
                      " is " + ltx.getStatus());
             }
          }
@@ -575,7 +587,8 @@
                if (success)
                {
                   ltx.commit();
-               } else
+               }
+               else
                {
                   ltx.rollback();
                }
@@ -643,7 +656,8 @@
          {
             log.error("method invocation failed", t);
             throw t;
-         } finally
+         }
+         finally
          {
             ctx.setOptionOverrides(originalOption);
          }
@@ -726,11 +740,13 @@
             if (configuration.isNodeLockingOptimistic())
             {
                commitCommand = commandsFactory.buildOptimisticPrepareCommand(gtx, null);
-            } else
+            }
+            else
             {
                commitCommand = commandsFactory.buildPrepareCommand(gtx, modifications, rpcManager.getLocalAddress(), true);
             }
-         } else
+         }
+         else
          {
             commitCommand = commandsFactory.buildCommitCommand(gtx);
          }
@@ -781,12 +797,13 @@
     *
     * @param gtx
     */
-   protected void runRollbackPhase(InvocationContext ctx, GlobalTransaction gtx, Transaction tx, List modifications)
+   protected void runRollbackPhase(InvocationContext ctx, GlobalTransaction gtx, Transaction tx, List<TxCacheCommand> modifications)
    {
       //Transaction ltx = null;
       try
       {
          ctx.setTxHasMods(modifications != null && modifications.size() > 0);
+
          // JBCACHE-457
          CacheCommand rollbackCommand = commandsFactory.buildRollbackCommand(gtx);
          if (trace)
@@ -829,7 +846,8 @@
       if (configuration.isNodeLockingOptimistic())
       {
          prepareCommand = commandsFactory.buildOptimisticPrepareCommand(gtx, null);
-      } else if (configuration.getCacheMode() != Configuration.CacheMode.REPL_ASYNC)
+      }
+      else if (configuration.getCacheMode() != Configuration.CacheMode.REPL_ASYNC)
       {
          prepareCommand = commandsFactory.buildPrepareCommand(gtx, modifications, rpcManager.getLocalAddress(),
                false);// don't commit or rollback - wait for call
@@ -855,7 +873,8 @@
       {
          ctx.setExecutingCommand(prepareCommand);
          result = invokeNextInterceptor(ctx, prepareCommand);
-      } else
+      }
+      else
       {
          log.warn("Local transaction does not exist or does not match expected transaction " + gtx);
          throw new CacheException(" local transaction " + ltx + " does not exist or does not match expected transaction " + gtx);
@@ -888,7 +907,8 @@
             {
                log.trace("is a remotely initiated gtx so no need to register a tx for it");
             }
-         } else
+         }
+         else
          {
             if (trace)
             {
@@ -898,10 +918,12 @@
             LocalSynchronizationHandler myHandler = new LocalSynchronizationHandler(gtx, tx, !ctx.isOriginLocal());
             registerHandler(tx, myHandler, ctx, txTable.get(gtx));
          }
-      } else if ((gtx = (GlobalTransaction) rollbackTransactions.get(tx)) != null)
+      }
+      else if ((gtx = (GlobalTransaction) rollbackTransactions.get(tx)) != null)
       {
          if (log.isDebugEnabled()) log.debug("Transaction " + tx + " is already registered and is rolling back.");
-      } else
+      }
+      else
       {
          if (log.isDebugEnabled()) log.debug("Transaction " + tx + " is already registered.");
 

Modified: core/trunk/src/main/java/org/jboss/cache/lock/LockManager.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/lock/LockManager.java	2008-04-15 00:34:48 UTC (rev 5566)
+++ core/trunk/src/main/java/org/jboss/cache/lock/LockManager.java	2008-04-15 02:09:13 UTC (rev 5567)
@@ -5,10 +5,10 @@
 import org.jboss.cache.Fqn;
 import org.jboss.cache.InvocationContext;
 import org.jboss.cache.NodeSPI;
-import org.jboss.cache.factories.annotations.Inject;
 import org.jboss.cache.commands.CommandsFactory;
 import org.jboss.cache.commands.cachedata.PutDataMapCommand;
 import org.jboss.cache.config.Configuration;
+import org.jboss.cache.factories.annotations.Inject;
 import org.jboss.cache.invocation.CacheData;
 import org.jboss.cache.transaction.GlobalTransaction;
 import org.jboss.cache.transaction.TransactionEntry;
@@ -263,7 +263,8 @@
          //if we'll rollback the tx data should be added to the node again
          Map oldData = new HashMap(childNode.getDataDirect());
          PutDataMapCommand command = commandsFactory.buildPutDataMapCommand(gtx, fqn, oldData, false, false);
-         txTable.get(gtx).addUndoOperation(command);
+         // txTable.get(gtx).addUndoOperation(command); --- now need to make sure this is added to the normal mods list instead
+         txTable.get(gtx).addModification(command);
          //we're prepared for rollback, now reset the node
          childNode.clearDataDirect();
          if (createdNodes != null)

Modified: core/trunk/src/main/java/org/jboss/cache/transaction/TransactionEntry.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/transaction/TransactionEntry.java	2008-04-15 00:34:48 UTC (rev 5566)
+++ core/trunk/src/main/java/org/jboss/cache/transaction/TransactionEntry.java	2008-04-15 02:09:13 UTC (rev 5567)
@@ -12,8 +12,8 @@
 import org.apache.commons.logging.LogFactory;
 import org.jboss.cache.Fqn;
 import org.jboss.cache.Modification;
-import org.jboss.cache.commands.functional.TxCacheCommand;
 import org.jboss.cache.commands.cachedata.RemoveNodeCommand;
+import org.jboss.cache.commands.functional.TxCacheCommand;
 import org.jboss.cache.config.Option;
 import org.jboss.cache.interceptors.OrderedSynchronizationHandler;
 import org.jboss.cache.lock.IdentityLock;
@@ -23,7 +23,13 @@
 import javax.transaction.RollbackException;
 import javax.transaction.SystemException;
 import javax.transaction.Transaction;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.LinkedHashSet;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.ListIterator;
 import java.util.concurrent.CopyOnWriteArrayList;
 
 /**
@@ -68,14 +74,6 @@
    private List<TxCacheCommand> classLoadeModList = new CopyOnWriteArrayList<TxCacheCommand>();
 
    /**
-    * List<MethodCall>. List of compensating {@link org.jboss.cache.marshall.MethodCall} objects
-    * which revert the ones in <tt>modification_list</tt>. For each entry in the modification list,
-    * we have a corresponding entry in this list. A rollback will simply iterate over this list in
-    * reverse to undo the modifications. Note that these undo-ops will never be replicated.
-    */
-   private final List<TxCacheCommand> undoList = new LinkedList();
-
-   /**
     * LinkedHashSet<IdentityLock> of locks acquired by the transaction. We use
     * a LinkedHashSet because we need efficient Set semantics (same lock can
     * be added multiple times) but also need guaranteed ordering for use
@@ -147,15 +145,6 @@
    }
 
    /**
-    * Returns the undo operations in use.
-    * Note:  This list may be concurrently modified.
-    */
-   public List<TxCacheCommand> getUndoOperations()
-   {
-      return undoList;
-   }
-
-   /**
     * Sets the local transaction for this entry.
     */
    public void setTransaction(Transaction tx)
@@ -326,12 +315,12 @@
    {
       if (log.isTraceEnabled())
       {
-         log.trace("undoOperations " + undoList);
+         log.trace("undoOperations " + modificationList);
       }
       ArrayList<TxCacheCommand> copy;
-      synchronized (undoList)
+      synchronized (modificationList)
       {
-         copy = new ArrayList<TxCacheCommand>(undoList);
+         copy = new ArrayList<TxCacheCommand>(modificationList);
       }
       for (ListIterator i = copy.listIterator(copy.size()); i.hasPrevious();)
       {
@@ -348,10 +337,6 @@
    {
       StringBuffer sb = new StringBuffer();
       sb.append("TransactionEntry\nmodificationList: ").append(modificationList);
-      synchronized (undoList)
-      {
-         sb.append("\nundoList: ").append(undoList);
-      }
       synchronized (locks)
       {
          sb.append("\nlocks: ").append(locks);
@@ -407,17 +392,12 @@
       return !modificationList.isEmpty() || !classLoadeModList.isEmpty();
    }
 
-   public void addUndoOperation(TxCacheCommand TxCacheCommand)
-   {
-      undoList.add(TxCacheCommand);
-   }
-
    public boolean wasRemovedInTx(Fqn fqn)
    {
       for (TxCacheCommand txCacheCommand : getCacheLoaderModifications())
       {
          //todo - revisit this as it is ugly. phps add an isRemovred(fqn) somwhere on command hierarchy?
-         if (txCacheCommand instanceof RemoveNodeCommand && fqn.isChildOrEquals(((RemoveNodeCommand)txCacheCommand).getFqn()))
+         if (txCacheCommand instanceof RemoveNodeCommand && fqn.isChildOrEquals(((RemoveNodeCommand) txCacheCommand).getFqn()))
          {
             return true;
          }

Modified: core/trunk/src/main/java/org/jboss/cache/transaction/TransactionTable.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/transaction/TransactionTable.java	2008-04-15 00:34:48 UTC (rev 5566)
+++ core/trunk/src/main/java/org/jboss/cache/transaction/TransactionTable.java	2008-04-15 02:09:13 UTC (rev 5567)
@@ -110,7 +110,8 @@
       {
          if (log.isDebugEnabled()) log.debug("Found local TX=" + ltx + ", global TX=" + gtx);
          return ltx;
-      } else
+      }
+      else
       {
          throw new IllegalStateException(" found no local TX for global TX " + gtx);
       }
@@ -205,20 +206,6 @@
    }
 
    /**
-    * Adds an undo operation to the global transaction.
-    */
-   public void addUndoOperation(GlobalTransaction gtx, TxCacheCommand cacheCommand)
-   {
-      TransactionEntry entry = get(gtx);
-      if (entry == null)
-      {
-         log.error("transaction not found (globalTransaction=" + gtx + ")");
-         return;
-      }
-      entry.addUndoOperation(cacheCommand);
-   }
-
-   /**
     * Adds a lock to the global transaction.
     */
    public void addLock(GlobalTransaction gtx, NodeLock l)

Modified: core/trunk/src/test/java/org/jboss/cache/replicated/AsyncReplTest.java
===================================================================
--- core/trunk/src/test/java/org/jboss/cache/replicated/AsyncReplTest.java	2008-04-15 00:34:48 UTC (rev 5566)
+++ core/trunk/src/test/java/org/jboss/cache/replicated/AsyncReplTest.java	2008-04-15 02:09:13 UTC (rev 5567)
@@ -43,6 +43,7 @@
       cache2 = createCache("CacheGroup");
    }
 
+
    private CacheSPI<Object, Object> createCache(String name) throws Exception
    {
       CacheSPI<Object, Object> cache = (CacheSPI<Object, Object>) new DefaultCacheFactory().createCache(UnitTestCacheConfigurationFactory.createConfiguration(CacheMode.REPL_ASYNC), false);
@@ -86,19 +87,7 @@
    @AfterMethod(alwaysRun = true)
    public void tearDown() throws Exception
    {
-      if (cache1 != null)
-      {
-         log("stopping cache1");
-         cache1.stop();
-         cache1 = null;
-      }
-
-      if (cache2 != null)
-      {
-         log("stopping cache2");
-         cache2.stop();
-         cache2 = null;
-      }
+      TestingUtil.killCaches(cache1, cache2);
    }
 
    public void testTxCompletion() throws Exception
@@ -138,19 +127,6 @@
 
       assertEquals("value2", cache1.get(fqn, key));
       assertEquals("value2", cache2.get(fqn, key));
-
-      if (cache1 != null)
-      {
-         cache1.stop();
-         cache1 = null;
-      }
-
-      if (cache2 != null)
-      {
-         cache2.stop();
-         cache2 = null;
-      }
-
    }
 
    public void testPutShouldNotReplicateToDifferentCluster()

Modified: core/trunk/src/test/java/org/jboss/cache/replicated/SyncReplTxTest.java
===================================================================
--- core/trunk/src/test/java/org/jboss/cache/replicated/SyncReplTxTest.java	2008-04-15 00:34:48 UTC (rev 5566)
+++ core/trunk/src/test/java/org/jboss/cache/replicated/SyncReplTxTest.java	2008-04-15 02:09:13 UTC (rev 5567)
@@ -164,34 +164,25 @@
       Integer age;
       Transaction tx;
 
-      try
-      {
-         initCaches(Configuration.CacheMode.REPL_SYNC);
-         cache1.getConfiguration().setSyncCommitPhase(true);
-         cache2.getConfiguration().setSyncCommitPhase(true);
+      initCaches(Configuration.CacheMode.REPL_SYNC);
+      cache1.getConfiguration().setSyncCommitPhase(true);
+      cache2.getConfiguration().setSyncCommitPhase(true);
 
-         // assertEquals(2, cache1.getMembers().size());
+      TransactionManager mgr = beginTransaction();
+      cache1.put("/a/b/c", "age", 38);
+      tx = mgr.suspend();
+      assertNull("age on cache2 must be null as the TX has not yet been committed", cache2.get("/a/b/c", "age"));
+      log.debug("cache1: locks held before commit: " + CachePrinter.printCacheLockingInfo(cache1));
+      log.debug("cache2: locks held before commit: " + CachePrinter.printCacheLockingInfo(cache2));
+      mgr.resume(tx);
+      mgr.commit();
+      log.debug("cache1: locks held after commit: " + CachePrinter.printCacheLockingInfo(cache1));
+      log.debug("cache2: locks held after commit: " + CachePrinter.printCacheLockingInfo(cache2));
 
-         TransactionManager mgr = beginTransaction();
-         cache1.put("/a/b/c", "age", 38);
-         tx = mgr.suspend();
-         assertNull("age on cache2 must be null as the TX has not yet been committed", cache2.get("/a/b/c", "age"));
-         log.debug("cache1: locks held before commit: " + CachePrinter.printCacheLockingInfo(cache1));
-         log.debug("cache2: locks held before commit: " + CachePrinter.printCacheLockingInfo(cache2));
-         mgr.resume(tx);
-         mgr.commit();
-         log.debug("cache1: locks held after commit: " + CachePrinter.printCacheLockingInfo(cache1));
-         log.debug("cache2: locks held after commit: " + CachePrinter.printCacheLockingInfo(cache2));
-
-         // value on cache2 must be 38
-         age = (Integer) cache2.get("/a/b/c", "age");
-         assertNotNull("\"age\" obtained from cache2 must be non-null ", age);
-         assertTrue("\"age\" must be 38", age == 38);
-      }
-      catch (Exception e)
-      {
-         fail(e.toString());
-      }
+      // value on cache2 must be 38
+      age = (Integer) cache2.get("/a/b/c", "age");
+      assertNotNull("\"age\" obtained from cache2 must be non-null ", age);
+      assertTrue("\"age\" must be 38", age == 38);
    }
 
    /**




More information about the jbosscache-commits mailing list