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

Author: manik.surtani(a)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); } /**