Author: manik.surtani(a)jboss.com Date: 2008-09-26 07:28:53 -0400 (Fri, 26 Sep 2008) New Revision: 6802 Removed: core/branches/2.2.X/src/main/java/org/jboss/cache/util/concurrent/locks/ Modified: core/branches/2.2.X/src/main/java/org/jboss/cache/loader/AdjListJDBCCacheLoader.java core/branches/2.2.X/src/main/java/org/jboss/cache/loader/AdjListJDBCCacheLoaderConfig.java core/branches/2.2.X/src/main/java/org/jboss/cache/loader/JDBCCacheLoader.java core/branches/2.2.X/src/main/java/org/jboss/cache/loader/JDBCCacheLoaderOld.java core/branches/2.2.X/src/main/java/org/jboss/cache/lock/StripedLock.java Log: Better impl for JBCACHE-1410: JDBCCacheLoader may attempt to create a node entry in the DB twice Modified: core/branches/2.2.X/src/main/java/org/jboss/cache/loader/AdjListJDBCCacheLoader.java =================================================================== --- core/branches/2.2.X/src/main/java/org/jboss/cache/loader/AdjListJDBCCacheLoader.java 2008-09-26 11:20:35 UTC (rev 6801) +++ core/branches/2.2.X/src/main/java/org/jboss/cache/loader/AdjListJDBCCacheLoader.java 2008-09-26 11:28:53 UTC (rev 6802) @@ -243,8 +243,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() { @@ -411,10 +449,11 @@ /** * Inserts a node into the database * - * @param name the fqn - * @param node the node + * @param name the fqn + * @param node 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 node) + protected void insertNode(Fqn name, Map node, boolean rowMayExist) { Connection con = null; PreparedStatement ps = null; @@ -428,8 +467,12 @@ con = cf.getConnection(); ps = con.prepareStatement(config.getInsertNodeSql()); - ps.setString(1, name.toString()); + String fqnString = name.toString(); + // the Fqn needs to be in the 1st and 4th places. + ps.setString(1, fqnString); + ps.setString(4, fqnString); + if (node != null) { // ByteArrayOutputStream baos = new ByteArrayOutputStream(); @@ -465,7 +508,7 @@ } int rows = ps.executeUpdate(); - if (rows != 1) + if (!rowMayExist && rows != 1) { throw new IllegalStateException("Expected one insert row but got " + rows); } Modified: core/branches/2.2.X/src/main/java/org/jboss/cache/loader/AdjListJDBCCacheLoaderConfig.java =================================================================== --- core/branches/2.2.X/src/main/java/org/jboss/cache/loader/AdjListJDBCCacheLoaderConfig.java 2008-09-26 11:20:35 UTC (rev 6801) +++ core/branches/2.2.X/src/main/java/org/jboss/cache/loader/AdjListJDBCCacheLoaderConfig.java 2008-09-26 11:28:53 UTC (rev 6802) @@ -579,14 +579,44 @@ 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/branches/2.2.X/src/main/java/org/jboss/cache/loader/JDBCCacheLoader.java =================================================================== --- core/branches/2.2.X/src/main/java/org/jboss/cache/loader/JDBCCacheLoader.java 2008-09-26 11:20:35 UTC (rev 6801) +++ core/branches/2.2.X/src/main/java/org/jboss/cache/loader/JDBCCacheLoader.java 2008-09-26 11:28:53 UTC (rev 6802) @@ -240,21 +240,13 @@ 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/branches/2.2.X/src/main/java/org/jboss/cache/loader/JDBCCacheLoaderOld.java =================================================================== --- core/branches/2.2.X/src/main/java/org/jboss/cache/loader/JDBCCacheLoaderOld.java 2008-09-26 11:20:35 UTC (rev 6801) +++ core/branches/2.2.X/src/main/java/org/jboss/cache/loader/JDBCCacheLoaderOld.java 2008-09-26 11:28:53 UTC (rev 6802) @@ -135,11 +135,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; @@ -317,11 +317,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/branches/2.2.X/src/main/java/org/jboss/cache/lock/StripedLock.java =================================================================== --- core/branches/2.2.X/src/main/java/org/jboss/cache/lock/StripedLock.java 2008-09-26 11:20:35 UTC (rev 6801) +++ core/branches/2.2.X/src/main/java/org/jboss/cache/lock/StripedLock.java 2008-09-26 11:28:53 UTC (rev 6802) @@ -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)]; }