Author: manik.surtani(a)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 {