Author: manik.surtani(a)jboss.com Date: 2007-09-21 09:53:59 -0400 (Fri, 21 Sep 2007) New Revision: 4497 Added: core/trunk/src/main/java/org/jboss/cache/loader/ReadOnlyDelegatingCacheLoader.java Modified: core/trunk/src/main/docbook/userguide/en/modules/transactions.xml core/trunk/src/main/java/org/jboss/cache/CacheImpl.java core/trunk/src/main/java/org/jboss/cache/Node.java core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticValidatorInterceptor.java core/trunk/src/main/java/org/jboss/cache/interceptors/PessimisticLockInterceptor.java core/trunk/src/main/java/org/jboss/cache/loader/CacheLoaderManager.java core/trunk/src/main/java/org/jboss/cache/optimistic/WorkspaceNodeImpl.java core/trunk/src/test/java/org/jboss/cache/loader/CacheLoaderTestsBase.java core/trunk/src/test/java/org/jboss/cache/loader/FileCacheLoaderTest.java core/trunk/src/test/java/org/jboss/cache/lock/WriteLockOnParentTest.java Log: JBCACHE-957 - lockParentForChildInsertRemove now configurable on a per-node basis JBCACHE-751 - IgnoreModifications to be made available to all cache loaders, not just chained ones. Modified: core/trunk/src/main/docbook/userguide/en/modules/transactions.xml =================================================================== --- core/trunk/src/main/docbook/userguide/en/modules/transactions.xml 2007-09-21 03:15:01 UTC (rev 4496) +++ core/trunk/src/main/docbook/userguide/en/modules/transactions.xml 2007-09-21 13:53:59 UTC (rev 4497) @@ -160,6 +160,11 @@ <emphasis>write lock</emphasis> on the parent node. </para> + <para> + In addition to the above, in version 2.1.0 and above, JBoss Cache offers the ability to override this + configuration on a per-node basis. See <literal>Node.setLockForChildInsertRemove()</literal> and it's + corresponding javadocs for details. + </para> </section> </section> Modified: core/trunk/src/main/java/org/jboss/cache/CacheImpl.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/CacheImpl.java 2007-09-21 03:15:01 UTC (rev 4496) +++ core/trunk/src/main/java/org/jboss/cache/CacheImpl.java 2007-09-21 13:53:59 UTC (rev 4497) @@ -864,6 +864,7 @@ { cacheStatus = CacheStatus.DESTROYING; regionManager = null; + cacheLoaderManager = null; notifier = null; // The rest of these should have already been taken care of in stop, Modified: core/trunk/src/main/java/org/jboss/cache/Node.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/Node.java 2007-09-21 03:15:01 UTC (rev 4496) +++ core/trunk/src/main/java/org/jboss/cache/Node.java 2007-09-21 13:53:59 UTC (rev 4497) @@ -277,4 +277,25 @@ */ boolean isValid(); + /** + * Tests whether this node is configured to be exclusively locked when inserting or removing children. + * <p /> + * The default + * value for this is what is configured in the <tt>LockParentForChildInsertRemove</tt> configuration property, + * programatically reachable by querying {@link org.jboss.cache.config.Configuration#isLockParentForChildInsertRemove()} + * <p /> + * This can also be configured on a per-node basis using {@link #setLockForChildInsertRemove(boolean)} + * @return true if the node is configured to be exclusively locked for child insertions and removal, false otherwise. + * @since 2.1.0 + */ + boolean isLockForChildInsertRemove(); + + /** + * Configures the behaviour of how this node is locked when adding/removing children. + * @param lockForChildInsertRemove if true, exclusive locks will be obtained when children are added/removed. If + * false, a shared "read lock" will be obtained instead. + * @since 2.1.0 + */ + void setLockForChildInsertRemove(boolean lockForChildInsertRemove); + } Modified: core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java 2007-09-21 03:15:01 UTC (rev 4496) +++ core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java 2007-09-21 13:53:59 UTC (rev 4497) @@ -68,6 +68,8 @@ */ private final Map<K, V> data = new HashMap<K, V>(); + private boolean lockForChildInsertRemove; + /** * Constructs a new node with an FQN of Root. */ @@ -107,6 +109,7 @@ { throw new IllegalArgumentException("Child " + child_name + " must be last part of " + fqn); } + lockForChildInsertRemove = cache.getConfiguration().isLockParentForChildInsertRemove(); } /** @@ -742,4 +745,14 @@ // TODO; implement this property, to detect if it has been evicted, removed by another thread, etc. Method added for now as a dummy so it exists in the API return true; } + + public boolean isLockForChildInsertRemove() + { + return lockForChildInsertRemove; + } + + public void setLockForChildInsertRemove(boolean lockForChildInsertRemove) + { + this.lockForChildInsertRemove = lockForChildInsertRemove; + } } Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticValidatorInterceptor.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticValidatorInterceptor.java 2007-09-21 03:15:01 UTC (rev 4496) +++ core/trunk/src/main/java/org/jboss/cache/interceptors/OptimisticValidatorInterceptor.java 2007-09-21 13:53:59 UTC (rev 4497) @@ -199,7 +199,7 @@ underlyingNode.removeChildDirect(child.getLastElement()); } - updateVersion = cache.getConfiguration().isLockParentForChildInsertRemove(); + updateVersion = underlyingNode.isLockForChildInsertRemove(); // do we need to notify listeners of a modification?? If all we've done is added children then don't // notify. } Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/PessimisticLockInterceptor.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/interceptors/PessimisticLockInterceptor.java 2007-09-21 03:15:01 UTC (rev 4496) +++ core/trunk/src/main/java/org/jboss/cache/interceptors/PessimisticLockInterceptor.java 2007-09-21 13:53:59 UTC (rev 4497) @@ -320,7 +320,7 @@ } else { - if (writeLockNeeded(ctx, lock_type, i, treeNodeSize, isEvictionOperation, isDeleteOperation, createIfNotExists, isRemoveDataOperation, fqn, child_node.getFqn())) + if (writeLockNeeded(ctx, lock_type, i, treeNodeSize, isEvictionOperation, isDeleteOperation, createIfNotExists, isRemoveDataOperation, fqn, child_node)) { lockTypeRequired = NodeLock.LockType.WRITE; @@ -373,21 +373,21 @@ n.markAsDeleted(false); } - private boolean writeLockNeeded(InvocationContext ctx, NodeLock.LockType lock_type, int currentNodeIndex, int treeNodeSize, boolean isEvictOperation, boolean isRemoveOperation, boolean isPutOperation, boolean isRemoveDataOperation, Fqn targetFqn, Fqn currentFqn) + private boolean writeLockNeeded(InvocationContext ctx, NodeLock.LockType lock_type, int currentNodeIndex, int treeNodeSize, boolean isEvictOperation, boolean isRemoveOperation, boolean isPutOperation, boolean isRemoveDataOperation, Fqn targetFqn, NodeSPI currentNode) { // write lock forced!! boolean isTargetNode = isTargetNode(currentNodeIndex, treeNodeSize); if (ctx.getOptionOverrides().isForceWriteLock() && isTargetNode) return true; - if (cache.getConfiguration().isLockParentForChildInsertRemove()) + if (currentNode.isLockForChildInsertRemove()) { if (isRemoveOperation && currentNodeIndex == treeNodeSize - 2) { return true;// we're doing a remove and we've reached the PARENT node of the target to be removed. } - if (!isTargetNode && cache.peek(new Fqn(currentFqn, targetFqn.get(currentNodeIndex + 1)), false) == null) + if (!isTargetNode && cache.peek(new Fqn(currentNode.getFqn(), targetFqn.get(currentNodeIndex + 1)), false) == null) { return isPutOperation;// we're at a node in the tree, not yet at the target node, and we need to create the next node. So we need a WL here. } Modified: core/trunk/src/main/java/org/jboss/cache/loader/CacheLoaderManager.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/loader/CacheLoaderManager.java 2007-09-21 03:15:01 UTC (rev 4496) +++ core/trunk/src/main/java/org/jboss/cache/loader/CacheLoaderManager.java 2007-09-21 13:53:59 UTC (rev 4497) @@ -176,6 +176,13 @@ tmpLoader = asyncDecorator; } + if (cfg.isIgnoreModifications()) + { + AbstractDelegatingCacheLoader readOnlyDecorator; + readOnlyDecorator = new ReadOnlyDelegatingCacheLoader(tmpLoader); + tmpLoader = readOnlyDecorator; + } + // singleton? SingletonStoreConfig ssc = cfg.getSingletonStoreConfig(); if (ssc != null && ssc.isSingletonStoreEnabled()) Added: core/trunk/src/main/java/org/jboss/cache/loader/ReadOnlyDelegatingCacheLoader.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/loader/ReadOnlyDelegatingCacheLoader.java (rev 0) +++ core/trunk/src/main/java/org/jboss/cache/loader/ReadOnlyDelegatingCacheLoader.java 2007-09-21 13:53:59 UTC (rev 4497) @@ -0,0 +1,85 @@ +package org.jboss.cache.loader; + +import org.jboss.cache.Fqn; +import org.jboss.cache.Modification; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + +import java.util.Map; +import java.util.List; +import java.io.ObjectInputStream; + +/** + * Provides ignoreModifications features to all cache loaders. + * + * @author <a href="mailto:manik@jboss.org">Manik Surtani</a> + * @since 2.1.0 + */ +public class ReadOnlyDelegatingCacheLoader extends AbstractDelegatingCacheLoader +{ + private static Log log = LogFactory.getLog(ReadOnlyDelegatingCacheLoader.class); + + public ReadOnlyDelegatingCacheLoader(CacheLoader cl) + { + super(cl); + } + + public Object put(Fqn name, Object key, Object value) throws Exception + { + log.trace("Not delegating write operation to underlying cache loader"); + Map map = get(name); + return (map == null) ? null : map.get(key); + } + + public void put(Fqn name, Map attributes) throws Exception + { + log.trace("Not delegating write operation to underlying cache loader"); + } + + public void put(List<Modification> modifications) throws Exception + { + log.trace("Not delegating write operation to underlying cache loader"); + } + + public Object remove(Fqn fqn, Object key) throws Exception + { + log.trace("Not delegating write operation to underlying cache loader"); + Map map = get(fqn); + return (map == null) ? null : map.get(key); + } + + public void remove(Fqn fqn) throws Exception + { + log.trace("Not delegating write operation to underlying cache loader"); + } + + public void removeData(Fqn fqn) throws Exception + { + log.trace("Not delegating write operation to underlying cache loader"); + } + + public void prepare(Object tx, List<Modification> modifications, boolean one_phase) throws Exception + { + log.trace("Not delegating write operation to underlying cache loader"); + } + + public void commit(Object tx) throws Exception + { + log.trace("Not delegating write operation to underlying cache loader"); + } + + public void rollback(Object tx) + { + log.trace("Not delegating write operation to underlying cache loader"); + } + + public void storeEntireState(ObjectInputStream is) throws Exception + { + log.trace("Not delegating write operation to underlying cache loader"); + } + + public void storeState(Fqn subtree, ObjectInputStream is) throws Exception + { + log.trace("Not delegating write operation to underlying cache loader"); + } +} Modified: core/trunk/src/main/java/org/jboss/cache/optimistic/WorkspaceNodeImpl.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/optimistic/WorkspaceNodeImpl.java 2007-09-21 03:15:01 UTC (rev 4496) +++ core/trunk/src/main/java/org/jboss/cache/optimistic/WorkspaceNodeImpl.java 2007-09-21 13:53:59 UTC (rev 4497) @@ -329,6 +329,16 @@ throw new UnsupportedOperationException(); } + public boolean isLockForChildInsertRemove() + { + throw new UnsupportedOperationException(); + } + + public void setLockForChildInsertRemove(boolean lockForChildInsertRemove) + { + throw new UnsupportedOperationException(); + } + public NodeSPI<K, V> getChild(Fqn f) { if (f.size() > 1) Modified: core/trunk/src/test/java/org/jboss/cache/loader/CacheLoaderTestsBase.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/loader/CacheLoaderTestsBase.java 2007-09-21 03:15:01 UTC (rev 4496) +++ core/trunk/src/test/java/org/jboss/cache/loader/CacheLoaderTestsBase.java 2007-09-21 13:53:59 UTC (rev 4497) @@ -39,6 +39,7 @@ import org.jboss.util.stream.MarshalledValueOutputStream; import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; /** * Commons tests for all CacheLoaders @@ -46,6 +47,7 @@ * @author Bela Ban * @version $Id$ */ +@Test(groups = {"functional"}) abstract public class CacheLoaderTestsBase extends AbstractCacheLoaderTestBase { @@ -2099,6 +2101,33 @@ } + public void testIgnoreModifications() throws Exception + { + Fqn fqn = Fqn.fromString("/a"); + cache.put(fqn, "k", "v"); + assert "v".equals(cache.get(fqn, "k")); + assert "v".equals(loader.get(fqn).get("k")); + + // now stop the cache + cache.stop(); + cache.destroy(); + cache.getConfiguration().getCacheLoaderConfig().getIndividualCacheLoaderConfigs().get(0).setIgnoreModifications(true); + cache.start(); + loader = cache.getCacheLoader(); + + // test that the cache loader is wrapped by a read-only delegate + assert loader instanceof ReadOnlyDelegatingCacheLoader; + + // old state should be persisted. + assert "v".equals(cache.get(fqn, "k")); + assert "v".equals(loader.get(fqn).get("k")); + + // the loader should now be read-only + cache.put(fqn, "k", "v2"); + assert "v2".equals(cache.get(fqn, "k")); + assert "v".equals(loader.get(fqn).get("k")); + } + public void testCacheLoaderThreadSafety() throws Exception { threadSafetyTest(true); Modified: core/trunk/src/test/java/org/jboss/cache/loader/FileCacheLoaderTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/loader/FileCacheLoaderTest.java 2007-09-21 03:15:01 UTC (rev 4496) +++ core/trunk/src/test/java/org/jboss/cache/loader/FileCacheLoaderTest.java 2007-09-21 13:53:59 UTC (rev 4497) @@ -1,16 +1,12 @@ package org.jboss.cache.loader; import static org.testng.AssertJUnit.assertEquals; +import org.testng.annotations.Test; import org.jboss.cache.Fqn; import org.jboss.cache.misc.TestingUtil; -/** - * Created by IntelliJ IDEA. - * User: bela - * Date: Jun 9, 2004 - * Time: 9:05:19 AM - */ +@Test(groups = {"functional"}) public class FileCacheLoaderTest extends CacheLoaderTestsBase { protected void configureCache() throws Exception Modified: core/trunk/src/test/java/org/jboss/cache/lock/WriteLockOnParentTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/lock/WriteLockOnParentTest.java 2007-09-21 03:15:01 UTC (rev 4496) +++ core/trunk/src/test/java/org/jboss/cache/lock/WriteLockOnParentTest.java 2007-09-21 13:53:59 UTC (rev 4497) @@ -13,6 +13,7 @@ import org.jboss.cache.CacheSPI; import org.jboss.cache.DefaultCacheFactory; import org.jboss.cache.Fqn; +import org.jboss.cache.Node; import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; @@ -175,4 +176,92 @@ assertNotNull("/a/c should exist", cache.peek(a_c, false)); } + public void testPerNodeConfigurationDefaultLock() throws Exception + { + testPerNodeConfiguration(true); + } + + public void testPerNodeConfigurationDefaultNoLock() throws Exception + { + testPerNodeConfiguration(false); + } + + private void testPerNodeConfiguration(boolean defaultLock) throws Exception + { + cache.getConfiguration().setLockParentForChildInsertRemove(defaultLock); + + cache.put(a, Collections.emptyMap()); + if (!defaultLock) + { + // set a per-node lock for a + cache.getRoot().getChild(a).setLockForChildInsertRemove(true); + } + + cache.put(a_b, Collections.emptyMap()); + cache.put(a_c, Collections.emptyMap()); + + assertNotNull("/a should exist", cache.peek(a, false)); + assertNotNull("/a/b should exist", cache.peek(a_b, false)); + assertNotNull("/a/c should exist", cache.peek(a_c, false)); + + // concurrent remove of /a/b and /a/c + tm.begin(); + cache.removeNode(a_b); + Transaction t1 = tm.suspend(); + + tm.begin(); + try + { + cache.removeNode(a_c); + fail("Should not get here."); + } + catch (TimeoutException e) + { + // expected + } + tm.commit(); + + tm.resume(t1); + tm.commit(); + + assertNotNull("/a should exist", cache.peek(a, false)); + assertNull("/a/b should not exist", cache.peek(a_b, false)); + assertNotNull("/a/c should exist", cache.peek(a_c, false)); + + Fqn b = Fqn.fromString("/b"); + Fqn b_b = Fqn.fromString("/b/b"); + Fqn b_c = Fqn.fromString("/b/c"); + cache.put(b, Collections.emptyMap()); + + if (defaultLock) + { + // set a per-node locking config for node b + cache.getRoot().getChild(b).setLockForChildInsertRemove(false); + } + + + cache.put(b_b, Collections.emptyMap()); + cache.put(b_c, Collections.emptyMap()); + + assertNotNull("/a should exist", cache.peek(b, false)); + assertNotNull("/a/b should exist", cache.peek(b_b, false)); + assertNotNull("/a/c should exist", cache.peek(b_c, false)); + + // concurrent remove of /a/b and /a/c + tm.begin(); + cache.removeNode(b_b); + t1 = tm.suspend(); + + tm.begin(); + cache.removeNode(b_c); + tm.commit(); + + tm.resume(t1); + tm.commit(); + + assertNotNull("/b should exist", cache.peek(b, false)); + assertNull("/b/b should not exist", cache.peek(b_b, false)); + assertNull("/b/c should not exist", cache.peek(b_c, false)); + } + }