Author: manik.surtani(a)jboss.com Date: 2007-12-11 12:07:21 -0500 (Tue, 11 Dec 2007) New Revision: 4828 Added: core/trunk/src/test/java/org/jboss/cache/optimistic/OptimisticLockInterceptorTest.java Removed: core/trunk/src/test/java/org/jboss/cache/optimistic/OpLockingInterceptorTest.java Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticLockingInterceptor.java Log: JBCACHE-1228 - opt locking interceptor always acquires WRITE locks Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticLockingInterceptor.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticLockingInterceptor.java 2007-12-11 17:00:59 UTC (rev 4827) +++ core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticLockingInterceptor.java 2007-12-11 17:07:21 UTC (rev 4828) @@ -6,8 +6,14 @@ */ package org.jboss.cache.interceptors; -import org.jboss.cache.*; +import org.jboss.cache.CacheException; +import org.jboss.cache.CacheSPI; +import org.jboss.cache.Fqn; +import org.jboss.cache.InvocationContext; +import org.jboss.cache.NodeSPI; import org.jboss.cache.lock.NodeLock; +import static org.jboss.cache.lock.NodeLock.LockType.READ; +import static org.jboss.cache.lock.NodeLock.LockType.WRITE; import org.jboss.cache.optimistic.TransactionWorkspace; import org.jboss.cache.optimistic.WorkspaceNode; import org.jboss.cache.transaction.GlobalTransaction; @@ -48,16 +54,20 @@ try { TransactionWorkspace<?, ?> workspace = getTransactionWorkspace(gtx); + TransactionEntry te = cache.getTransactionTable().get(gtx); if (log.isDebugEnabled()) log.debug("Locking nodes in transaction workspace for GlobalTransaction " + gtx); for (WorkspaceNode workspaceNode : workspace.getNodes().values()) { NodeSPI node = workspaceNode.getNode(); - boolean acquired = node.getLock().acquire(gtx, lockAcquisitionTimeout, NodeLock.LockType.WRITE); + + boolean isWriteLockNeeded = workspaceNode.isDirty() || (workspaceNode.isChildrenModified() && (configuration.isLockParentForChildInsertRemove() || node.isLockForChildInsertRemove())); + + boolean acquired = node.getLock().acquire(gtx, lockAcquisitionTimeout, isWriteLockNeeded ? WRITE : READ); if (acquired) { if (trace) log.trace("Acquired lock on node " + node.getFqn()); - cache.getTransactionTable().addLock(gtx, node.getLock()); + te.addLock(node.getLock()); } else { @@ -102,15 +112,14 @@ throw new CacheException("_lock() passed up the interceptor stack when Optimistic Locking is used. This is NOT supported."); } - private Object transactionFinalized(InvocationContext ctx) - throws Throwable + private Object transactionFinalized(InvocationContext ctx) throws Throwable { Object retval = null; // we need to let the stack run its commits or rollbacks first - // we unlock last - even if an exception occurs try - { - retval = nextInterceptor(ctx); + { + retval = nextInterceptor(ctx); } finally { Deleted: core/trunk/src/test/java/org/jboss/cache/optimistic/OpLockingInterceptorTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/optimistic/OpLockingInterceptorTest.java 2007-12-11 17:00:59 UTC (rev 4827) +++ core/trunk/src/test/java/org/jboss/cache/optimistic/OpLockingInterceptorTest.java 2007-12-11 17:07:21 UTC (rev 4828) @@ -1,366 +0,0 @@ -/* - * Created on 17-Feb-2005 - * - * - * - */ -package org.jboss.cache.optimistic; -import static org.testng.AssertJUnit.assertEquals; -import static org.testng.AssertJUnit.assertNotNull; -import static org.testng.AssertJUnit.assertTrue; -import static org.testng.AssertJUnit.fail; - -import java.util.HashMap; -import java.util.Iterator; -import java.util.Map; - -import javax.transaction.Transaction; - -import org.jboss.cache.CacheImpl; -import org.jboss.cache.Fqn; -import org.jboss.cache.NodeSPI; -import org.jboss.cache.interceptors.Interceptor; -import org.jboss.cache.interceptors.OptimisticCreateIfNotExistsInterceptor; -import org.jboss.cache.interceptors.OptimisticLockingInterceptor; -import org.jboss.cache.interceptors.OptimisticNodeInterceptor; -import org.jboss.cache.loader.SamplePojo; -import org.jboss.cache.lock.NodeLock; -import org.jboss.cache.marshall.MethodCall; -import org.jboss.cache.marshall.MethodCallFactory; -import org.jboss.cache.marshall.MethodDeclarations; -import org.jboss.cache.transaction.DummyTransactionManager; -import org.jboss.cache.transaction.GlobalTransaction; -import org.jboss.cache.transaction.OptimisticTransactionEntry; -import org.jboss.cache.transaction.TransactionTable; - -/** - * @author xenephon - */ -@SuppressWarnings("unchecked") -public class OpLockingInterceptorTest extends AbstractOptimisticTestCase -{ - public void testTransactionPrepareMethod() throws Exception - { - - TestListener listener = new TestListener(); - final CacheImpl cache = createCacheWithListener(listener); - - Interceptor lockingInterceptor = new OptimisticLockingInterceptor(); - lockingInterceptor.setCache(cache); - Interceptor interceptor = new OptimisticCreateIfNotExistsInterceptor(); - interceptor.setCache(cache); - Interceptor nodeInterceptor = new OptimisticNodeInterceptor(); - nodeInterceptor.setCache(cache); - MockInterceptor dummy = new MockInterceptor(); - dummy.setCache(cache); - lockingInterceptor.setNext(interceptor); - interceptor.setNext(nodeInterceptor); - nodeInterceptor.setNext(dummy); - - cache.setInterceptorChain(lockingInterceptor); - - // first set up a node with a pojo - DummyTransactionManager mgr = DummyTransactionManager.getInstance(); - mgr.begin(); - Transaction tx = mgr.getTransaction(); - - // inject InvocationContext - cache.getInvocationContext().setTransaction(tx); - cache.getInvocationContext().setGlobalTransaction(cache.getCurrentTransaction(tx)); - - SamplePojo pojo = new SamplePojo(21, "test"); - Map temp = new HashMap(); - temp.put("key1", pojo); - cache.put("/one/two", temp); - - assertEquals(null, dummy.getCalled()); - TransactionTable table = cache.getTransactionTable(); - - GlobalTransaction gtx = table.get(tx); - - OptimisticTransactionEntry entry = (OptimisticTransactionEntry) table.get(gtx); - - TransactionWorkspace workspace = entry.getTransactionWorkSpace(); - - /*GlobalTransaction.class, - List.class, - Address.class, - boolean.class*/ - - assertEquals(3, workspace.getNodes().size()); - assertNotNull(workspace.getNode(Fqn.fromString("/one/two"))); - assertEquals(pojo, workspace.getNode(Fqn.fromString("/one/two")).get("key1")); - assertEquals(1, workspace.getNode(Fqn.fromString("/one/two")).getMergedData().size()); - assertTrue(entry.getLocks().isEmpty()); - assertEquals(1, entry.getModifications().size()); - assertTrue(!cache.exists("/one/two")); - assertEquals(null, dummy.getCalled()); - - //now let us do a prepare - MethodCall prepareMethod = MethodCallFactory.create(MethodDeclarations.optimisticPrepareMethod, new Object[]{gtx, entry.getModifications(), gtx.getAddress(), Boolean.FALSE}); - try - { - cache._replicate(prepareMethod); - } - catch (Throwable t) - { - - } - - - assertEquals(3, workspace.getNodes().size()); - assertNotNull(workspace.getNode(Fqn.fromString("/one/two"))); - assertEquals(pojo, workspace.getNode(Fqn.fromString("/one/two")).get("key1")); - assertEquals(1, workspace.getNode(Fqn.fromString("/one/two")).getMergedData().size()); - assertEquals(3, entry.getLocks().size()); - for (Iterator it = entry.getLocks().iterator(); it.hasNext();) - { - NodeLock lock = (NodeLock) it.next(); - assertTrue(lock.isWriteLocked()); - assertEquals(gtx, lock.getWriterOwner()); - } - assertEquals(1, entry.getModifications().size()); - assertTrue(!cache.exists("/one/two")); - //assertEquals(null,dummy.getCalled()); - - - mgr.commit(); - - cache.stop(); - - } - - public void testTransactionCommitMethod() throws Exception - { - - TestListener listener = new TestListener(); - final CacheImpl cache = createCacheWithListener(listener); - - Interceptor lockingInterceptor = new OptimisticLockingInterceptor(); - lockingInterceptor.setCache(cache); - Interceptor interceptor = new OptimisticCreateIfNotExistsInterceptor(); - interceptor.setCache(cache); - Interceptor nodeInterceptor = new OptimisticNodeInterceptor(); - nodeInterceptor.setCache(cache); - MockInterceptor dummy = new MockInterceptor(); - dummy.setCache(cache); - lockingInterceptor.setNext(interceptor); - interceptor.setNext(nodeInterceptor); - nodeInterceptor.setNext(dummy); - - cache.setInterceptorChain(lockingInterceptor); - - // first set up a node with a pojo - DummyTransactionManager mgr = DummyTransactionManager.getInstance(); - mgr.begin(); - Transaction tx = mgr.getTransaction(); - - // inject InvocationContext - cache.getInvocationContext().setTransaction(tx); - cache.getInvocationContext().setGlobalTransaction(cache.getCurrentTransaction(tx)); - - SamplePojo pojo = new SamplePojo(21, "test"); - Map temp = new HashMap(); - temp.put("key1", pojo); - cache.put("/one/two", temp); - - assertEquals(null, dummy.getCalled()); - TransactionTable table = cache.getTransactionTable(); - - GlobalTransaction gtx = table.get(tx); - - OptimisticTransactionEntry entry = (OptimisticTransactionEntry) table.get(gtx); - - TransactionWorkspace workspace = entry.getTransactionWorkSpace(); - - /*GlobalTransaction.class, - List.class, - Address.class, - boolean.class*/ - - assertEquals(3, workspace.getNodes().size()); - assertNotNull(workspace.getNode(Fqn.fromString("/one/two"))); - assertEquals(pojo, workspace.getNode(Fqn.fromString("/one/two")).get("key1")); - assertEquals(1, workspace.getNode(Fqn.fromString("/one/two")).getMergedData().size()); - assertTrue(entry.getLocks().isEmpty()); - assertEquals(1, entry.getModifications().size()); - assertTrue(!cache.exists("/one/two")); - assertEquals(null, dummy.getCalled()); - - //now let us do a prepare - MethodCall prepareMethod = MethodCallFactory.create(MethodDeclarations.optimisticPrepareMethod, new Object[]{gtx, entry.getModifications(), gtx.getAddress(), Boolean.FALSE}); - try - { - cache._replicate(prepareMethod); - } - catch (Throwable t) - { - - } - - - assertEquals(3, workspace.getNodes().size()); - assertNotNull(workspace.getNode(Fqn.fromString("/one/two"))); - assertEquals(pojo, workspace.getNode(Fqn.fromString("/one/two")).get("key1")); - assertEquals(1, workspace.getNode(Fqn.fromString("/one/two")).getMergedData().size()); - assertEquals(3, entry.getLocks().size()); - for (Iterator it = entry.getLocks().iterator(); it.hasNext();) - { - NodeLock lock = (NodeLock) it.next(); - assertTrue(lock.isWriteLocked()); - assertEquals(gtx, lock.getWriterOwner()); - } - assertEquals(1, entry.getModifications().size()); - assertTrue(!cache.exists("/one/two")); - //assertEquals(null,dummy.getCalled()); - assertEquals(MethodDeclarations.optimisticPrepareMethod, dummy.getCalled()); - - - MethodCall commitMethod = MethodCallFactory.create(MethodDeclarations.commitMethod, new Object[]{gtx}); - try - { - cache._replicate(commitMethod); - } - catch (Throwable t) - { - fail(); - } - assertEquals(3, entry.getLocks().size()); - for (Iterator it = entry.getLocks().iterator(); it.hasNext();) - { - NodeLock lock = (NodeLock) it.next(); - assertEquals(false, lock.isLocked()); - - } - //make sure the nodes and locks are the same order - int i = 0; - for (Iterator it = workspace.getNodes().values().iterator(); it.hasNext();) - { - NodeSPI node = ((WorkspaceNode) it.next()).getNode(); - assertEquals(node.getLock(), entry.getLocks().get(i)); - i++; - } - assertEquals(MethodDeclarations.commitMethod, dummy.getCalled()); - mgr.commit(); - - cache.stop(); - - } - - public void testTransactionRollbackMethod() throws Exception - { - - TestListener listener = new TestListener(); - final CacheImpl cache = createCacheWithListener(listener); - - Interceptor lockingInterceptor = new OptimisticLockingInterceptor(); - lockingInterceptor.setCache(cache); - Interceptor interceptor = new OptimisticCreateIfNotExistsInterceptor(); - interceptor.setCache(cache); - Interceptor nodeInterceptor = new OptimisticNodeInterceptor(); - nodeInterceptor.setCache(cache); - MockInterceptor dummy = new MockInterceptor(); - dummy.setCache(cache); - lockingInterceptor.setNext(interceptor); - interceptor.setNext(nodeInterceptor); - nodeInterceptor.setNext(dummy); - - cache.setInterceptorChain(lockingInterceptor); - - // first set up a node with a pojo - DummyTransactionManager mgr = DummyTransactionManager.getInstance(); - mgr.begin(); - Transaction tx = mgr.getTransaction(); - - // inject InvocationContext - cache.getInvocationContext().setTransaction(tx); - cache.getInvocationContext().setGlobalTransaction(cache.getCurrentTransaction(tx)); - - SamplePojo pojo = new SamplePojo(21, "test"); - Map temp = new HashMap(); - temp.put("key1", pojo); - cache.put("/one/two", temp); - - assertEquals(null, dummy.getCalled()); - TransactionTable table = cache.getTransactionTable(); - - GlobalTransaction gtx = table.get(tx); - - OptimisticTransactionEntry entry = (OptimisticTransactionEntry) table.get(gtx); - - TransactionWorkspace workspace = entry.getTransactionWorkSpace(); - - /*GlobalTransaction.class, - List.class, - Address.class, - boolean.class*/ - - assertEquals(3, workspace.getNodes().size()); - assertNotNull(workspace.getNode(Fqn.fromString("/one/two"))); - assertEquals(pojo, workspace.getNode(Fqn.fromString("/one/two")).get("key1")); - assertEquals(1, workspace.getNode(Fqn.fromString("/one/two")).getMergedData().size()); - assertTrue(entry.getLocks().isEmpty()); - assertEquals(1, entry.getModifications().size()); - assertTrue(!cache.exists("/one/two")); - assertEquals(null, dummy.getCalled()); - - //now let us do a prepare - MethodCall prepareMethod = MethodCallFactory.create(MethodDeclarations.optimisticPrepareMethod, new Object[]{gtx, entry.getModifications(), gtx.getAddress(), Boolean.FALSE}); - try - { - cache._replicate(prepareMethod); - } - catch (Throwable t) - { - - } - - - assertEquals(3, workspace.getNodes().size()); - assertNotNull(workspace.getNode(Fqn.fromString("/one/two"))); - assertEquals(pojo, workspace.getNode(Fqn.fromString("/one/two")).get("key1")); - assertEquals(1, workspace.getNode(Fqn.fromString("/one/two")).getMergedData().size()); - assertEquals(3, entry.getLocks().size()); - for (Iterator it = entry.getLocks().iterator(); it.hasNext();) - { - NodeLock lock = (NodeLock) it.next(); - assertTrue(lock.isWriteLocked()); - assertEquals(gtx, lock.getWriterOwner()); - } - assertEquals(1, entry.getModifications().size()); - assertTrue(!cache.exists("/one/two")); - assertEquals(MethodDeclarations.optimisticPrepareMethod, dummy.getCalled()); - - - MethodCall rollbackMethod = MethodCallFactory.create(MethodDeclarations.rollbackMethod, new Object[]{gtx}); - try - { - cache._replicate(rollbackMethod); - } - catch (Throwable t) - { - fail(); - } - assertEquals(3, entry.getLocks().size()); - for (Iterator it = entry.getLocks().iterator(); it.hasNext();) - { - NodeLock lock = (NodeLock) it.next(); - assertEquals(false, lock.isLocked()); - - } - //make sure the nodes and locks are the same order - int i = 0; - for (Iterator it = workspace.getNodes().values().iterator(); it.hasNext();) - { - NodeSPI node = ((WorkspaceNode) it.next()).getNode(); - assertEquals(node.getLock(), entry.getLocks().get(i)); - i++; - } - assertEquals(MethodDeclarations.rollbackMethod, dummy.getCalled()); - mgr.commit(); - - cache.stop(); - - } - -} Added: core/trunk/src/test/java/org/jboss/cache/optimistic/OptimisticLockInterceptorTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/optimistic/OptimisticLockInterceptorTest.java (rev 0) +++ core/trunk/src/test/java/org/jboss/cache/optimistic/OptimisticLockInterceptorTest.java 2007-12-11 17:07:21 UTC (rev 4828) @@ -0,0 +1,246 @@ +package org.jboss.cache.optimistic; + +import junit.framework.Assert; +import org.jboss.cache.CacheSPI; +import org.jboss.cache.Fqn; +import org.jboss.cache.InvocationContext; +import org.jboss.cache.NodeSPI; +import org.jboss.cache.interceptors.Interceptor; +import org.jboss.cache.interceptors.OptimisticInterceptor; +import org.jboss.cache.interceptors.OptimisticLockingInterceptor; +import org.jboss.cache.lock.NodeLock; +import static org.jboss.cache.lock.NodeLock.LockType.READ; +import static org.jboss.cache.lock.NodeLock.LockType.WRITE; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import javax.transaction.TransactionManager; +import java.util.HashMap; +import java.util.Iterator; +import java.util.Map; + +/** + * // Test for JBCACHE-1228 + * + * @author Manik Surtani (<a href="mailto:manik@jboss.org">manik@jboss.org</a>) + */ +@Test(groups = "functional") +public class OptimisticLockInterceptorTest extends AbstractOptimisticTestCase +{ + private CacheSPI<Object, Object> cache; + private LockReportInterceptor lri; + private Fqn parent = Fqn.fromString("/parent"); + private Fqn child = Fqn.fromString("/parent/child"); + private TransactionManager tm; + + @BeforeMethod + protected void setUp() throws Exception + { + cache = createCache(); + lri = new LockReportInterceptor(); + lri.setCache(cache); + + Interceptor i = cache.getInterceptorChain().get(0); + Interceptor lockInterceptor = i; + // find the OptimisticLockInterceptor and insert LockReportInterceptor after that. + while (!(lockInterceptor instanceof OptimisticLockingInterceptor)) + { + System.out.println("Inspecting " + lockInterceptor.getClass().getSimpleName()); + lockInterceptor = lockInterceptor.getNext(); + } + Interceptor next = lockInterceptor.getNext(); + + lockInterceptor.setNext(lri); + lri.setNext(next); + + cache.put(child, "key", "value"); + + tm = cache.getTransactionManager(); + } + + @AfterMethod + public void tearDown() + { + cache.stop(); + } + + public void testPut() throws Exception + { + tm.begin(); + cache.put(child, "key2", "value2"); + lri.reset(); + lri.expectsReadLock(Fqn.ROOT); + lri.expectsReadLock(parent); + lri.expectsWriteLock(child); + tm.commit(); + lri.assertReceivedExpectedLocks(); + + assertNoStaleLocks(); + } + + public void testGet() throws Exception + { + tm.begin(); + cache.get(child, "key2"); + lri.reset(); + // nothing is stale, expecting nothing here. + tm.commit(); + lri.assertReceivedExpectedLocks(); + + assertNoStaleLocks(); + } + + public void testRemove() throws Exception + { + tm.begin(); + cache.remove(child, "key2"); + lri.reset(); + lri.expectsReadLock(Fqn.ROOT); + lri.expectsReadLock(parent); + lri.expectsWriteLock(child); + tm.commit(); + lri.assertReceivedExpectedLocks(); + + assertNoStaleLocks(); + } + + public void testPutLockParentForCIR() throws Exception + { + cache.getConfiguration().setLockParentForChildInsertRemove(true); + cache.removeNode(parent); + cache.put(parent, "k", "v"); + + tm.begin(); + cache.put(child, "key2", "value2"); + lri.reset(); + lri.expectsReadLock(Fqn.ROOT); + lri.expectsWriteLock(parent); + lri.expectsWriteLock(child); + tm.commit(); + lri.assertReceivedExpectedLocks(); + + assertNoStaleLocks(); + } + + public void testGetLockParentForCIR() throws Exception + { + cache.getConfiguration().setLockParentForChildInsertRemove(true); + tm.begin(); + cache.get(child, "key2"); + lri.reset(); + // nothing is stale, expecting nothing here. + tm.commit(); + lri.assertReceivedExpectedLocks(); + + assertNoStaleLocks(); + } + + public void testRemoveLockParentForCIR() throws Exception + { + cache.getConfiguration().setLockParentForChildInsertRemove(true); + tm.begin(); + cache.removeNode(child); + lri.reset(); + lri.expectsReadLock(Fqn.ROOT); + lri.expectsWriteLock(parent); + lri.expectsWriteLock(child); + tm.commit(); + lri.assertReceivedExpectedLocks(); + + assertNoStaleLocks(); + } + + + public void testPutNodeNotExists() throws Exception + { + cache.removeNode(Fqn.ROOT); + tm.begin(); + cache.put(child, "key2", "value2"); + lri.reset(); + lri.expectsReadLock(Fqn.ROOT); + lri.expectsWriteLock(parent); + lri.expectsWriteLock(child); + tm.commit(); + lri.assertReceivedExpectedLocks(); + + assertNoStaleLocks(); + } + + public void testGetNodeNotExists() throws Exception + { + cache.removeNode(Fqn.ROOT); + tm.begin(); + cache.get(child, "key2"); + lri.reset(); + // nothing is stale, expecting nothing here. + tm.commit(); + lri.assertReceivedExpectedLocks(); + + assertNoStaleLocks(); + } + + public void testRemoveNodeNotExists() throws Exception + { + cache.removeNode(Fqn.ROOT); + tm.begin(); + cache.remove(child, "key2"); + lri.reset(); + // nothing is stale, expecting nothing here. + tm.commit(); + lri.assertReceivedExpectedLocks(); + + assertNoStaleLocks(); + } + + private void assertNoStaleLocks() + { + assert cache.getNumberOfLocksHeld() == 0; + } +} + +class LockReportInterceptor extends OptimisticInterceptor +{ + private Map<Fqn, NodeLock.LockType> expected = new HashMap<Fqn, NodeLock.LockType>(); + private Map<Fqn, NodeLock.LockType> actual = new HashMap<Fqn, NodeLock.LockType>(); + + void reset() + { + expected.clear(); + actual.clear(); + } + + void assertReceivedExpectedLocks() + { + Assert.assertEquals(expected, actual); + } + + void expectsReadLock(Fqn f) + { + expected.put(f, READ); + } + + void expectsWriteLock(Fqn f) + { + expected.put(f, WRITE); + } + + @Override + public Object invoke(InvocationContext ctx) throws Throwable + { + TransactionWorkspace w = getTransactionWorkspace(ctx.getGlobalTransaction()); + Map nodeMap = w.getNodes(); + for (Iterator i = nodeMap.keySet().iterator(); i.hasNext();) + { + WorkspaceNode wn = (WorkspaceNode) nodeMap.get(i.next()); + NodeSPI n = wn.getNode(); + NodeLock lock = n.getLock(); + if (lock.isLocked()) + { + actual.put(n.getFqn(), lock.isReadLocked() ? READ : WRITE); + } + } + + return super.invoke(ctx); + } +}