Author: manik.surtani(a)jboss.com Date: 2008-07-03 10:40:16 -0400 (Thu, 03 Jul 2008) New Revision: 6161 Added: core/trunk/src/test/java/org/jboss/cache/api/mvcc/LockAssert.java core/trunk/src/test/java/org/jboss/cache/api/mvcc/repeatable_read/RepeatableReadLockTest.java core/trunk/src/test/java/org/jboss/cache/api/mvcc/repeatable_read/WriteSkewTest.java Removed: core/trunk/src/test/java/org/jboss/cache/api/mvcc/repeatable_read/RepeatableReadNoWriteSkewLockTest.java core/trunk/src/test/java/org/jboss/cache/api/mvcc/repeatable_read/RepeatableReadWriteSkewLockTest.java Modified: core/trunk/src/main/java/org/jboss/cache/commands/legacy/write/PessRemoveNodeCommand.java core/trunk/src/main/java/org/jboss/cache/commands/write/MoveCommand.java core/trunk/src/main/java/org/jboss/cache/commands/write/RemoveNodeCommand.java core/trunk/src/main/java/org/jboss/cache/interceptors/MVCCLockingInterceptor.java core/trunk/src/main/java/org/jboss/cache/mvcc/ReadCommittedNode.java core/trunk/src/main/java/org/jboss/cache/mvcc/RepeatableReadNode.java core/trunk/src/test/java/org/jboss/cache/api/CacheAPITest.java core/trunk/src/test/java/org/jboss/cache/api/NodeMoveAPITest.java core/trunk/src/test/java/org/jboss/cache/api/mvcc/LockTestBase.java core/trunk/src/test/java/org/jboss/cache/api/mvcc/read_committed/NodeMoveMvccTest.java core/trunk/src/test/java/org/jboss/cache/api/mvcc/repeatable_read/NodeMoveMvccTest.java core/trunk/src/test/java/org/jboss/cache/commands/legacy/write/RemoveNodeCommandTest.java Log: Lots more MVCC fixes Modified: core/trunk/src/main/java/org/jboss/cache/commands/legacy/write/PessRemoveNodeCommand.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/commands/legacy/write/PessRemoveNodeCommand.java 2008-07-03 14:05:31 UTC (rev 6160) +++ core/trunk/src/main/java/org/jboss/cache/commands/legacy/write/PessRemoveNodeCommand.java 2008-07-03 14:40:16 UTC (rev 6161) @@ -33,6 +33,12 @@ @Override public Object perform(InvocationContext ctx) { + NodeSPI targetNode = peekVersioned(ctx); + if (targetNode != null) + { + Map data = targetNode.getDataDirect(); + if (data != null && !data.isEmpty()) originalData = new HashMap(data); + } boolean found = (Boolean) super.perform(ctx); // now record rollback info. Modified: core/trunk/src/main/java/org/jboss/cache/commands/write/MoveCommand.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/commands/write/MoveCommand.java 2008-07-03 14:05:31 UTC (rev 6160) +++ core/trunk/src/main/java/org/jboss/cache/commands/write/MoveCommand.java 2008-07-03 14:40:16 UTC (rev 6161) @@ -102,7 +102,7 @@ if (trace) log.trace("Moving " + fqn + " to sit under " + to); - NodeSPI oldParent = (NodeSPI) node.getParent(); + NodeSPI oldParent = node.getParentDirect(); Object nodeName = toMoveFqn.getLastElement(); // now that we have the parent and target nodes: Modified: core/trunk/src/main/java/org/jboss/cache/commands/write/RemoveNodeCommand.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/commands/write/RemoveNodeCommand.java 2008-07-03 14:05:31 UTC (rev 6160) +++ core/trunk/src/main/java/org/jboss/cache/commands/write/RemoveNodeCommand.java 2008-07-03 14:40:16 UTC (rev 6161) @@ -56,6 +56,9 @@ boolean found = targetNode.isValid() && !targetNode.isDeleted(); targetNode.markAsDeleted(true, true); + // make sure we clear all data on this node! + targetNode.clearDataDirect(); + notifyAfterRemove(ctx); return found; } Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/MVCCLockingInterceptor.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/interceptors/MVCCLockingInterceptor.java 2008-07-03 14:05:31 UTC (rev 6160) +++ core/trunk/src/main/java/org/jboss/cache/interceptors/MVCCLockingInterceptor.java 2008-07-03 14:40:16 UTC (rev 6161) @@ -169,6 +169,8 @@ if (rcn != null) { rcn.copyNodeForUpdate(dataContainer, allowWriteSkew, ctx, nodeFactory); + ReadCommittedNode parent = (ReadCommittedNode) ctx.lookUpNode(fqn.getParent()); + parent.addChildDirect(nodeFactory.createNodeInvocationDelegate((InternalNode) rcn.getDelegationTarget())); if (isRecursive) { @@ -343,7 +345,7 @@ { // for each of these, swap refs ReadCommittedNode rcn = (ReadCommittedNode) ctx.lookUpNode(fqnsToUnlock[i]); - if (rcn != null) rcn.commitUpdate(dataContainer, nodeFactory); // could be null with read-committed + if (rcn != null) rcn.commitUpdate(dataContainer, nodeFactory, ctx); // could be null with read-committed // and then unlock lockManager.unlock(fqnsToUnlock[i], owner); } @@ -373,7 +375,7 @@ if (commit) { // for each of these, swap refs - rcn.commitUpdate(dataContainer, nodeFactory); + rcn.commitUpdate(dataContainer, nodeFactory, ctx); } else { @@ -431,7 +433,7 @@ InternalNode node = dataContainer.peekInternalNode(f, false); if (node != null) { - lockManager.lock(f, WRITE, ctx); + lock(ctx, f); NodeSPI wrapped = nodeFactory.createMvccNode(node); ctx.putLookedUpNode(f, wrapped); } @@ -467,6 +469,12 @@ n.copyNodeForUpdate(dataContainer, allowWriteSkew, context, nodeFactory); } if (trace) log.trace("Retrieving wrapped node " + fqn); + if (n.isDeleted() && createIfAbsent) + { + if (trace) log.trace("Node is deleted in current scope. Need to un-delete."); + n.markAsDeleted(false); + n.setValid(true, false); + } return n; } Modified: core/trunk/src/main/java/org/jboss/cache/mvcc/ReadCommittedNode.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/mvcc/ReadCommittedNode.java 2008-07-03 14:05:31 UTC (rev 6160) +++ core/trunk/src/main/java/org/jboss/cache/mvcc/ReadCommittedNode.java 2008-07-03 14:40:16 UTC (rev 6161) @@ -40,32 +40,25 @@ node.setVersion(newVersion); } - public void commitUpdate(DataContainer container, NodeFactory nf) + public void commitUpdate(DataContainer container, NodeFactory nf, InvocationContext ctx) { if (changed) { - updateNode(container, nf); + updateNode(container, nf, ctx); changed = false; backup = null; } } - protected void updateNode(DataContainer container, NodeFactory nf) + protected void updateNode(DataContainer container, NodeFactory nf, InvocationContext ctx) { if (isDeleted()) { Fqn fqn = getFqn(); if (!fqn.isRoot()) { - NodeSPI parent = container.peek(fqn.getParent()); - if (parent != null) - { - parent.removeChildDirect(fqn.getLastElement()); - } - else - { - if (trace) log.trace("Parent is null in the underlying cache; cannot remove."); - } + NodeSPI parent = ctx.lookUpNode(fqn.getParent()); // this will be there since parent nodes are locked when a child is removed. + parent.removeChildDirect(fqn.getLastElement()); } else { Modified: core/trunk/src/main/java/org/jboss/cache/mvcc/RepeatableReadNode.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/mvcc/RepeatableReadNode.java 2008-07-03 14:05:31 UTC (rev 6160) +++ core/trunk/src/main/java/org/jboss/cache/mvcc/RepeatableReadNode.java 2008-07-03 14:40:16 UTC (rev 6161) @@ -51,7 +51,7 @@ } @Override - protected void updateNode(DataContainer dataContainer, NodeFactory nf) + protected void updateNode(DataContainer dataContainer, NodeFactory nf, InvocationContext ctx) { if (trace) log.trace("Updating RepeatableReadNode. IsDeleted? " + isDeleted() + " isValid? " + isValid() + " isChanged? " + isChanged()); Modified: core/trunk/src/test/java/org/jboss/cache/api/CacheAPITest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/api/CacheAPITest.java 2008-07-03 14:05:31 UTC (rev 6160) +++ core/trunk/src/test/java/org/jboss/cache/api/CacheAPITest.java 2008-07-03 14:40:16 UTC (rev 6161) @@ -167,10 +167,14 @@ assertEquals(true, cache.removeNode(fqn)); assertFalse(cache.getRoot().hasChild(fqn)); + // remove should REALLY remove though and not just mark as deleted/invalid. + NodeSPI n = cache.peek(fqn, true, true); + assert n == null; + assertEquals(false, cache.removeNode(fqn)); // remove should REALLY remove though and not just mark as deleted/invalid. - NodeSPI n = cache.peek(fqn, true, true); + n = cache.peek(fqn, true, true); assert n == null; System.out.println("Cache: " + CachePrinter.printCacheDetails(cache)); Modified: core/trunk/src/test/java/org/jboss/cache/api/NodeMoveAPITest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/api/NodeMoveAPITest.java 2008-07-03 14:05:31 UTC (rev 6160) +++ core/trunk/src/test/java/org/jboss/cache/api/NodeMoveAPITest.java 2008-07-03 14:40:16 UTC (rev 6161) @@ -50,6 +50,8 @@ // start a single cache instance cache = (CacheSPI<Object, Object>) new DefaultCacheFactory<Object, Object>().createCache("configs/local-tx.xml", false); cache.getConfiguration().setNodeLockingScheme(nodeLockingScheme); + cache.getConfiguration().setFetchInMemoryState(false); + cache.getConfiguration().setEvictionConfig(null); configure(cache.getConfiguration()); cache.start(); rootNode = cache.getRoot(); @@ -401,14 +403,22 @@ cache.move(nodeC.getFqn(), nodeB.getFqn()); - assertEquals("ROOT should have a RL, nodeC should have a RL, nodeA should have a RL, nodeB should have a WL", 4, cache.getNumberOfLocksHeld()); + checkLocks(); tm.commit(); + assertNoLocks(); + } + protected void checkLocks() + { + assertEquals("ROOT should have a RL, nodeC should have a RL, nodeA should have a RL, nodeB should have a WL", 4, cache.getNumberOfLocksHeld()); + } + + protected void assertNoLocks() + { assertEquals(0, cache.getNumberOfLocksHeld()); } - @Test(groups = {"functional"}) public void testConcurrency() throws InterruptedException { Added: core/trunk/src/test/java/org/jboss/cache/api/mvcc/LockAssert.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/api/mvcc/LockAssert.java (rev 0) +++ core/trunk/src/test/java/org/jboss/cache/api/mvcc/LockAssert.java 2008-07-03 14:40:16 UTC (rev 6161) @@ -0,0 +1,36 @@ +package org.jboss.cache.api.mvcc; + +import org.jboss.cache.Fqn; +import org.jboss.cache.invocation.InvocationContextContainer; +import org.jboss.cache.lock.LockManager; +import org.jboss.cache.lock.MVCCLockManager.LockContainer; +import org.jboss.cache.util.TestingUtil; + +/** + * Helper class to assert lock status in MVCC + * + * @author Manik Surtani (<a href="mailto:manik@jboss.org">manik@jboss.org</a>) + * @since 3.0 + */ +public class LockAssert +{ + public static void assertLocked(Fqn fqn, LockManager lockManager, InvocationContextContainer icc) + { + assert lockManager.isLocked(fqn) : fqn + " not locked!"; + assert icc.get().getLocks().contains(fqn) : "Lock not recorded for " + fqn; + } + + public static void assertNotLocked(Fqn fqn, LockManager lockManager, InvocationContextContainer icc) + { + // can't rely on the negative test since other nodes may share the same lock with lock striping. +// assert !lockManager.isLocked(fqn) : fqn + " is locked!"; + assert !icc.get().getLocks().contains(fqn) : fqn + " lock recorded!"; + } + + public static void assertNoLocks(LockManager lockManager, InvocationContextContainer icc) + { + LockContainer lc = (LockContainer) TestingUtil.extractField(lockManager, "lockContainer"); + assert lc.getNumLocksHeld() == 0 : "Stale locks exist! NumLocksHeld is " + lc.getNumLocksHeld() + " and lock info is " + lockManager.printLockInfo(); + assert icc.get().getLocks().isEmpty() : "Stale (?) locks recorded! " + icc.get().getLocks(); + } +} Modified: core/trunk/src/test/java/org/jboss/cache/api/mvcc/LockTestBase.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/api/mvcc/LockTestBase.java 2008-07-03 14:05:31 UTC (rev 6160) +++ core/trunk/src/test/java/org/jboss/cache/api/mvcc/LockTestBase.java 2008-07-03 14:40:16 UTC (rev 6161) @@ -9,7 +9,6 @@ import org.jboss.cache.invocation.InvocationContextContainer; import org.jboss.cache.lock.IsolationLevel; import org.jboss.cache.lock.LockManager; -import org.jboss.cache.lock.MVCCLockManager.LockContainer; import org.jboss.cache.lock.TimeoutException; import org.jboss.cache.transaction.DummyTransactionManagerLookup; import org.jboss.cache.util.TestingUtil; @@ -17,14 +16,9 @@ import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; -import javax.transaction.SystemException; import javax.transaction.Transaction; import javax.transaction.TransactionManager; -import java.util.Collection; import java.util.Collections; -import java.util.HashSet; -import java.util.Set; -import java.util.concurrent.CountDownLatch; /** * @author Manik Surtani (<a href="mailto:manik@jboss.org">manik@jboss.org</a>) @@ -42,7 +36,6 @@ protected LockManager lockManager; protected InvocationContextContainer icc; protected boolean repeatableRead = true; - protected boolean allowWriteSkew = false; @BeforeMethod public void setUp() @@ -51,7 +44,6 @@ cache.getConfiguration().setNodeLockingScheme(NodeLockingScheme.MVCC); cache.getConfiguration().setTransactionManagerLookupClass(DummyTransactionManagerLookup.class.getName()); cache.getConfiguration().setIsolationLevel(repeatableRead ? IsolationLevel.REPEATABLE_READ : IsolationLevel.READ_COMMITTED); - cache.getConfiguration().setAllowWriteSkew(allowWriteSkew); // reduce lock acquisition timeout so this doesn't take forever to run cache.getConfiguration().setLockAcquisitionTimeout(200); // 200 ms cache.start(); @@ -68,22 +60,17 @@ protected void assertLocked(Fqn fqn) { - assert lockManager.isLocked(fqn) : fqn + " not locked!"; - assert icc.get().getLocks().contains(fqn) : "Lock not recorded for " + fqn; + LockAssert.assertLocked(fqn, lockManager, icc); } protected void assertNotLocked(Fqn fqn) { - // can't rely on the negative test since other nodes may share the same lock with lock striping. -// assert !lockManager.isLocked(fqn) : fqn + " is locked!"; - assert !icc.get().getLocks().contains(fqn) : fqn + " lock recorded!"; + LockAssert.assertNotLocked(fqn, lockManager, icc); } protected void assertNoLocks() { - LockContainer lc = (LockContainer) TestingUtil.extractField(lockManager, "lockContainer"); - assert lc.getNumLocksHeld() == 0 : "Stale locks exist! NumLocksHeld is " + lc.getNumLocksHeld() + " and lock info is " + lockManager.printLockInfo(); - assert icc.get().getLocks().isEmpty() : "Stale (?) locks recorded! " + icc.get().getLocks(); + LockAssert.assertNoLocks(lockManager, icc); } public void testLocksOnPutKeyVal() throws Exception @@ -463,120 +450,4 @@ assert null == cache.getNode(AB); assertNoLocks(); } - - public void testWriteSkew() throws Exception - { - if (repeatableRead) - { - cache.put(AB, "k", "v"); - final Set<Exception> w1exceptions = new HashSet<Exception>(); - final Set<Exception> w2exceptions = new HashSet<Exception>(); - final CountDownLatch w1Signal = new CountDownLatch(1); - final CountDownLatch w2Signal = new CountDownLatch(1); - final CountDownLatch threadSignal = new CountDownLatch(2); - - Thread w1 = new Thread("Writer-1") - { - public void run() - { - boolean didCoundDown = false; - try - { - tm.begin(); - assert "v".equals(cache.get(AB, "k")); - threadSignal.countDown(); - didCoundDown = true; - w1Signal.await(); - cache.put(AB, "k", "v2"); - tm.commit(); - } - catch (Exception e) - { - w1exceptions.add(e); - } - finally - { - if (!didCoundDown) threadSignal.countDown(); - } - } - }; - - Thread w2 = new Thread("Writer-2") - { - public void run() - { - boolean didCoundDown = false; - try - { - tm.begin(); - assert "v".equals(cache.get(AB, "k")); - threadSignal.countDown(); - didCoundDown = true; - w2Signal.await(); - cache.put(AB, "k", "v3"); - tm.commit(); - } - catch (Exception e) - { - w2exceptions.add(e); - // the exception will be thrown when doing a cache.put(). We should make sure we roll back the tx to release locks. - if (!allowWriteSkew) - { - try - { - tm.rollback(); - } - catch (SystemException e1) - { - // do nothing. - } - } - } - finally - { - if (!didCoundDown) threadSignal.countDown(); - } - } - }; - - w1.start(); - w2.start(); - - threadSignal.await(); - // now. both txs have read. - // let tx1 start writing - w1Signal.countDown(); - w1.join(); - - w2Signal.countDown(); - w2.join(); - - if (allowWriteSkew) - { - // should have no exceptions!! - throwExceptions(w1exceptions, w2exceptions); - assert w2exceptions.size() == 0; - assert w1exceptions.size() == 0; - assert "v3".equals(cache.get(AB, "k")) : "W2 should have overwritten W1's work!"; - } - else - { - // there should be a single exception from w2. - assert w2exceptions.size() == 1; - throwExceptions(w1exceptions); - assert w1exceptions.size() == 0; - assert "v2".equals(cache.get(AB, "k")) : "W2 should NOT have overwritten W1's work!"; - } - - assertNoLocks(); - } - } - - protected void throwExceptions(Collection<Exception>... exceptions) throws Exception - { - for (Collection<Exception> ce : exceptions) - { - for (Exception e : ce) throw e; - } - } } Modified: core/trunk/src/test/java/org/jboss/cache/api/mvcc/read_committed/NodeMoveMvccTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/api/mvcc/read_committed/NodeMoveMvccTest.java 2008-07-03 14:05:31 UTC (rev 6160) +++ core/trunk/src/test/java/org/jboss/cache/api/mvcc/read_committed/NodeMoveMvccTest.java 2008-07-03 14:40:16 UTC (rev 6161) @@ -6,10 +6,16 @@ */ package org.jboss.cache.api.mvcc.read_committed; +import org.jboss.cache.Fqn; import org.jboss.cache.api.NodeMoveAPITest; +import org.jboss.cache.api.mvcc.LockAssert; import org.jboss.cache.config.Configuration; import org.jboss.cache.config.Configuration.NodeLockingScheme; +import org.jboss.cache.factories.ComponentRegistry; +import org.jboss.cache.invocation.InvocationContextContainer; import org.jboss.cache.lock.IsolationLevel; +import org.jboss.cache.lock.LockManager; +import org.jboss.cache.util.TestingUtil; import org.testng.annotations.Test; @Test(groups = {"functional", "mvcc"}) @@ -25,4 +31,27 @@ { c.setIsolationLevel(IsolationLevel.READ_COMMITTED); } + + @Override + protected void checkLocks() + { + ComponentRegistry cr = TestingUtil.extractComponentRegistry(cache); + LockManager lm = cr.getComponent(LockManager.class); + InvocationContextContainer icc = cr.getComponent(InvocationContextContainer.class); + + LockAssert.assertNotLocked(A, lm, icc); + LockAssert.assertLocked(Fqn.ROOT, lm, icc); + LockAssert.assertLocked(C, lm, icc); + LockAssert.assertLocked(Fqn.fromRelativeFqn(A, B), lm, icc); + LockAssert.assertLocked(Fqn.fromRelativeFqn(B, C), lm, icc); + } + + @Override + protected void assertNoLocks() + { + ComponentRegistry cr = TestingUtil.extractComponentRegistry(cache); + LockManager lm = cr.getComponent(LockManager.class); + InvocationContextContainer icc = cr.getComponent(InvocationContextContainer.class); + LockAssert.assertNoLocks(lm, icc); + } } \ No newline at end of file Modified: core/trunk/src/test/java/org/jboss/cache/api/mvcc/repeatable_read/NodeMoveMvccTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/api/mvcc/repeatable_read/NodeMoveMvccTest.java 2008-07-03 14:05:31 UTC (rev 6160) +++ core/trunk/src/test/java/org/jboss/cache/api/mvcc/repeatable_read/NodeMoveMvccTest.java 2008-07-03 14:40:16 UTC (rev 6161) @@ -6,10 +6,16 @@ */ package org.jboss.cache.api.mvcc.repeatable_read; +import org.jboss.cache.Fqn; import org.jboss.cache.api.NodeMoveAPITest; +import org.jboss.cache.api.mvcc.LockAssert; import org.jboss.cache.config.Configuration; import org.jboss.cache.config.Configuration.NodeLockingScheme; +import org.jboss.cache.factories.ComponentRegistry; +import org.jboss.cache.invocation.InvocationContextContainer; import org.jboss.cache.lock.IsolationLevel; +import org.jboss.cache.lock.LockManager; +import org.jboss.cache.util.TestingUtil; import org.testng.annotations.Test; @Test(groups = {"functional", "mvcc"}) @@ -25,4 +31,27 @@ { c.setIsolationLevel(IsolationLevel.REPEATABLE_READ); } + + @Override + protected void checkLocks() + { + ComponentRegistry cr = TestingUtil.extractComponentRegistry(cache); + LockManager lm = cr.getComponent(LockManager.class); + InvocationContextContainer icc = cr.getComponent(InvocationContextContainer.class); + + LockAssert.assertNotLocked(A, lm, icc); + LockAssert.assertLocked(Fqn.ROOT, lm, icc); + LockAssert.assertLocked(C, lm, icc); + LockAssert.assertLocked(Fqn.fromRelativeFqn(A, B), lm, icc); + LockAssert.assertLocked(Fqn.fromRelativeFqn(B, C), lm, icc); + } + + @Override + protected void assertNoLocks() + { + ComponentRegistry cr = TestingUtil.extractComponentRegistry(cache); + LockManager lm = cr.getComponent(LockManager.class); + InvocationContextContainer icc = cr.getComponent(InvocationContextContainer.class); + LockAssert.assertNoLocks(lm, icc); + } } Copied: core/trunk/src/test/java/org/jboss/cache/api/mvcc/repeatable_read/RepeatableReadLockTest.java (from rev 6157, core/trunk/src/test/java/org/jboss/cache/api/mvcc/repeatable_read/RepeatableReadTestBase.java) =================================================================== --- core/trunk/src/test/java/org/jboss/cache/api/mvcc/repeatable_read/RepeatableReadLockTest.java (rev 0) +++ core/trunk/src/test/java/org/jboss/cache/api/mvcc/repeatable_read/RepeatableReadLockTest.java 2008-07-03 14:40:16 UTC (rev 6161) @@ -0,0 +1,90 @@ +package org.jboss.cache.api.mvcc.repeatable_read; + +import org.jboss.cache.api.mvcc.LockTestBase; +import org.testng.annotations.Test; + +import javax.transaction.Transaction; + +@Test(groups = {"functional", "mvcc"}) +public class RepeatableReadLockTest extends LockTestBase +{ + public RepeatableReadLockTest() + { + repeatableRead = true; + } + + public void testRepeatableReadWithRemove() throws Exception + { + cache.put(AB, "k", "v"); + + tm.begin(); + assert cache.getNode(AB) != null; + Transaction reader = tm.suspend(); + + tm.begin(); + assert cache.removeNode(AB); + assert cache.getNode(AB) == null; + tm.commit(); + + assert cache.getNode(AB) == null; + + tm.resume(reader); + assert cache.getNode(AB) != null; + assert "v".equals(cache.get(AB, "k")); + tm.commit(); + + assert cache.getNode(AB) == null; + assertNoLocks(); + } + + public void testRepeatableReadWithEvict() throws Exception + { + cache.put(AB, "k", "v"); + + tm.begin(); + assert cache.getNode(AB) != null; + Transaction reader = tm.suspend(); + + tm.begin(); + cache.evict(AB); + assert cache.getNode(AB) == null; + tm.commit(); + + assert cache.getNode(AB) == null; + + tm.resume(reader); + assert cache.getNode(AB) != null; + assert "v".equals(cache.get(AB, "k")); + tm.commit(); + + assert cache.getNode(AB) == null; + assertNoLocks(); + } + + public void testRepeatableReadWithNull() throws Exception + { + assert cache.getNode(AB) == null; + + tm.begin(); + assert cache.getNode(AB) == null; + Transaction reader = tm.suspend(); + + tm.begin(); + cache.put(AB, "k", "v"); + assert cache.getNode(AB) != null; + assert "v".equals(cache.get(AB, "k")); + tm.commit(); + + assert cache.getNode(AB) != null; + assert "v".equals(cache.get(AB, "k")); + + tm.resume(reader); + assert cache.getNode(AB) == null; + assert cache.get(AB, "k") == null; + tm.commit(); + + assert cache.getNode(AB) != null; + assert "v".equals(cache.get(AB, "k")); + assertNoLocks(); + } +} Deleted: core/trunk/src/test/java/org/jboss/cache/api/mvcc/repeatable_read/RepeatableReadNoWriteSkewLockTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/api/mvcc/repeatable_read/RepeatableReadNoWriteSkewLockTest.java 2008-07-03 14:05:31 UTC (rev 6160) +++ core/trunk/src/test/java/org/jboss/cache/api/mvcc/repeatable_read/RepeatableReadNoWriteSkewLockTest.java 2008-07-03 14:40:16 UTC (rev 6161) @@ -1,12 +0,0 @@ -package org.jboss.cache.api.mvcc.repeatable_read; - -import org.testng.annotations.Test; - -@Test(groups = {"functional", "mvcc"}) -public class RepeatableReadNoWriteSkewLockTest extends RepeatableReadTestBase -{ - public RepeatableReadNoWriteSkewLockTest() - { - allowWriteSkew = false; - } -} Deleted: core/trunk/src/test/java/org/jboss/cache/api/mvcc/repeatable_read/RepeatableReadWriteSkewLockTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/api/mvcc/repeatable_read/RepeatableReadWriteSkewLockTest.java 2008-07-03 14:05:31 UTC (rev 6160) +++ core/trunk/src/test/java/org/jboss/cache/api/mvcc/repeatable_read/RepeatableReadWriteSkewLockTest.java 2008-07-03 14:40:16 UTC (rev 6161) @@ -1,12 +0,0 @@ -package org.jboss.cache.api.mvcc.repeatable_read; - -import org.testng.annotations.Test; - -@Test(groups = {"functional", "mvcc"}) -public class RepeatableReadWriteSkewLockTest extends RepeatableReadTestBase -{ - public RepeatableReadWriteSkewLockTest() - { - allowWriteSkew = true; - } -} Added: core/trunk/src/test/java/org/jboss/cache/api/mvcc/repeatable_read/WriteSkewTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/api/mvcc/repeatable_read/WriteSkewTest.java (rev 0) +++ core/trunk/src/test/java/org/jboss/cache/api/mvcc/repeatable_read/WriteSkewTest.java 2008-07-03 14:40:16 UTC (rev 6161) @@ -0,0 +1,199 @@ +package org.jboss.cache.api.mvcc.repeatable_read; + +import org.jboss.cache.Cache; +import org.jboss.cache.DefaultCacheFactory; +import org.jboss.cache.Fqn; +import org.jboss.cache.api.mvcc.LockAssert; +import org.jboss.cache.config.Configuration.CacheMode; +import org.jboss.cache.config.Configuration.NodeLockingScheme; +import org.jboss.cache.factories.UnitTestCacheConfigurationFactory; +import org.jboss.cache.invocation.InvocationContextContainer; +import org.jboss.cache.lock.IsolationLevel; +import org.jboss.cache.lock.LockManager; +import org.jboss.cache.transaction.DummyTransactionManagerLookup; +import org.jboss.cache.util.TestingUtil; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import javax.transaction.SystemException; +import javax.transaction.TransactionManager; +import java.util.Collection; +import java.util.HashSet; +import java.util.Set; +import java.util.concurrent.CountDownLatch; + +@Test(groups = {"functional", "mvcc"}) +public class WriteSkewTest +{ + protected Cache<String, String> cache; + protected TransactionManager tm; + protected Fqn A = Fqn.fromString("/a"); + protected Fqn AB = Fqn.fromString("/a/b"); + protected Fqn ABC = Fqn.fromString("/a/b/c"); + protected Fqn ABCD = Fqn.fromString("/a/b/c/d"); + protected LockManager lockManager; + protected InvocationContextContainer icc; + protected boolean repeatableRead = true; + + @BeforeMethod + public void setUp() + { + cache = new DefaultCacheFactory<String, String>().createCache(UnitTestCacheConfigurationFactory.createConfiguration(CacheMode.LOCAL), false); + cache.getConfiguration().setNodeLockingScheme(NodeLockingScheme.MVCC); + cache.getConfiguration().setTransactionManagerLookupClass(DummyTransactionManagerLookup.class.getName()); + cache.getConfiguration().setIsolationLevel(repeatableRead ? IsolationLevel.REPEATABLE_READ : IsolationLevel.READ_COMMITTED); + // reduce lock acquisition timeout so this doesn't take forever to run + cache.getConfiguration().setLockAcquisitionTimeout(200); // 200 ms + } + + @AfterMethod + public void tearDown() + { + TestingUtil.killCaches(cache); + } + + private void postStart() + { + lockManager = TestingUtil.extractComponentRegistry(cache).getComponent(LockManager.class); + icc = TestingUtil.extractComponentRegistry(cache).getComponent(InvocationContextContainer.class); + tm = TestingUtil.extractComponentRegistry(cache).getComponent(TransactionManager.class); + } + + protected void assertNoLocks() + { + LockAssert.assertNoLocks(lockManager, icc); + } + + public void testAllowWriteSkew() throws Exception + { + cache.getConfiguration().setAllowWriteSkew(true); + cache.start(); + postStart(); + doTest(true); + } + + public void testDontAllowWriteSkew() throws Exception + { + cache.getConfiguration().setAllowWriteSkew(false); + cache.start(); + postStart(); + doTest(false); + } + + private void doTest(final boolean allowWriteSkew) throws Exception + { + if (repeatableRead) + { + cache.put(AB, "k", "v"); + final Set<Exception> w1exceptions = new HashSet<Exception>(); + final Set<Exception> w2exceptions = new HashSet<Exception>(); + final CountDownLatch w1Signal = new CountDownLatch(1); + final CountDownLatch w2Signal = new CountDownLatch(1); + final CountDownLatch threadSignal = new CountDownLatch(2); + + Thread w1 = new Thread("Writer-1") + { + public void run() + { + boolean didCoundDown = false; + try + { + tm.begin(); + assert "v".equals(cache.get(AB, "k")); + threadSignal.countDown(); + didCoundDown = true; + w1Signal.await(); + cache.put(AB, "k", "v2"); + tm.commit(); + } + catch (Exception e) + { + w1exceptions.add(e); + } + finally + { + if (!didCoundDown) threadSignal.countDown(); + } + } + }; + + Thread w2 = new Thread("Writer-2") + { + public void run() + { + boolean didCoundDown = false; + try + { + tm.begin(); + assert "v".equals(cache.get(AB, "k")); + threadSignal.countDown(); + didCoundDown = true; + w2Signal.await(); + cache.put(AB, "k", "v3"); + tm.commit(); + } + catch (Exception e) + { + w2exceptions.add(e); + // the exception will be thrown when doing a cache.put(). We should make sure we roll back the tx to release locks. + if (!allowWriteSkew) + { + try + { + tm.rollback(); + } + catch (SystemException e1) + { + // do nothing. + } + } + } + finally + { + if (!didCoundDown) threadSignal.countDown(); + } + } + }; + + w1.start(); + w2.start(); + + threadSignal.await(); + // now. both txs have read. + // let tx1 start writing + w1Signal.countDown(); + w1.join(); + + w2Signal.countDown(); + w2.join(); + + if (allowWriteSkew) + { + // should have no exceptions!! + throwExceptions(w1exceptions, w2exceptions); + assert w2exceptions.size() == 0; + assert w1exceptions.size() == 0; + assert "v3".equals(cache.get(AB, "k")) : "W2 should have overwritten W1's work!"; + } + else + { + // there should be a single exception from w2. + assert w2exceptions.size() == 1; + throwExceptions(w1exceptions); + assert w1exceptions.size() == 0; + assert "v2".equals(cache.get(AB, "k")) : "W2 should NOT have overwritten W1's work!"; + } + + assertNoLocks(); + } + } + + private void throwExceptions(Collection<Exception>... exceptions) throws Exception + { + for (Collection<Exception> ce : exceptions) + { + for (Exception e : ce) throw e; + } + } +} Modified: core/trunk/src/test/java/org/jboss/cache/commands/legacy/write/RemoveNodeCommandTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/commands/legacy/write/RemoveNodeCommandTest.java 2008-07-03 14:05:31 UTC (rev 6160) +++ core/trunk/src/test/java/org/jboss/cache/commands/legacy/write/RemoveNodeCommandTest.java 2008-07-03 14:40:16 UTC (rev 6161) @@ -27,6 +27,8 @@ public void testNonExistentNode() { expect(container.peek(fqn)).andReturn(null); + // again + expect(container.peek(fqn)).andReturn(null); control.replay(); assert Boolean.FALSE == command.perform(ctx) : "nonexistent node was not remove; false expected"; } @@ -41,6 +43,8 @@ //check perform expect(container.peek(fqn)).andReturn(nodes.adfNode); + // again + expect(container.peek(fqn)).andReturn(nodes.adfNode); control.replay(); assert Boolean.TRUE == command.perform(ctx); assert nodes.adfgNode.isDeleted(); @@ -64,6 +68,8 @@ nodes.adfNode.setVersion(dataVersion); expect(container.peek(fqn)).andReturn(nodes.adfNode); + // again + expect(container.peek(fqn)).andReturn(nodes.adfNode); control.replay(); assert Boolean.FALSE == command.perform(ctx); assert nodes.adfgNode.isDeleted(); @@ -77,6 +83,8 @@ nodes.adfNode.setVersion(dataVersion); expect(container.peek(fqn)).andReturn(nodes.adfNode); + // again + expect(container.peek(fqn)).andReturn(nodes.adfNode); notifier.notifyNodeRemoved(fqn, true, nodes.adfNode.getDataDirect(), ctx); notifier.notifyNodeRemoved(fqn, false, null, ctx); control.replay();