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

jbosscache-commits at lists.jboss.org jbosscache-commits at lists.jboss.org
Wed May 14 05:59:57 EDT 2008


Author: manik.surtani at jboss.com
Date: 2008-05-14 05:59:57 -0400 (Wed, 14 May 2008)
New Revision: 5837

Modified:
   core/trunk/src/main/java/org/jboss/cache/InvocationContext.java
   core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java
   core/trunk/src/main/java/org/jboss/cache/commands/tx/OptimisticPrepareCommand.java
   core/trunk/src/main/java/org/jboss/cache/commands/tx/PrepareCommand.java
   core/trunk/src/main/java/org/jboss/cache/interceptors/ActivationInterceptor.java
   core/trunk/src/main/java/org/jboss/cache/interceptors/CacheLoaderInterceptor.java
   core/trunk/src/main/java/org/jboss/cache/interceptors/CacheStoreInterceptor.java
   core/trunk/src/main/java/org/jboss/cache/interceptors/CallInterceptor.java
   core/trunk/src/main/java/org/jboss/cache/interceptors/InvalidationInterceptor.java
   core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticNodeInterceptor.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/transaction/TransactionEntry.java
   core/trunk/src/main/java/org/jboss/cache/transaction/TransactionTable.java
   core/trunk/src/test/java/org/jboss/cache/api/pfer/PutForExternalReadTestBase.java
   core/trunk/src/test/java/org/jboss/cache/invocationcontext/TransactionTest.java
   core/trunk/src/test/java/org/jboss/cache/options/cachemodelocal/CacheModeLocalTestBase.java
Log:
JBCACHE-1345:  Invocations using LOCAL mode override cannot be rolled back

Modified: core/trunk/src/main/java/org/jboss/cache/InvocationContext.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/InvocationContext.java	2008-05-14 01:36:56 UTC (rev 5836)
+++ core/trunk/src/main/java/org/jboss/cache/InvocationContext.java	2008-05-14 09:59:57 UTC (rev 5837)
@@ -287,19 +287,6 @@
       txHasMods = b;
    }
 
-   /**
-    * Cache loader might have mods which are different from TX's mods; e.g. when cache is local and passivation is on.
-    */
-   public boolean isCacheLoaderHasMods()
-   {
-      return cacheLoaderHasMods;
-   }
-
-   public void setCacheLoaderHasMods(boolean cacheLoaderHasMods)
-   {
-      this.cacheLoaderHasMods = cacheLoaderHasMods;
-   }
-
    public boolean isLocalRollbackOnly()
    {
       return localRollbackOnly;
@@ -346,7 +333,6 @@
       this.setOriginLocal(template.isOriginLocal());
       this.setTransaction(template.getTransaction());
       this.setTxHasMods(template.isTxHasMods());
-      this.setCacheLoaderHasMods(template.isCacheLoaderHasMods());
    }
 
    @Override

Modified: core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java	2008-05-14 01:36:56 UTC (rev 5836)
+++ core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java	2008-05-14 09:59:57 UTC (rev 5837)
@@ -246,7 +246,6 @@
 
    private NodeSPI getOrCreateChild(Object child_name, GlobalTransaction gtx, boolean createIfNotExists, boolean notify)
    {
-
       NodeSPI child;
       if (child_name == null)
       {
@@ -279,7 +278,7 @@
                if (gtx != null)
                {
                   CreateNodeCommand createNodeCommand = commandsFactory.buildCreateNodeCommand(child_fqn);
-                  transactionTable.addModification(gtx, createNodeCommand);
+                  ctx.getTransactionEntry().addModification(createNodeCommand);
                }
             }
          }

Modified: core/trunk/src/main/java/org/jboss/cache/commands/tx/OptimisticPrepareCommand.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/commands/tx/OptimisticPrepareCommand.java	2008-05-14 01:36:56 UTC (rev 5836)
+++ core/trunk/src/main/java/org/jboss/cache/commands/tx/OptimisticPrepareCommand.java	2008-05-14 09:59:57 UTC (rev 5837)
@@ -54,6 +54,12 @@
    }
 
    @Override
+   public OptimisticPrepareCommand clone() throws CloneNotSupportedException
+   {
+      return (OptimisticPrepareCommand) super.clone();
+   }
+
+   @Override
    @SuppressWarnings("unchecked")
    public void setParameters(int commandId, Object[] args)
    {

Modified: core/trunk/src/main/java/org/jboss/cache/commands/tx/PrepareCommand.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/commands/tx/PrepareCommand.java	2008-05-14 01:36:56 UTC (rev 5836)
+++ core/trunk/src/main/java/org/jboss/cache/commands/tx/PrepareCommand.java	2008-05-14 09:59:57 UTC (rev 5837)
@@ -6,6 +6,7 @@
 import org.jboss.cache.transaction.GlobalTransaction;
 import org.jgroups.Address;
 
+import java.util.Collection;
 import java.util.List;
 
 /**
@@ -30,6 +31,11 @@
       this.onePhaseCommit = onePhaseCommit;
    }
 
+   public void removeModifications(Collection<ReversibleCommand> modificationsToRemove)
+   {
+      if (modifications != null) modifications.removeAll(modificationsToRemove);
+   }
+
    public PrepareCommand()
    {
    }
@@ -111,6 +117,11 @@
       return result;
    }
 
+   @Override
+   public PrepareCommand clone() throws CloneNotSupportedException
+   {
+      return (PrepareCommand) super.clone();
+   }
 
    @Override
    public String toString()

Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/ActivationInterceptor.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/interceptors/ActivationInterceptor.java	2008-05-14 01:36:56 UTC (rev 5836)
+++ core/trunk/src/main/java/org/jboss/cache/interceptors/ActivationInterceptor.java	2008-05-14 09:59:57 UTC (rev 5837)
@@ -292,7 +292,7 @@
       }
       List<Modification> cacheLoaderModifications = new ArrayList<Modification>();
 
-      builder.visitCollection(null, entry.getCacheLoaderModifications());
+      builder.visitCollection(null, entry.getModifications());
       if (cacheLoaderModifications.size() > 0)
       {
          loader.prepare(gtx, cacheLoaderModifications, false);

Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/CacheLoaderInterceptor.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/interceptors/CacheLoaderInterceptor.java	2008-05-14 01:36:56 UTC (rev 5836)
+++ core/trunk/src/main/java/org/jboss/cache/interceptors/CacheLoaderInterceptor.java	2008-05-14 09:59:57 UTC (rev 5837)
@@ -441,7 +441,7 @@
       TransactionEntry entry = ctx.getTransactionEntry();
       if (entry == null) return false;
 
-      for (ReversibleCommand txCacheCommand : entry.getCacheLoaderModifications())
+      for (ReversibleCommand txCacheCommand : entry.getModifications())
       {
          if (txCacheCommand instanceof RemoveNodeCommand && fqn.isChildOrEquals(txCacheCommand.getFqn())) return true;
       }

Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/CacheStoreInterceptor.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/interceptors/CacheStoreInterceptor.java	2008-05-14 01:36:56 UTC (rev 5836)
+++ core/trunk/src/main/java/org/jboss/cache/interceptors/CacheStoreInterceptor.java	2008-05-14 09:59:57 UTC (rev 5837)
@@ -105,7 +105,7 @@
       if (inTransaction())
       {
          if (trace) log.trace("transactional so don't put stuff in the cloader yet.");
-         if (ctx.isCacheLoaderHasMods())
+         if (ctx.isTxHasMods())
          {
             // this is a commit call.
             GlobalTransaction gtx = command.getGlobalTransaction();
@@ -154,7 +154,7 @@
       if (inTransaction())
       {
          if (trace) log.trace("transactional so don't put stuff in the cloader yet.");
-         if (ctx.isCacheLoaderHasMods())
+         if (ctx.isTxHasMods())
          {
             GlobalTransaction gtx = command.getGlobalTransaction();
             // this is a rollback method
@@ -352,7 +352,7 @@
       {
          throw new Exception("entry for transaction " + gtx + " not found in transaction table");
       }
-      List<ReversibleCommand> modifications = entry.getCacheLoaderModifications();
+      List<ReversibleCommand> modifications = entry.getModifications();
       if (modifications.size() == 0)
       {
          if (trace) log.trace("Transaction has not logged any modifications!");

Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/CallInterceptor.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/interceptors/CallInterceptor.java	2008-05-14 01:36:56 UTC (rev 5836)
+++ core/trunk/src/main/java/org/jboss/cache/interceptors/CallInterceptor.java	2008-05-14 09:59:57 UTC (rev 5837)
@@ -14,7 +14,6 @@
 import org.jboss.cache.commands.write.RemoveDataCommand;
 import org.jboss.cache.commands.write.RemoveKeyCommand;
 import org.jboss.cache.commands.write.RemoveNodeCommand;
-import org.jboss.cache.config.Option;
 import org.jboss.cache.factories.annotations.Inject;
 import org.jboss.cache.factories.annotations.Start;
 import org.jboss.cache.interceptors.base.CommandInterceptor;
@@ -165,20 +164,7 @@
          }
          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
-            {
-               // TODO: 2.2.0: Revisit this, this is a bug if a local rollback occurs!!
-               transactionTable.addModification(gtx, command);
-            }
-
-            // TODO: 2.2.0: consolidate cache loader and regular modification lists!!
-//            if (cacheLoaderManager != null)
-            if (cacheLoadingEnabled) transactionTable.addCacheLoaderModification(gtx, command);
+            ctx.getTransactionEntry().addModification(command);
          }
       }
       return result;

Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/InvalidationInterceptor.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/interceptors/InvalidationInterceptor.java	2008-05-14 01:36:56 UTC (rev 5836)
+++ core/trunk/src/main/java/org/jboss/cache/interceptors/InvalidationInterceptor.java	2008-05-14 09:59:57 UTC (rev 5837)
@@ -37,9 +37,9 @@
 
 import javax.transaction.SystemException;
 import javax.transaction.Transaction;
+import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.HashSet;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -125,17 +125,23 @@
    {
       Object retval = invokeNextInterceptor(ctx, command);
       Transaction tx = ctx.getTransaction();
-      if (tx != null && !optimistic)
+      if (tx != null)
       {
          if (trace) log.trace("Entering InvalidationInterceptor's prepare phase");
          // fetch the modifications before the transaction is committed (and thus removed from the txTable)
          GlobalTransaction gtx = ctx.getGlobalTransaction();
          TransactionEntry entry = ctx.getTransactionEntry();
          if (entry == null) throw new IllegalStateException("cannot find transaction entry for " + gtx);
-         List<ReversibleCommand> modifications = new LinkedList<ReversibleCommand>(command.getModifications());
-         if (modifications.size() > 0)
+
+         if (entry.hasModifications())
          {
-            broadcastInvalidate(modifications, gtx, tx, ctx);
+            if (entry.hasLocalModifications())
+            {
+               PrepareCommand clone = command.clone();
+               clone.removeModifications(entry.getLocalModifications());
+               command = clone;
+            }
+            broadcastInvalidate(command.getModifications(), gtx, tx, ctx);
          }
          else
          {
@@ -156,10 +162,12 @@
          GlobalTransaction gtx = ctx.getGlobalTransaction();
          TransactionEntry entry = ctx.getTransactionEntry();
          if (entry == null) throw new IllegalStateException("cannot find transaction entry for " + gtx);
-         List<ReversibleCommand> modifications = new LinkedList<ReversibleCommand>(command.getModifications());
-         if (modifications.size() > 0)
+
+         if (entry.hasModifications())
          {
-            txMods.put(gtx, modifications);
+            List<ReversibleCommand> mods = new ArrayList<ReversibleCommand>(entry.getModifications());
+            if (entry.hasLocalModifications()) mods.removeAll(entry.getLocalModifications());
+            txMods.put(gtx, mods);
          }
       }
       return retval;

Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticNodeInterceptor.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticNodeInterceptor.java	2008-05-14 01:36:56 UTC (rev 5836)
+++ core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticNodeInterceptor.java	2008-05-14 09:59:57 UTC (rev 5837)
@@ -29,7 +29,6 @@
 import org.jboss.cache.config.Option;
 import org.jboss.cache.factories.annotations.Inject;
 import org.jboss.cache.factories.annotations.Start;
-import org.jboss.cache.loader.CacheLoaderManager;
 import org.jboss.cache.notifications.Notifier;
 import static org.jboss.cache.notifications.event.NodeModifiedEvent.ModificationType.*;
 import org.jboss.cache.optimistic.DataVersion;
@@ -57,17 +56,14 @@
     */
    private NodeFactory nodeFactory;
    private Notifier notifier;
-   private CacheLoaderManager cacheLoaderManager;
    private DataContainer dataContainer;
    private long lockAcquisitionTimeout;
 
    @Inject
-   protected void injectDependencies(Notifier notifier, NodeFactory nodeFactory, CacheLoaderManager cacheLoaderManager,
-                                     DataContainer dataContainer)
+   protected void injectDependencies(Notifier notifier, NodeFactory nodeFactory, DataContainer dataContainer)
    {
       this.notifier = notifier;
       this.nodeFactory = nodeFactory;
-      this.cacheLoaderManager = cacheLoaderManager;
       this.dataContainer = dataContainer;
    }
 
@@ -94,7 +90,7 @@
          setVersioning(ctx, workspace, workspaceNode);
       }
       Object result = removeNode(workspace, workspaceNode, true, ctx);
-      addToModificationList(gtx, command, ctx);
+      addToModificationList(command, ctx);
       return result;
    }
 
@@ -117,7 +113,7 @@
          }
       }
       Object result = putDataKeyValueAndNotify(command.getKey(), command.getValue(), workspace, workspaceNode, ctx);
-      addToModificationList(gtx, command, ctx);
+      addToModificationList(command, ctx);
       return result;
    }
 
@@ -140,7 +136,7 @@
          }
       }
       putDataMapAndNotify(command.getData(), workspace, workspaceNode, ctx);
-      addToModificationList(gtx, command, ctx);
+      addToModificationList(command, ctx);
       return null;
    }
 
@@ -160,7 +156,7 @@
          setVersioning(ctx, workspace, workspaceNode);
       }
       moveNodeAndNotify(command.getTo(), workspaceNode, workspace, ctx);
-      addToModificationList(gtx, command, ctx);
+      addToModificationList(command, ctx);
       return null;
    }
 
@@ -176,7 +172,7 @@
          setVersioning(ctx, workspace, workspaceNode);
       }
       Object result = removeKeyAndNotify(command.getKey(), workspace, workspaceNode, ctx);
-      addToModificationList(gtx, command, ctx);
+      addToModificationList(command, ctx);
       return result;
    }
 
@@ -191,7 +187,7 @@
          setVersioning(ctx, workspace, workspaceNode);
       }
       removeDataAndNotify(workspace, workspaceNode, ctx);
-      addToModificationList(gtx, command, ctx);
+      addToModificationList(command, ctx);
       return null;
    }
 
@@ -345,18 +341,15 @@
 
    /**
     * Adds a method call to the modification list of a given transaction's transaction entry
-    *
-    * @param gtx transaction
     */
-   private void addToModificationList(GlobalTransaction gtx, ReversibleCommand command, InvocationContext ctx)
+   private void addToModificationList(ReversibleCommand command, InvocationContext ctx)
    {
       Option opt = ctx.getOptionOverrides();
       if (opt == null || !opt.isCacheModeLocal())
       {
-         txTable.addModification(gtx, command);
+         ctx.getTransactionEntry().addModification(command);
          if (log.isDebugEnabled()) log.debug("Adding command " + command + " to modification list");
       }
-      if (cacheLoaderManager != null) txTable.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-05-14 01:36:56 UTC (rev 5836)
+++ core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticReplicationInterceptor.java	2008-05-14 09:59:57 UTC (rev 5837)
@@ -35,6 +35,7 @@
 import org.jboss.cache.optimistic.WorkspaceNode;
 import org.jboss.cache.transaction.GlobalTransaction;
 import org.jboss.cache.transaction.OptimisticTransactionEntry;
+import org.jboss.cache.transaction.TransactionEntry;
 import org.jboss.cache.util.concurrent.ConcurrentHashSet;
 
 import java.util.ArrayList;
@@ -87,12 +88,16 @@
       if (!skipReplicationOfTransactionMethod(ctx))
       {
          GlobalTransaction gtx = getGlobalTransaction(ctx);
-
-         if (!gtx.isRemote() && ctx.isOriginLocal())
+         TransactionEntry te = ctx.getTransactionEntry();
+         if (te.hasLocalModifications())
          {
-            // replicate the prepare call.
-            broadcastPrepare(command, gtx, ctx);
+            OptimisticPrepareCommand replicablePrepareCommand = command.clone(); // makre sure we remove any "local" transactions
+            replicablePrepareCommand.removeModifications(te.getLocalModifications());
+            command = replicablePrepareCommand;
          }
+
+         // replicate the prepare call.
+         broadcastPrepare(command, gtx, ctx);
       }
       return retval;
    }

Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/ReplicationInterceptor.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/interceptors/ReplicationInterceptor.java	2008-05-14 01:36:56 UTC (rev 5836)
+++ core/trunk/src/main/java/org/jboss/cache/interceptors/ReplicationInterceptor.java	2008-05-14 09:59:57 UTC (rev 5837)
@@ -1,6 +1,7 @@
 package org.jboss.cache.interceptors;
 
 import org.jboss.cache.InvocationContext;
+import org.jboss.cache.commands.ReversibleCommand;
 import org.jboss.cache.commands.VisitableCommand;
 import org.jboss.cache.commands.tx.CommitCommand;
 import org.jboss.cache.commands.tx.PrepareCommand;
@@ -14,6 +15,7 @@
 import org.jboss.cache.config.Configuration;
 import org.jboss.cache.config.Option;
 import org.jboss.cache.transaction.GlobalTransaction;
+import org.jboss.cache.transaction.TransactionEntry;
 
 /**
  * Takes care of replicating modifications to other nodes in a cluster. Also
@@ -49,6 +51,14 @@
    public Object visitPrepareCommand(InvocationContext ctx, PrepareCommand command) throws Throwable
    {
       Object retVal = invokeNextInterceptor(ctx, command);
+      TransactionEntry te = ctx.getTransactionEntry();
+      if (te.hasLocalModifications())
+      {
+         PrepareCommand replicablePrepareCommand = command.clone(); // makre sure we remove any "local" transactions
+         replicablePrepareCommand.removeModifications(te.getLocalModifications());
+         command = replicablePrepareCommand;
+      }
+
       if (!skipReplicationOfTransactionMethod(ctx)) runPreparePhase(command, command.getGlobalTransaction(), ctx);
       return retVal;
    }
@@ -74,6 +84,13 @@
          {
             ctx.getTransactionEntry().setForceAsyncReplication(true);
          }
+
+         if (ctx.getOptionOverrides().isCacheModeLocal())
+         {
+            if (log.isDebugEnabled()) log.debug("Local mode override detected, will not replicate this command.");
+            ctx.getTransactionEntry().addLocalModification(command);
+         }
+
          return returnValue;
       }
       else
@@ -130,17 +147,16 @@
                   configuration.getCacheMode() + ", exclude_self=" + true + ", timeout=" +
                   configuration.getSyncReplTimeout());
          }
-         if (!isSynchronous(ctx.getOptionOverrides()) || forceAsync)
+
+         replicateCall(ctx, command, !forceAsync && isSynchronous(ctx.getOptionOverrides()), ctx.getOptionOverrides());
+      }
+      else
+      {
+         if (ctx.getOptionOverrides().isCacheModeLocal())
          {
-            // 2. Replicate change to all *other* members (exclude self !)
-            replicateCall(ctx, command, false, ctx.getOptionOverrides());
+            if (log.isDebugEnabled()) log.debug("Local mode override detected, will not replicate this command.");
+            ctx.getTransactionEntry().addLocalModification((ReversibleCommand) command);
          }
-         else
-         {
-            // REVISIT Needs to exclude itself and apply the local change manually.
-            // This is needed such that transient field is modified properly in-VM.
-            replicateCall(ctx, command, true, ctx.getOptionOverrides());
-         }
       }
       return returnValue;
    }

Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/TxInterceptor.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/interceptors/TxInterceptor.java	2008-05-14 01:36:56 UTC (rev 5836)
+++ core/trunk/src/main/java/org/jboss/cache/interceptors/TxInterceptor.java	2008-05-14 09:59:57 UTC (rev 5837)
@@ -554,11 +554,10 @@
    /**
     * creates a commit()
     */
-   protected void runCommitPhase(InvocationContext ctx, GlobalTransaction gtx, List modifications, List clModifications, boolean onePhaseCommit)
+   protected void runCommitPhase(InvocationContext ctx, GlobalTransaction gtx, List modifications, boolean onePhaseCommit)
    {
       // set the hasMods flag in the invocation ctx.  This should not be replicated, just used locally by the interceptors.
       ctx.setTxHasMods(modifications != null && modifications.size() > 0);
-      ctx.setCacheLoaderHasMods(clModifications != null && clModifications.size() > 0);
       try
       {
          VisitableCommand commitCommand = onePhaseCommit ? buildPrepareCommand(gtx, modifications, true) : commandsFactory.buildCommitCommand(gtx);
@@ -950,13 +949,11 @@
 
             if (trace) log.trace("calling aftercompletion for " + gtx);
 
-            List cacheLoaderModifications = null;
             // set any transaction wide options as current for this thread.
             if (entry != null)
             {
                // this should ideally be set in beforeCompletion(), after compacting the list.
                if (modifications == null) modifications = entry.getModifications();
-               cacheLoaderModifications = entry.getCacheLoaderModifications();
                ctx.setOptionOverrides(entry.getOption());
             }
             if (tx != null) transactions.remove(tx);
@@ -966,7 +963,7 @@
                case Status.STATUS_COMMITTED:
                   boolean onePhaseCommit = !configuration.getCacheMode().isSynchronous();
                   if (log.isDebugEnabled()) log.debug("Running commit phase.  One phase? " + onePhaseCommit);
-                  runCommitPhase(ctx, gtx, modifications, cacheLoaderModifications, onePhaseCommit);
+                  runCommitPhase(ctx, gtx, modifications, onePhaseCommit);
                   log.debug("Finished commit phase");
                   break;
                case Status.STATUS_UNKNOWN:

Modified: core/trunk/src/main/java/org/jboss/cache/transaction/TransactionEntry.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/transaction/TransactionEntry.java	2008-05-14 01:36:56 UTC (rev 5836)
+++ core/trunk/src/main/java/org/jboss/cache/transaction/TransactionEntry.java	2008-05-14 09:59:57 UTC (rev 5837)
@@ -28,7 +28,6 @@
 import java.util.LinkedList;
 import java.util.List;
 import java.util.ListIterator;
-import java.util.concurrent.CopyOnWriteArrayList;
 
 /**
  * Information associated with a {@link GlobalTransaction} about the transaction state.
@@ -66,12 +65,12 @@
    /**
     * List&lt;ReversibleCommand&gt; of modifications ({@link ReversibleCommand}). They will be replicated on TX commit
     */
-   private final List<ReversibleCommand> modificationList = new LinkedList<ReversibleCommand>();
+   private List<ReversibleCommand> modificationList;
+   /**
+    * A list of modifications that have been encountered with a LOCAL mode option.  These will be removed from the modification list during replication.
+    */
+   private List<ReversibleCommand> localModifications;
 
-   // For some reason we see multiple threads accessing this list - even within the same tx.  Could be due to reuse of
-   // tx identifiers in the DummyTM, which is where we see this problem.
-   private final List<ReversibleCommand> classLoadeModList = new CopyOnWriteArrayList<ReversibleCommand>();
-
    /**
     * LinkedHashSet<IdentityLock> of locks acquired by the transaction. We use
     * a LinkedHashSet because we need efficient Set semantics (same lock can
@@ -103,29 +102,40 @@
    public void addModification(ReversibleCommand command)
    {
       if (command == null) return;
+      if (modificationList == null) modificationList = new LinkedList<ReversibleCommand>();
       modificationList.add(command);
    }
 
-   public void addCacheLoaderModification(ReversibleCommand command)
-   {
-      if (command != null) classLoadeModList.add(command);
-   }
-
    /**
     * Returns all modifications.
     */
    public List<ReversibleCommand> getModifications()
    {
+      if (modificationList == null) return Collections.emptyList();
       return modificationList;
    }
 
-   public List<ReversibleCommand> getCacheLoaderModifications()
+   /**
+    * Adds a modification to the local modification list.
+    */
+   public void addLocalModification(ReversibleCommand command)
    {
-      // make sure this isn't modified externally
-      return Collections.unmodifiableList(classLoadeModList);
+      if (command == null) return;
+      if (localModifications == null) localModifications = new LinkedList<ReversibleCommand>();
+      localModifications.add(command);
    }
 
    /**
+    * Returns all modifications that have been invoked with the LOCAL cache mode option.  These will also be in the standard modification list.
+    */
+   public List<ReversibleCommand> getLocalModifications()
+   {
+      if (localModifications == null) return Collections.emptyList();
+      return localModifications;
+   }
+
+
+   /**
     * Adds the node that has been removed.
     *
     * @param fqn
@@ -391,17 +401,26 @@
     */
    public boolean hasModifications()
    {
-      return !modificationList.isEmpty() || !classLoadeModList.isEmpty();
+      return modificationList != null && !modificationList.isEmpty();
    }
 
    /**
+    * @return true if any modifications have been invoked with cache mode being LOCAL.
+    */
+   public boolean hasLocalModifications()
+   {
+      return localModifications != null && !localModifications.isEmpty();
+   }
+
+
+   /**
     * Cleans up internal state
     */
    public void reset()
    {
       orderedSynchronizationHandler = null;
-      modificationList.clear();
-      classLoadeModList.clear();
+      if (modificationList != null) modificationList.clear();
+      if (localModifications != null) localModifications.clear();
       option = null;
       locks.clear();
       if (dummyNodesCreatedByCacheLoader != null) dummyNodesCreatedByCacheLoader.clear();

Modified: core/trunk/src/main/java/org/jboss/cache/transaction/TransactionTable.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/transaction/TransactionTable.java	2008-05-14 01:36:56 UTC (rev 5836)
+++ core/trunk/src/main/java/org/jboss/cache/transaction/TransactionTable.java	2008-05-14 09:59:57 UTC (rev 5837)
@@ -11,7 +11,6 @@
 import org.jboss.cache.CacheException;
 import org.jboss.cache.InvocationContext;
 import org.jboss.cache.RPCManager;
-import org.jboss.cache.commands.ReversibleCommand;
 import org.jboss.cache.config.Configuration;
 import org.jboss.cache.factories.annotations.Inject;
 import org.jboss.cache.factories.annotations.NonVolatile;
@@ -201,34 +200,6 @@
    }
 
    /**
-    * Adds a motification to the global transaction.
-    */
-   public void addModification(GlobalTransaction gtx, ReversibleCommand m)
-   {
-      TransactionEntry entry = get(gtx);
-      if (entry == null)
-      {
-         log.error("transaction not found (globalTransaction=" + gtx + ")");
-         return;
-      }
-      entry.addModification(m);
-   }
-
-   public void addCacheLoaderModification(GlobalTransaction gtx, ReversibleCommand m)
-   {
-      if (m != null)
-      {
-         TransactionEntry entry = get(gtx);
-         if (entry == null)
-         {
-            log.error("transaction not found (globalTransaction=" + gtx + ")");
-            return;
-         }
-         entry.addCacheLoaderModification(m);
-      }
-   }
-
-   /**
     * Adds a lock to the global transaction.
     */
    public void addLock(GlobalTransaction gtx, NodeLock l)

Modified: core/trunk/src/test/java/org/jboss/cache/api/pfer/PutForExternalReadTestBase.java
===================================================================
--- core/trunk/src/test/java/org/jboss/cache/api/pfer/PutForExternalReadTestBase.java	2008-05-14 01:36:56 UTC (rev 5836)
+++ core/trunk/src/test/java/org/jboss/cache/api/pfer/PutForExternalReadTestBase.java	2008-05-14 09:59:57 UTC (rev 5837)
@@ -54,8 +54,6 @@
       cache1 = (CacheSPI<String, String>) cf.createCache(UnitTestCacheConfigurationFactory.createConfiguration(cacheMode), false);
       cache1.getConfiguration().setTransactionManagerLookupClass("org.jboss.cache.transaction.DummyTransactionManagerLookup");
       cache1.getConfiguration().setNodeLockingScheme(optimistic ? Configuration.NodeLockingScheme.OPTIMISTIC : Configuration.NodeLockingScheme.PESSIMISTIC);
-//      cache1.getConfiguration().setSyncCommitPhase(optimistic);
-//      cache1.getConfiguration().setSyncRollbackPhase(optimistic);
 
       cache1.start();
       tm1 = cache1.getConfiguration().getRuntimeConfig().getTransactionManager();
@@ -63,8 +61,6 @@
       cache2 = (CacheSPI<String, String>) cf.createCache(UnitTestCacheConfigurationFactory.createConfiguration(cacheMode), false);
       cache2.getConfiguration().setTransactionManagerLookupClass("org.jboss.cache.transaction.DummyTransactionManagerLookup");
       cache2.getConfiguration().setNodeLockingScheme(optimistic ? Configuration.NodeLockingScheme.OPTIMISTIC : Configuration.NodeLockingScheme.PESSIMISTIC);
-//      cache2.getConfiguration().setSyncCommitPhase(optimistic);
-//      cache2.getConfiguration().setSyncRollbackPhase(optimistic);
 
       cache2.start();
       tm2 = cache2.getConfiguration().getRuntimeConfig().getTransactionManager();

Modified: core/trunk/src/test/java/org/jboss/cache/invocationcontext/TransactionTest.java
===================================================================
--- core/trunk/src/test/java/org/jboss/cache/invocationcontext/TransactionTest.java	2008-05-14 01:36:56 UTC (rev 5836)
+++ core/trunk/src/test/java/org/jboss/cache/invocationcontext/TransactionTest.java	2008-05-14 09:59:57 UTC (rev 5837)
@@ -141,7 +141,7 @@
 
       // check that the transaction entry hasn't leaked stuff.
       assert entry.getModifications().isEmpty() : "Should have scrubbed modifications in transaction entry";
-      assert entry.getCacheLoaderModifications().isEmpty() : "Should have scrubbed modifications in transaction entry";
+      assert entry.getLocks().isEmpty() : "Should have scrubbed modifications in transaction entry";
       assert entry.getOrderedSynchronizationHandler() == null : "Should have removed the ordered sync handler";
    }
 

Modified: core/trunk/src/test/java/org/jboss/cache/options/cachemodelocal/CacheModeLocalTestBase.java
===================================================================
--- core/trunk/src/test/java/org/jboss/cache/options/cachemodelocal/CacheModeLocalTestBase.java	2008-05-14 01:36:56 UTC (rev 5836)
+++ core/trunk/src/test/java/org/jboss/cache/options/cachemodelocal/CacheModeLocalTestBase.java	2008-05-14 09:59:57 UTC (rev 5837)
@@ -536,7 +536,7 @@
       assertNull("should be null", cache2.get(fqn, key));
    }
 
-   public void testTransactionalBehaviour() throws Exception
+   public void testTransactionalBehaviourCommit() throws Exception
    {
       TransactionManager mgr = cache1.getTransactionManager();
       mgr.begin();
@@ -604,13 +604,13 @@
       cache2.getInvocationContext().getOptionOverrides().setCacheModeLocal(true);
       cache2.put(fqn, key, "value2");
       cache2.getInvocationContext().getOptionOverrides().reset();
-      cache2.put(fqn, key, "value2");
+      cache2.put(fqn, key, "value4");
       mgr.commit();
       delay();
-      assertEquals("value2", cache2.get(fqn, key));
+      assertEquals("value4", cache2.get(fqn, key));
       if (!isInvalidation)
       {
-         assertEquals("value2", cache1.get(fqn, key));
+         assertEquals("value4", cache1.get(fqn, key));
       }
       else
       {
@@ -619,6 +619,37 @@
 
    }
 
+   public void testTransactionalBehaviourRollback() throws Exception
+   {
+      TransactionManager mgr = cache1.getTransactionManager();
+
+      // create these first ...
+      cache1.put("/a", key, "old");
+      cache1.put("/b", key, "old");
+      delay();
+      mgr.begin();
+      cache1.getInvocationContext().getOptionOverrides().reset();
+      cache1.put("/a", key, "value1");
+      cache1.getInvocationContext().getOptionOverrides().setCacheModeLocal(true);
+      cache1.put("/b", key, "value2");
+      mgr.rollback();
+      delay();
+      // cache1 should NOT have this
+      assert cache1.get("/a", key).equals("old");
+      assert cache1.get("/b", key).equals("old");
+
+      if (isInvalidation)
+      {
+         assert cache2.get("/a", key) == null;
+         assert cache2.get("/b", key) == null;
+      }
+      else
+      {
+         assert cache2.get("/a", key).equals("old");
+         assert cache2.get("/b", key).equals("old");
+      }
+   }
+
    public void testTransactionalBehaviourViaNodeAPI() throws Exception
    {
       Node node1 = cache1.getRoot().addChild(fqn);
@@ -689,13 +720,13 @@
       cache2.getInvocationContext().getOptionOverrides().setCacheModeLocal(true);
       node2.put(key, "value2");
       cache2.getInvocationContext().getOptionOverrides().reset();
-      node2.put(key, "value2");
+      node2.put(key, "value4");
       mgr.commit();
       delay();
-      assertEquals("value2", cache2.get(fqn, key));
+      assertEquals("value4", cache2.get(fqn, key));
       if (!isInvalidation)
       {
-         assertEquals("value2", cache1.get(fqn, key));
+         assertEquals("value4", cache1.get(fqn, key));
       }
       else
       {




More information about the jbosscache-commits mailing list