Author: manik.surtani(a)jboss.com Date: 2008-09-26 07:50:32 -0400 (Fri, 26 Sep 2008) New Revision: 6803 Added: core/trunk/src/main/java/org/jboss/cache/interceptors/LegacyCacheStoreInterceptor.java core/trunk/src/test/java/org/jboss/cache/loader/DummyInMemoryCacheLoaderPessimisticTest.java Removed: core/trunk/src/main/java/org/jboss/cache/util/concurrent/locks/UpgradableLock.java Modified: core/trunk/src/main/java/org/jboss/cache/factories/InterceptorChainFactory.java core/trunk/src/main/java/org/jboss/cache/interceptors/CacheLoaderInterceptor.java core/trunk/src/main/java/org/jboss/cache/interceptors/CacheStoreInterceptor.java core/trunk/src/main/java/org/jboss/cache/interceptors/LegacyCacheLoaderInterceptor.java core/trunk/src/main/java/org/jboss/cache/loader/AdjListJDBCCacheLoader.java core/trunk/src/main/java/org/jboss/cache/loader/AdjListJDBCCacheLoaderConfig.java core/trunk/src/main/java/org/jboss/cache/loader/CacheLoader.java core/trunk/src/main/java/org/jboss/cache/loader/FileCacheLoader.java core/trunk/src/main/java/org/jboss/cache/loader/JDBCCacheLoader.java core/trunk/src/main/java/org/jboss/cache/loader/JDBCCacheLoaderOld.java core/trunk/src/main/java/org/jboss/cache/lock/StripedLock.java core/trunk/src/test/java/org/jboss/cache/loader/AsyncFileCacheLoaderTest.java core/trunk/src/test/java/org/jboss/cache/loader/CacheLoaderTestsBase.java core/trunk/src/test/java/org/jboss/cache/loader/CacheLoaderWithReplicationTest.java core/trunk/src/test/java/org/jboss/cache/loader/DummyInMemoryCacheLoader.java Log: 1) Better impl for JBCACHE-1410: JDBCCacheLoader may attempt to create a node entry in the DB twice 2) JBCACHE-1368: Optimize the CacheLoader.putAll() method so reads are not necessary 3) JBCACHE-1414: inefficient exists() in JDBCCacheLoader Modified: core/trunk/src/main/java/org/jboss/cache/factories/InterceptorChainFactory.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/factories/InterceptorChainFactory.java 2008-09-26 11:28:53 UTC (rev 6802) +++ core/trunk/src/main/java/org/jboss/cache/factories/InterceptorChainFactory.java 2008-09-26 11:50:32 UTC (rev 6803) @@ -145,8 +145,14 @@ else { if (configuration.getNodeLockingScheme() != NodeLockingScheme.MVCC) + { interceptorChain.appendIntereceptor(createInterceptor(LegacyCacheLoaderInterceptor.class)); - interceptorChain.appendIntereceptor(createInterceptor(CacheStoreInterceptor.class)); + interceptorChain.appendIntereceptor(createInterceptor(LegacyCacheStoreInterceptor.class)); + } + else + { + interceptorChain.appendIntereceptor(createInterceptor(CacheStoreInterceptor.class)); + } } } Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/CacheLoaderInterceptor.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/interceptors/CacheLoaderInterceptor.java 2008-09-26 11:28:53 UTC (rev 6802) +++ core/trunk/src/main/java/org/jboss/cache/interceptors/CacheLoaderInterceptor.java 2008-09-26 11:50:32 UTC (rev 6803) @@ -116,7 +116,7 @@ { if (command.getFqn() != null) { - loadIfNeeded(ctx, command.getFqn(), null, false, true, false, false, false, false, false); + loadIfNeeded(ctx, command.getFqn(), null, true, true, false, false, false, false, true); } return invokeNextInterceptor(ctx, command); } Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/CacheStoreInterceptor.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/interceptors/CacheStoreInterceptor.java 2008-09-26 11:28:53 UTC (rev 6802) +++ core/trunk/src/main/java/org/jboss/cache/interceptors/CacheStoreInterceptor.java 2008-09-26 11:50:32 UTC (rev 6803) @@ -77,7 +77,7 @@ private HashMap txStores = new HashMap(); private Map<GlobalTransaction, Set<Fqn>> preparingTxs = new ConcurrentHashMap<GlobalTransaction, Set<Fqn>>(); private long cacheStores = 0; - private CacheLoader loader; + CacheLoader loader; private CacheLoaderManager loaderManager; private boolean optimistic; private boolean statsEnabled; @@ -281,12 +281,17 @@ return returnValue; } - loader.put(command.getFqn(), command.getData()); + storeStateForPutDataMap(command.getFqn(), ctx); if (getStatisticsEnabled()) cacheStores++; return returnValue; } + protected void storeStateForPutDataMap(Fqn f, InvocationContext ctx) throws Exception + { + loader.put(f, ctx.lookUpNode(f).getDelegationTarget().getData()); + } + @Override protected Object handlePutKeyValueCommand(InvocationContext ctx, PutKeyValueCommand command) throws Throwable { @@ -328,7 +333,7 @@ NodeSPI n = ctx.lookUpNode(f); if (n != null && !n.isDeleted()) { - Map internalState = n.getInternalState(true); + Map internalState = n.getInternalState(false); loader.put(f, internalState); } } Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/LegacyCacheLoaderInterceptor.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/interceptors/LegacyCacheLoaderInterceptor.java 2008-09-26 11:28:53 UTC (rev 6802) +++ core/trunk/src/main/java/org/jboss/cache/interceptors/LegacyCacheLoaderInterceptor.java 2008-09-26 11:50:32 UTC (rev 6803) @@ -119,7 +119,7 @@ { if (command.getFqn() != null) { - loadIfNeeded(ctx, command.getFqn(), null, false, true, false, ctx.getTransactionContext(), false, false, false); + loadIfNeeded(ctx, command.getFqn(), null, true, true, false, ctx.getTransactionContext(), false, false, false); } return invokeNextInterceptor(ctx, command); } Added: core/trunk/src/main/java/org/jboss/cache/interceptors/LegacyCacheStoreInterceptor.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/interceptors/LegacyCacheStoreInterceptor.java (rev 0) +++ core/trunk/src/main/java/org/jboss/cache/interceptors/LegacyCacheStoreInterceptor.java 2008-09-26 11:50:32 UTC (rev 6803) @@ -0,0 +1,37 @@ +/* + * JBoss, Home of Professional Open Source + * Copyright 2008, Red Hat Middleware LLC, and individual contributors + * by the @authors tag. See the copyright.txt in the distribution for a + * full listing of individual contributors. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ +package org.jboss.cache.interceptors; + +import org.jboss.cache.Fqn; +import org.jboss.cache.InvocationContext; +import org.jboss.cache.annotations.Compat; + +@Compat +@Deprecated +public class LegacyCacheStoreInterceptor extends CacheStoreInterceptor +{ + @Override + protected void storeStateForPutDataMap(Fqn f, InvocationContext ctx) throws Exception + { + loader.put(f, ctx.lookUpNode(f).getDataDirect()); + } +} Modified: core/trunk/src/main/java/org/jboss/cache/loader/AdjListJDBCCacheLoader.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/loader/AdjListJDBCCacheLoader.java 2008-09-26 11:28:53 UTC (rev 6802) +++ core/trunk/src/main/java/org/jboss/cache/loader/AdjListJDBCCacheLoader.java 2008-09-26 11:50:32 UTC (rev 6803) @@ -69,7 +69,10 @@ protected ConnectionFactory cf; protected String driverName; private AdjListJDBCCacheLoaderConfig config; - protected StripedLock lock = new StripedLock(); + protected StripedLock lock = new StripedLock(128); + // dummy, used for serializing empty maps. One should NOT use Collections.emptyMap() here since if an emptyMap is + // serialized, upon deserialization it cannot be added to. + private static final Map<Object, Object> EMPTY_HASHMAP = new HashMap<Object, Object>(0, 1); public void setConfig(CacheLoaderConfig.IndividualCacheLoaderConfig base) { @@ -265,8 +268,46 @@ safeClose(st); cf.close(con); } + + createDummyTableIfNeeded(); } + private void createDummyTableIfNeeded() throws Exception + { + Connection conn = null; + PreparedStatement ps = null; + try + { + conn = cf.getConnection(); + ps = conn.prepareStatement(config.getDummyTableRemovalDDL()); + ps.execute(); + } + catch (Exception e) + { + // ignore - it just means we didn't need to drop any database tables. + } + finally + { + safeClose(ps); + cf.close(conn); + } + + try + { + conn = cf.getConnection(); + ps = conn.prepareStatement(config.getDummyTableCreationDDL()); + ps.execute(); + safeClose(ps); + ps = conn.prepareStatement(config.getDummyTablePopulationSql()); + ps.execute(); + } + finally + { + safeClose(ps); + cf.close(conn); + } + } + @Override public void stop() { @@ -309,14 +350,23 @@ public boolean exists(Fqn name) throws Exception { lock.acquireLock(name, false); + Connection conn = null; + PreparedStatement ps = null; + ResultSet rs = null; try { - final Map node = loadNode(name); - return node != null;// && node != NULL_NODE_IN_ROW; + conn = cf.getConnection(); + ps = conn.prepareStatement(config.getExistsSql()); + ps.setString(1, name.toString()); + rs = ps.executeQuery(); + return rs.next(); } finally { lock.releaseLock(name); + safeClose(rs); + safeClose(ps); + cf.close(conn); } } @@ -424,11 +474,14 @@ /** * Inserts a node into the database * - * @param name the fqn - * @param dataMap the node + * @param name the fqn + * @param dataMap the node + * @param rowMayExist if true, then this method will not be strict in testing for 1 row being inserted, since 0 may be inserted if the row already exists. */ - protected void insertNode(Fqn name, Map dataMap) + protected void insertNode(Fqn name, Map dataMap, boolean rowMayExist) { + if (dataMap != null && !(dataMap instanceof HashMap)) + throw new RuntimeException("Cannot persist map of type " + dataMap.getClass()); Connection con = null; PreparedStatement ps = null; try @@ -444,7 +497,7 @@ populatePreparedStatementForInsert(name, dataMap, ps); int rows = ps.executeUpdate(); - if (rows != 1) + if (!rowMayExist && rows != 1) { throw new IllegalStateException("Expected one insert row but got " + rows); } @@ -471,7 +524,8 @@ protected void populatePreparedStatementForInsert(Fqn name, Map dataMap, PreparedStatement ps) throws Exception { - ps.setString(1, name.toString()); + String fqnString = name.toString(); + ps.setString(1, fqnString); if (dataMap != null) { @@ -501,6 +555,9 @@ { ps.setString(3, name.getAncestor(name.size() - 1).toString()); } + + // and a repeat - the 4th param is the same as the 1st one. + ps.setString(4, fqnString); } @@ -512,6 +569,8 @@ */ protected void updateNode(Fqn name, Map<Object, Object> node) { + if (node != null && !(node instanceof HashMap)) + throw new RuntimeException("Cannot persist map of type " + node.getClass()); Connection con = null; PreparedStatement ps = null; try @@ -524,7 +583,7 @@ con = cf.getConnection(); ps = con.prepareStatement(config.getUpdateNodeSql()); - if (node == null) node = Collections.emptyMap(); + if (node == null) node = EMPTY_HASHMAP; ByteBuffer byteBuffer = marshall(node); ps.setBinaryStream(1, byteBuffer.getStream(), byteBuffer.getLength()); Modified: core/trunk/src/main/java/org/jboss/cache/loader/AdjListJDBCCacheLoaderConfig.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/loader/AdjListJDBCCacheLoaderConfig.java 2008-09-26 11:28:53 UTC (rev 6802) +++ core/trunk/src/main/java/org/jboss/cache/loader/AdjListJDBCCacheLoaderConfig.java 2008-09-26 11:50:32 UTC (rev 6803) @@ -67,6 +67,7 @@ private String selectNodeSql; private String updateNodeSql; private String updateTableSql; + private String existsSql; private String connectionFactoryClass; private String primaryKey = PRIMARY_KEY_DEFAULT; private String fqnType = FQN_TYPE_DEFAULT; @@ -188,6 +189,21 @@ return insertNodeSql; } + public String getExistsSql() + { + if (existsSql == null) + { + setExistsSql(constructExistsSql()); + } + return existsSql; + } + + public void setExistsSql(String existsSql) + { + testImmutability("existsSql"); + this.existsSql = existsSql; + } + public void setInsertNodeSql(String insertNodeSql) { testImmutability("insertNodeSql"); @@ -577,16 +593,51 @@ return "select " + fqnColumn + " from " + table + " where " + parentColumn + "=?"; } + private String constructExistsSql() + { + return "select '1' from " + table + " where " + fqnColumn + "=?"; + } + private String constructInsertNodeSql() { - return "insert into " + - table + - " (" + - fqnColumn + - ", " + - nodeColumn + - ", " + - parentColumn + - ") values (?, ?, ?)"; + // This SQL string takes in 4 params - fqn, node (serialized data), parent, and fqn AGAIN. + // the benefit of this is is that it will run without failing even if the row already exists, so you don't need + // to check if the row exists before running this query. Returns '1' if the row was inserted, '0' otherwise, + // but does NOT fail on primary key conflict. + + // the 'dummy' table, table_D, *must* exist though, and could contain just a single dummy constant row. + + return "INSERT INTO " + + table + + " (" + + fqnColumn + + ", " + + nodeColumn + + ", " + + parentColumn + + ") SELECT ?, ?, ? FROM " + + table + + "_D WHERE NOT EXISTS (SELECT " + + fqnColumn + + " FROM " + + table + + " WHERE " + + fqnColumn + + " = ?)"; } + + public String getDummyTableCreationDDL() + { + return "create table " + table + "_D (i CHAR)"; + } + + public String getDummyTableRemovalDDL() + { + return "drop table " + table + "_D"; + } + + public String getDummyTablePopulationSql() + { + return "insert into " + table + "_D values ('x')"; + } } \ No newline at end of file Modified: core/trunk/src/main/java/org/jboss/cache/loader/CacheLoader.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/loader/CacheLoader.java 2008-09-26 11:28:53 UTC (rev 6802) +++ core/trunk/src/main/java/org/jboss/cache/loader/CacheLoader.java 2008-09-26 11:50:32 UTC (rev 6803) @@ -142,6 +142,11 @@ * insertion. * This is the same behavior as {@link Map#putAll}. * If the node does not exist, all parent nodes from the root down are created automatically + * <p/> + * <b>Note</b> since <i>3.0</i>, as an optimization, this method will require a definitive attribute map and + * not just a subset. This will allow cache loader implementations to overwrite rather than merge, if that is + * deemed more efficient. This will not break backward compatibility since performing a merge will not cause + * any loss of data even though it is an unnecessary step. * * @param name The fully qualified name of the node * @param attributes A Map of attributes. Can be null Modified: core/trunk/src/main/java/org/jboss/cache/loader/FileCacheLoader.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/loader/FileCacheLoader.java 2008-09-26 11:28:53 UTC (rev 6802) +++ core/trunk/src/main/java/org/jboss/cache/loader/FileCacheLoader.java 2008-09-26 11:50:32 UTC (rev 6803) @@ -257,7 +257,7 @@ public void put(Fqn fqn, Map attributes) throws Exception { - put(fqn, attributes, false); + put(fqn, attributes, true); } Modified: core/trunk/src/main/java/org/jboss/cache/loader/JDBCCacheLoader.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/loader/JDBCCacheLoader.java 2008-09-26 11:28:53 UTC (rev 6802) +++ core/trunk/src/main/java/org/jboss/cache/loader/JDBCCacheLoader.java 2008-09-26 11:50:32 UTC (rev 6803) @@ -98,10 +98,46 @@ */ public Object put(Fqn name, Object key, Object value) throws Exception { - Map<Object, Object> m = new HashMap<Object, Object>(1); - m.put(key, value); - Map existing = _put(name, m); - return existing == null ? null : existing.get(key); + lock.acquireLock(name, true); + try + { + Map<Object, Object> existing = loadNode(name); + + if (existing == null) + { + // do not use a singleton map here since this is serialized and stored in the DB. + Map<Object, Object> m = new HashMap<Object, Object>(1); + m.put(key, value); + addNewSubtree(name, m); + return null; + } + + if (existing == NULL_NODE_IN_ROW) + { + // do not use a singleton map here since this is serialized and stored in the DB. + Map<Object, Object> m = new HashMap<Object, Object>(1); + m.put(key, value); + updateNode(name, m); + return null; + } + + //creation sequence important - we need to overwrite old values + try + { + Object oldVal = existing.put(key, value); + updateNode(name, existing); + return oldVal; + } + catch (UnsupportedOperationException uoe) + { + log.error("Caught UOE - map is " + existing.getClass()); + throw uoe; + } + } + finally + { + lock.releaseLock(name); + } } /** @@ -110,7 +146,24 @@ */ public void put(Fqn name, Map attributes) throws Exception { - _put(name, attributes); + lock.acquireLock(name, true); + Map toStore = attributes; + if (toStore != null && !(toStore instanceof HashMap)) toStore = new HashMap(attributes); + try + { + if (!exists(name)) + { + addNewSubtree(name, toStore); + } + else + { + updateNode(name, toStore); + } + } + finally + { + lock.releaseLock(name); + } } @Override @@ -291,56 +344,18 @@ return result; } - private Map _put(Fqn name, Map attributes) throws Exception - { - lock.acquireLock(name, true); - try - { - Map result = null; - Map treeNode = loadNode(name); - if (treeNode == null) - { - addNewSubtree(name, attributes); - } - else if (treeNode == NULL_NODE_IN_ROW) - { - updateNode(name, attributes); - } - else - {//the node exists and the attribute map is NOT null - Map<Object, Object> newAttributes = new HashMap<Object, Object>(treeNode); - newAttributes.putAll(attributes);//creation sequnce important - we need to overwrite old values - updateNode(name, newAttributes); - result = treeNode; - } - return result; - } - finally - { - lock.releaseLock(name); - } - } - private void addNewSubtree(Fqn name, Map attributes) throws Exception { Fqn currentNode = name; do { - try + if (currentNode.equals(name)) { - lock.acquireLock(currentNode, true); - if (currentNode.equals(name)) - { - insertNode(currentNode, attributes); - } - else - { - if (loadNode(currentNode) == null) insertNode(currentNode, null); - } + insertNode(currentNode, attributes, false); } - finally + else { - lock.releaseLock(currentNode); + insertNode(currentNode, null, true); } if (currentNode.isRoot()) break; currentNode = currentNode.getParent(); Modified: core/trunk/src/main/java/org/jboss/cache/loader/JDBCCacheLoaderOld.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/loader/JDBCCacheLoaderOld.java 2008-09-26 11:28:53 UTC (rev 6802) +++ core/trunk/src/main/java/org/jboss/cache/loader/JDBCCacheLoaderOld.java 2008-09-26 11:50:32 UTC (rev 6803) @@ -149,11 +149,11 @@ final Fqn parent = name.getAncestor(i); if (!exists(parent)) { - insertNode(parent, null); + insertNode(parent, null, false); } } } - insertNode(name, node); + insertNode(name, node, false); } return oldValue; @@ -331,11 +331,11 @@ final Fqn parent = name.getAncestor(i); if (!exists(parent)) { - insertNode(parent, null); + insertNode(parent, null, false); } } } - insertNode(name, attrs); + insertNode(name, attrs, false); } } Modified: core/trunk/src/main/java/org/jboss/cache/lock/StripedLock.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/lock/StripedLock.java 2008-09-26 11:28:53 UTC (rev 6802) +++ core/trunk/src/main/java/org/jboss/cache/lock/StripedLock.java 2008-09-26 11:50:32 UTC (rev 6803) @@ -23,9 +23,9 @@ import net.jcip.annotations.ThreadSafe; import org.jboss.cache.Fqn; -import org.jboss.cache.util.concurrent.locks.UpgradableLock; import java.util.List; +import java.util.concurrent.locks.ReentrantReadWriteLock; /** * A simple implementation of lock striping, using Fqns as the keys to lock on, primarily used to help make @@ -48,7 +48,7 @@ private final int lockSegmentMask; private final int lockSegmentShift; - final UpgradableLock[] sharedLocks; + final ReentrantReadWriteLock[] sharedLocks; /** * This constructor just calls {@link #StripedLock(int)} with a default concurrency value of 20. @@ -75,9 +75,9 @@ lockSegmentShift = 32 - tempLockSegShift; lockSegmentMask = numLocks - 1; - sharedLocks = new UpgradableLock[numLocks]; + sharedLocks = new ReentrantReadWriteLock[numLocks]; - for (int i = 0; i < numLocks; i++) sharedLocks[i] = new UpgradableLock(); + for (int i = 0; i < numLocks; i++) sharedLocks[i] = new ReentrantReadWriteLock(); } /** @@ -88,11 +88,11 @@ */ public void acquireLock(Fqn fqn, boolean exclusive) { - UpgradableLock lock = getLock(fqn); + ReentrantReadWriteLock lock = getLock(fqn); if (exclusive) { - lock.acquireWriteLockWithUpgrade(); + lock.writeLock().lock(); } else { @@ -107,11 +107,18 @@ */ public void releaseLock(Fqn fqn) { - UpgradableLock lock = getLock(fqn); - lock.unlock(); + ReentrantReadWriteLock lock = getLock(fqn); + if (lock.isWriteLockedByCurrentThread()) + { + lock.writeLock().unlock(); + } + else + { + lock.readLock().unlock(); + } } - final UpgradableLock getLock(Object o) + final ReentrantReadWriteLock getLock(Object o) { return sharedLocks[hashToIndex(o)]; } Deleted: core/trunk/src/main/java/org/jboss/cache/util/concurrent/locks/UpgradableLock.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/util/concurrent/locks/UpgradableLock.java 2008-09-26 11:28:53 UTC (rev 6802) +++ core/trunk/src/main/java/org/jboss/cache/util/concurrent/locks/UpgradableLock.java 2008-09-26 11:50:32 UTC (rev 6803) @@ -1,88 +0,0 @@ -/* - * JBoss, Home of Professional Open Source - * Copyright 2008, Red Hat Middleware LLC, and individual contributors - * by the @authors tag. See the copyright.txt in the distribution for a - * full listing of individual contributors. - * - * This is free software; you can redistribute it and/or modify it - * under the terms of the GNU Lesser General Public License as - * published by the Free Software Foundation; either version 2.1 of - * the License, or (at your option) any later version. - * - * This software is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this software; if not, write to the Free - * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA - * 02110-1301 USA, or see the FSF site: http://www.fsf.org. - */ -package org.jboss.cache.util.concurrent.locks; - -import org.jboss.cache.util.concurrent.ConcurrentHashSet; - -import java.util.Set; -import java.util.concurrent.locks.ReentrantReadWriteLock; - -/** - * A {@link java.util.concurrent.locks.ReentrantReadWriteLock} with support for acquiring write locks when the current - * thread already has a read lock. Also supports releasing such write locks and re-acquiring read locks in the process. - */ -public class UpgradableLock extends ReentrantReadWriteLock -{ - private final Set<Thread> upgraders = new ConcurrentHashSet<Thread>(); - - /** - * Attempts to upgrade a read lock to a write lock. If no read locks are held, a write lock is simply attempted. - */ - public void acquireWriteLockWithUpgrade() - { - boolean upgradeNeeded = true; - try - { - readLock().unlock(); - } - catch (IllegalMonitorStateException imse) - { - // read lock was never held; no upgrading needed. - upgradeNeeded = false; - } - - if (upgradeNeeded) upgraders.add(Thread.currentThread()); - - writeLock().lock(); - } - - /** - * Unlocks any locks held by the current thread. If a write lock is held, it is released. If a read lock is held - * it is released. This method supports reentrant locks so unlocking repeatedly may release >1 write lock. It - * also supports upgraded locks, i.e., read locks that were upgraded to write locks. - */ - public void unlock() - { - if (isWriteLockedByCurrentThread()) - { - writeLock().unlock(); - Thread current; - if (!isWriteLockedByCurrentThread() && upgraders.contains((current = Thread.currentThread()))) - { - // re-acquire the RL. - readLock().lock(); - upgraders.remove(current); - } - } - else - { - try - { - readLock().unlock(); - } - catch (IllegalMonitorStateException imse) - { - // perhaps the RL was already released earlier? - } - } - } -} Modified: core/trunk/src/test/java/org/jboss/cache/loader/AsyncFileCacheLoaderTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/loader/AsyncFileCacheLoaderTest.java 2008-09-26 11:28:53 UTC (rev 6802) +++ core/trunk/src/test/java/org/jboss/cache/loader/AsyncFileCacheLoaderTest.java 2008-09-26 11:50:32 UTC (rev 6803) @@ -88,9 +88,10 @@ map.put("c", "d"); // Three kinds of puts! Modification mod = new Modification(Modification.ModificationType.PUT_KEY_VALUE, fqn, "e", "f"); + loader.put(fqn, map); loader.put(fqn, "a", "b"); - loader.put(fqn, map); loader.put(Collections.singletonList(mod)); + System.out.println(loader.get(fqn)); assertEquals("put right away", 3, loader.get(fqn).size()); loader.remove(fqn); } Modified: core/trunk/src/test/java/org/jboss/cache/loader/CacheLoaderTestsBase.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/loader/CacheLoaderTestsBase.java 2008-09-26 11:28:53 UTC (rev 6802) +++ core/trunk/src/test/java/org/jboss/cache/loader/CacheLoaderTestsBase.java 2008-09-26 11:50:32 UTC (rev 6803) @@ -19,6 +19,7 @@ import static org.testng.AssertJUnit.*; import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; import javax.transaction.Transaction; import javax.transaction.TransactionManager; @@ -41,7 +42,7 @@ * @author Bela Ban * @version $Id$ */ -//@Test(groups = {"functional"}) +@Test(groups = {"functional"}) abstract public class CacheLoaderTestsBase extends AbstractCacheLoaderTestBase { private static final Log log = LogFactory.getLog(CacheLoaderTestsBase.class); @@ -1646,6 +1647,9 @@ list.add(mod); Map<Object, Object> map = new HashMap<Object, Object>(); + // any call to putAll() will result in all the cached data being written + map.put("one", "two"); + map.put("three", "four"); map.put("five", "six"); map.put("seven", "eight"); mod = new Modification(); @@ -1950,7 +1954,58 @@ assertEquals(value, loader.get(fqn).get(key)); } + public void testPutAllAfterEvict() throws Exception + { + Fqn fqn = Fqn.fromString("/a/b"); + Map<String, String> original = new HashMap<String, String>(); + Map<String, String> toAdd = new HashMap<String, String>(); + Map<String, String> all = new HashMap<String, String>(); + original.put("1", "One"); + original.put("2", "Two"); + toAdd.put("3", "Three"); + toAdd.put("4", "Four"); + + all.putAll(original); + all.putAll(toAdd); + + cache.put(fqn, original); + assert loader.get(fqn).equals(original); + cache.evict(fqn); + assert loader.get(fqn).equals(original); + cache.put(fqn, toAdd); + assert loader.get(fqn).equals(all); + cache.evict(fqn); + assert loader.get(fqn).equals(all); + } + + public void testPutAllAfterEvictWithChild() throws Exception + { + Fqn fqn = Fqn.fromString("/a/b"); + Map<String, String> original = new HashMap<String, String>(); + Map<String, String> toAdd = new HashMap<String, String>(); + Map<String, String> all = new HashMap<String, String>(); + + original.put("1", "One"); + original.put("2", "Two"); + toAdd.put("3", "Three"); + toAdd.put("4", "Four"); + + all.putAll(original); + all.putAll(toAdd); + + cache.put(fqn, original); + cache.put(Fqn.fromRelativeElements(fqn, "c"), "x", "y"); // put stuff in a child so that evicting fqn will only clear its data map. + assert loader.get(fqn).equals(original); + cache.evict(fqn); + assert loader.get(fqn).equals(original); + cache.put(fqn, toAdd); + assert loader.get(fqn).equals(all); + cache.evict(fqn); + assert loader.get(fqn).equals(all); + } + + /** * Test load/store state. */ Modified: core/trunk/src/test/java/org/jboss/cache/loader/CacheLoaderWithReplicationTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/loader/CacheLoaderWithReplicationTest.java 2008-09-26 11:28:53 UTC (rev 6802) +++ core/trunk/src/test/java/org/jboss/cache/loader/CacheLoaderWithReplicationTest.java 2008-09-26 11:50:32 UTC (rev 6803) @@ -14,6 +14,7 @@ import org.jboss.cache.commands.tx.PrepareCommand; import org.jboss.cache.config.Configuration; import org.jboss.cache.config.Configuration.NodeLockingScheme; +import org.jboss.cache.transaction.DummyTransactionManagerLookup; import org.jboss.cache.util.TestingUtil; import org.jboss.cache.util.internals.ReplicationListener; import static org.testng.AssertJUnit.assertEquals; @@ -43,12 +44,12 @@ public void setUp() throws Exception { cache1 = new DefaultCacheFactory<Object, Object>().createCache(false); - cache1.getConfiguration().setCacheLoaderConfig(getSingleCacheLoaderConfig("", "org.jboss.cache.loader.DummyInMemoryCacheLoader", null, false, true, false)); - cache1.getConfiguration().setTransactionManagerLookupClass("org.jboss.cache.transaction.DummyTransactionManagerLookup"); + cache1.getConfiguration().setCacheLoaderConfig(getSingleCacheLoaderConfig("", DummyInMemoryCacheLoader.class.getName(), "debug=true", false, true, false)); + cache1.getConfiguration().setTransactionManagerLookupClass(DummyTransactionManagerLookup.class.getName()); cache2 = new DefaultCacheFactory<Object, Object>().createCache(false); - cache2.getConfiguration().setCacheLoaderConfig(getSingleCacheLoaderConfig("", "org.jboss.cache.loader.DummyInMemoryCacheLoader", null, false, true, false)); - cache2.getConfiguration().setTransactionManagerLookupClass("org.jboss.cache.transaction.DummyTransactionManagerLookup"); + cache2.getConfiguration().setCacheLoaderConfig(getSingleCacheLoaderConfig("", DummyInMemoryCacheLoader.class.getName(), "debug=true", false, true, false)); + cache2.getConfiguration().setTransactionManagerLookupClass(DummyTransactionManagerLookup.class.getName()); loader1 = loader2 = null; mgr1 = mgr2 = null; @@ -252,6 +253,10 @@ assertEquals("value", cache1.get(fqn, key)); assertEquals("value", cache2.get(fqn, key)); + + System.out.println(loader1.get(fqn)); + System.out.println(loader2.get(fqn)); + assertEquals("value", loader1.get(fqn).get(key)); assertEquals("value", loader2.get(fqn).get(key)); Modified: core/trunk/src/test/java/org/jboss/cache/loader/DummyInMemoryCacheLoader.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/loader/DummyInMemoryCacheLoader.java 2008-09-26 11:28:53 UTC (rev 6802) +++ core/trunk/src/test/java/org/jboss/cache/loader/DummyInMemoryCacheLoader.java 2008-09-26 11:50:32 UTC (rev 6803) @@ -181,6 +181,7 @@ { n = new DummyNode(name); } + n.data.clear(); // emulate cache loaders overwriting any internal data map with new data map passed in. if (attributes != null) n.data.putAll(injectNULLs(attributes)); getNodesMap().put(name, n); // we need to make sure parents get put in as well. Copied: core/trunk/src/test/java/org/jboss/cache/loader/DummyInMemoryCacheLoaderPessimisticTest.java (from rev 6786, core/trunk/src/test/java/org/jboss/cache/loader/DummyInMemoryCacheLoaderTest.java) =================================================================== --- core/trunk/src/test/java/org/jboss/cache/loader/DummyInMemoryCacheLoaderPessimisticTest.java (rev 0) +++ core/trunk/src/test/java/org/jboss/cache/loader/DummyInMemoryCacheLoaderPessimisticTest.java 2008-09-26 11:50:32 UTC (rev 6803) @@ -0,0 +1,22 @@ +package org.jboss.cache.loader; + +import org.jboss.cache.config.CacheLoaderConfig; +import org.jboss.cache.config.Configuration.NodeLockingScheme; +import org.testng.annotations.Test; + +/** + * Odd that we need a test for a test class, but if we intend to use the {@link DummyInMemoryCacheLoader} as a cache + * loader stub then we need to make sure it behaves as a valid cache loader. + */ +@Test(groups = {"functional"}) +public class DummyInMemoryCacheLoaderPessimisticTest extends CacheLoaderTestsBase +{ + protected void configureCache() throws Exception + { + // use the shared variation of the DIMCL so that state is persisted in a static variable in memory rather than an + // instance one. + CacheLoaderConfig clc = getSingleCacheLoaderConfig("", DummySharedInMemoryCacheLoader.class.getName(), "debug=true", false, true, false); + cache.getConfiguration().setCacheLoaderConfig(clc); + cache.getConfiguration().setNodeLockingScheme(NodeLockingScheme.PESSIMISTIC); + } +} \ No newline at end of file