Author: manik.surtani(a)jboss.com Date: 2008-05-28 10:49:35 -0400 (Wed, 28 May 2008) New Revision: 5896 Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/PessimisticLockInterceptor.java core/trunk/src/main/java/org/jboss/cache/lock/LockUtil.java core/trunk/src/main/java/org/jboss/cache/lock/PessimisticNodeBasedLockManager.java Log: Minor refactoring Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/PessimisticLockInterceptor.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/interceptors/PessimisticLockInterceptor.java 2008-05-28 13:38:24 UTC (rev 5895) +++ core/trunk/src/main/java/org/jboss/cache/interceptors/PessimisticLockInterceptor.java 2008-05-28 14:49:35 UTC (rev 5896) @@ -27,18 +27,21 @@ import org.jboss.cache.commands.write.PutKeyValueCommand; import org.jboss.cache.commands.write.RemoveKeyCommand; import org.jboss.cache.commands.write.RemoveNodeCommand; +import org.jboss.cache.factories.CommandsFactory; import org.jboss.cache.factories.annotations.Inject; import org.jboss.cache.interceptors.base.PostProcessingCommandInterceptor; import org.jboss.cache.lock.IsolationLevel; import org.jboss.cache.lock.LockManager; import static org.jboss.cache.lock.LockType.READ; import static org.jboss.cache.lock.LockType.WRITE; +import org.jboss.cache.lock.LockUtil; import org.jboss.cache.lock.PessimisticNodeBasedLockManager; import org.jboss.cache.transaction.GlobalTransaction; import org.jboss.cache.transaction.TransactionEntry; import java.util.LinkedList; import java.util.List; +import java.util.Map; /* * // TODO: 2.2.0: refactorings ideas @@ -61,12 +64,14 @@ { private DataContainerImpl dataContainer; private PessimisticNodeBasedLockManager lockManager; + private CommandsFactory commandsFactory; @Inject - public void injectDependencies(DataContainerImpl dataContainer, LockManager lockManager) + public void injectDependencies(DataContainerImpl dataContainer, LockManager lockManager, CommandsFactory commandsFactory) { this.dataContainer = dataContainer; this.lockManager = (PessimisticNodeBasedLockManager) lockManager; + this.commandsFactory = commandsFactory; } @Override @@ -101,7 +106,7 @@ Fqn childFqn = Fqn.fromElements(childName); NodeSPI childNode = n.getChildDirect(childFqn); if (childNode == null) childNode = n.addChildDirect(childFqn); - lockManager.manageReverseRemove(ctx, childNode, true, null); + LockUtil.manageReverseRemove(ctx, childNode, true, null, commandsFactory); n = childNode; } } @@ -222,7 +227,7 @@ } } - lockManager.lockAllForRemoval(dataContainer.peek(command.getFqn(), false, false), ctx, entry); + lockAllForRemoval(dataContainer.peek(command.getFqn(), false, false), ctx, entry); if (!createdNodes.isEmpty()) { @@ -248,6 +253,35 @@ return created ? false : retVal; } + /** + * Acquires write locks on the node and all child nodes, adding children to the list of removed nodes in the context. + * + * @param node node to inspect + * @param ctx invocation context + * @param entry transaction entry + * @throws InterruptedException in the event of interruption + */ + public void lockAllForRemoval(NodeSPI node, InvocationContext ctx, TransactionEntry entry) throws InterruptedException + { + if (node == null) return; + // lock node + lockManager.lockAndRecord(node, WRITE, ctx); + + // add to deleted list + if (entry != null) entry.addRemovedNode(node.getFqn()); + + // now children. + Map<Object, NodeSPI> children = node.getChildrenMapDirect(); + if (children != null) + { + for (NodeSPI child : children.values()) + { + // lock child. + lockAllForRemoval(child, ctx, entry); + } + } + } + @Override protected Object handleRemoveKeyCommand(InvocationContext ctx, RemoveKeyCommand command) throws Throwable { Modified: core/trunk/src/main/java/org/jboss/cache/lock/LockUtil.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/lock/LockUtil.java 2008-05-28 13:38:24 UTC (rev 5895) +++ core/trunk/src/main/java/org/jboss/cache/lock/LockUtil.java 2008-05-28 14:49:35 UTC (rev 5896) @@ -2,14 +2,22 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.jboss.cache.Fqn; +import org.jboss.cache.InvocationContext; import org.jboss.cache.NodeSPI; +import org.jboss.cache.commands.write.PutDataMapCommand; +import org.jboss.cache.factories.CommandsFactory; import org.jboss.cache.statetransfer.StateTransferManager; import org.jboss.cache.transaction.GlobalTransaction; +import org.jboss.cache.transaction.TransactionEntry; import org.jboss.cache.transaction.TransactionTable; import javax.transaction.Status; import javax.transaction.Transaction; import javax.transaction.TransactionManager; +import java.util.HashMap; +import java.util.List; +import java.util.Map; public abstract class LockUtil { @@ -199,4 +207,30 @@ return status; } + /** + * Test if this node needs to be 'undeleted' + * reverse the "remove" if the node has been previously removed in the same tx, if this operation is a put() + */ + public static void manageReverseRemove(InvocationContext ctx, NodeSPI childNode, boolean reverseRemoveCheck, List createdNodes, CommandsFactory commandsFactory) + { + if (ctx.getGlobalTransaction() != null) //if no tx then reverse remove does not make sense + { + Fqn fqn = childNode.getFqn(); + TransactionEntry entry = ctx.getTransactionEntry(); + boolean needToReverseRemove = reverseRemoveCheck && childNode.isDeleted() && entry != null && entry.getRemovedNodes().contains(fqn); + if (!needToReverseRemove) return; + childNode.markAsDeleted(false); + //if we'll rollback the tx data should be added to the node again + Map oldData = new HashMap(childNode.getDataDirect()); + PutDataMapCommand command = commandsFactory.buildPutDataMapCommand(ctx.getGlobalTransaction(), fqn, oldData); + // txTable.get(gtx).addUndoOperation(command); --- now need to make sure this is added to the normal mods list instead + entry.addModification(command); + //we're prepared for rollback, now reset the node + childNode.clearDataDirect(); + if (createdNodes != null) + { + createdNodes.add(childNode); + } + } + } } Modified: core/trunk/src/main/java/org/jboss/cache/lock/PessimisticNodeBasedLockManager.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/lock/PessimisticNodeBasedLockManager.java 2008-05-28 13:38:24 UTC (rev 5895) +++ core/trunk/src/main/java/org/jboss/cache/lock/PessimisticNodeBasedLockManager.java 2008-05-28 14:49:35 UTC (rev 5896) @@ -5,18 +5,13 @@ import org.jboss.cache.Fqn; import org.jboss.cache.InvocationContext; import org.jboss.cache.NodeSPI; -import org.jboss.cache.commands.write.PutDataMapCommand; import org.jboss.cache.factories.CommandsFactory; import org.jboss.cache.factories.annotations.Inject; import static org.jboss.cache.lock.LockType.WRITE; import org.jboss.cache.transaction.GlobalTransaction; -import org.jboss.cache.transaction.TransactionEntry; import org.jboss.cache.transaction.TransactionTable; -import java.util.HashMap; import java.util.List; -import java.util.Map; -import java.util.Set; /** * Contains specific methods for the PessimisticLockInterceptor. @@ -152,7 +147,7 @@ // actually acquire the lock we need. This method blocks. acquireNodeLock(ctx, currentNode, owner, gtx, lockTypeRequired, timeout); - manageReverseRemove(ctx, currentNode, reverseRemoveCheck, createdNodes); + LockUtil.manageReverseRemove(ctx, currentNode, reverseRemoveCheck, createdNodes, commandsFactory); // make sure the lock we acquired isn't on a deleted node/is an orphan!! // look into invalidated nodes as well NodeSPI repeek = dataContainer.peek(currentNodeFqn, true, true); @@ -255,62 +250,4 @@ } } } - - /** - * Test if this node needs to be 'undeleted' - * reverse the "remove" if the node has been previously removed in the same tx, if this operation is a put() - */ - public void manageReverseRemove(InvocationContext ctx, NodeSPI childNode, boolean reverseRemoveCheck, List createdNodes) - { - if (ctx.getGlobalTransaction() != null) //if no tx then reverse remove does not make sense - { - Fqn fqn = childNode.getFqn(); - TransactionEntry entry = ctx.getTransactionEntry(); - boolean needToReverseRemove = reverseRemoveCheck && childNode.isDeleted() && entry != null && entry.getRemovedNodes().contains(fqn); - if (!needToReverseRemove) return; - childNode.markAsDeleted(false); - //if we'll rollback the tx data should be added to the node again - Map oldData = new HashMap(childNode.getDataDirect()); - PutDataMapCommand command = commandsFactory.buildPutDataMapCommand(ctx.getGlobalTransaction(), fqn, oldData); - // txTable.get(gtx).addUndoOperation(command); --- now need to make sure this is added to the normal mods list instead - entry.addModification(command); - //we're prepared for rollback, now reset the node - childNode.clearDataDirect(); - if (createdNodes != null) - { - createdNodes.add(childNode); - } - } - } - - /** - * Acquires write locks on the node and all child nodes, adding children to the list of removed nodes in the context. - * - * @param node node to inspect - * @param ctx invocation context - * @param entry transaction entry - * @throws InterruptedException in the event of interruption - */ - public void lockAllForRemoval(NodeSPI node, InvocationContext ctx, TransactionEntry entry) throws InterruptedException - { - if (node == null) return; - - long timeout = ctx.getContextLockAcquisitionTimeout(lockAcquisitionTimeout); - GlobalTransaction gtx = ctx.getGlobalTransaction(); - Object owner = (gtx != null) ? gtx : Thread.currentThread(); - - Set<NodeLock> acquiredLocks = node.getLock().acquireAll(owner, timeout, WRITE); - if (acquiredLocks.size() > 0) - { - if (gtx != null) - { - ctx.getTransactionEntry().addLocks(acquiredLocks); - for (NodeLock l : acquiredLocks) entry.addRemovedNode(l.getFqn()); - } - else - { - ctx.addInvocationLocksAcquired(acquiredLocks); - } - } - } }