[jbosscache-commits] JBoss Cache SVN: r5190 - in core/trunk/src: main/java/org/jboss/cache/transaction and 1 other directories.
jbosscache-commits at lists.jboss.org
jbosscache-commits at lists.jboss.org
Tue Jan 22 09:42:59 EST 2008
Author: mircea.markus
Date: 2008-01-22 09:42:59 -0500 (Tue, 22 Jan 2008)
New Revision: 5190
Modified:
core/trunk/src/main/java/org/jboss/cache/interceptors/PessimisticLockInterceptor.java
core/trunk/src/main/java/org/jboss/cache/transaction/TransactionTable.java
core/trunk/src/test/java/org/jboss/cache/lock/pessimistic/ConcurrentPutRemoveTest.java
Log:
http://jira.jboss.com/jira/browse/JBCACHE-1165
Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/PessimisticLockInterceptor.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/interceptors/PessimisticLockInterceptor.java 2008-01-22 13:30:36 UTC (rev 5189)
+++ core/trunk/src/main/java/org/jboss/cache/interceptors/PessimisticLockInterceptor.java 2008-01-22 14:42:59 UTC (rev 5190)
@@ -118,7 +118,7 @@
}
else
{
- acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.WRITE, true, false, false, true, null, false);
+ acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.WRITE, true, false, false, true, null);
}
return nextInterceptor(ctx);
}
@@ -130,7 +130,7 @@
protected Object handleLockMethod(InvocationContext ctx, Fqn fqn, NodeLock.LockType lockType, boolean recursive) throws Throwable
{
- acquireLocksWithTimeout(ctx, fqn, lockType, false, false, false, false, null, false);
+ acquireLocksWithTimeout(ctx, fqn, lockType, false, false, false, false, null);
if (recursive)
{
//acquireLocksOnChildren(cache.peek(fqn, false), lockType, ctx);
@@ -204,7 +204,7 @@
if (trace) log.trace("Attempting to get WL on node to be moved [" + from + "]");
if (from != null && !(configuration.getIsolationLevel() == IsolationLevel.NONE))
{
- lock(ctx, from, NodeLock.LockType.WRITE, false, timeout, true, false, null, false);
+ lock(ctx, from, NodeLock.LockType.WRITE, false, timeout, true, false, null);
if (ctx.getGlobalTransaction() != null)
{
cache.getTransactionTable().get(ctx.getGlobalTransaction()).addRemovedNode(from);
@@ -215,7 +215,7 @@
{
//now for an RL for the new parent.
if (trace) log.trace("Attempting to get RL on new parent [" + to + "]");
- lock(ctx, to, NodeLock.LockType.READ, false, timeout, false, false, null, false);
+ lock(ctx, to, NodeLock.LockType.READ, false, timeout, false, false, null);
acquireLocksOnChildren(peekNode(ctx, to, false, true, false), NodeLock.LockType.READ, ctx);
}
Object retValue = nextInterceptor(ctx);
@@ -234,7 +234,7 @@
List<Fqn> createdNodes = new LinkedList<Fqn>();
// we need to mark new nodes created as deleted since they are only created to form a path to the node being removed, to
// create a lock.
- boolean created = acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.WRITE, true, false, true, false, createdNodes, true);
+ boolean created = acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.WRITE, true, false, true, false, createdNodes);
if (ctx.getGlobalTransaction() != null)
{
TransactionEntry entry = tx_table.get(ctx.getGlobalTransaction());
@@ -263,7 +263,7 @@
protected Object handlePutForExternalReadMethod(InvocationContext ctx, GlobalTransaction tx, Fqn fqn, Object key, Object value) throws Throwable
{
- acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.READ, true, true, false, true, null, false);
+ acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.READ, true, true, false, true, null);
return nextInterceptor(ctx);
}
@@ -274,61 +274,61 @@
protected Object handleRemoveDataMethod(InvocationContext ctx, GlobalTransaction tx, Fqn fqn, boolean createUndoOps) throws Throwable
{
- acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.WRITE, false, false, false, false, null, false);
+ acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.WRITE, false, false, false, false, null);
return nextInterceptor(ctx);
}
protected Object handleAddChildMethod(InvocationContext ctx, GlobalTransaction tx, Fqn parentFqn, Object childName, Node cn, boolean createUndoOps) throws Throwable
{
- acquireLocksWithTimeout(ctx, parentFqn, NodeLock.LockType.READ, false, false, false, false, null, false);
+ acquireLocksWithTimeout(ctx, parentFqn, NodeLock.LockType.READ, false, false, false, false, null);
return nextInterceptor(ctx);
}
protected Object handleEvictMethod(InvocationContext ctx, Fqn fqn) throws Throwable
{
- acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.WRITE, false, true, false, false, null, false);
+ acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.WRITE, false, true, false, false, null);
return nextInterceptor(ctx);
}
protected Object handleGetKeyValueMethod(InvocationContext ctx, Fqn fqn, Object key, boolean sendNodeEvent) throws Throwable
{
- acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.READ, false, false, false, false, null, false);
+ acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.READ, false, false, false, false, null);
return nextInterceptor(ctx);
}
protected Object handleGetNodeMethod(InvocationContext ctx, Fqn fqn) throws Throwable
{
- acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.READ, false, false, false, false, null, false);
+ acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.READ, false, false, false, false, null);
return nextInterceptor(ctx);
}
protected Object handleGetKeysMethod(InvocationContext ctx, Fqn fqn) throws Throwable
{
- acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.READ, false, false, false, false, null, false);
+ acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.READ, false, false, false, false, null);
return nextInterceptor(ctx);
}
protected Object handleGetChildrenNamesMethod(InvocationContext ctx, Fqn fqn) throws Throwable
{
- acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.READ, false, false, false, false, null, false);
+ acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.READ, false, false, false, false, null);
return nextInterceptor(ctx);
}
protected Object handlePrintMethod(InvocationContext ctx, Fqn fqn) throws Throwable
{
- acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.READ, false, false, false, false, null, false);
+ acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.READ, false, false, false, false, null);
return nextInterceptor(ctx);
}
protected Object handleReleaseAllLocksMethod(InvocationContext ctx, Fqn fqn) throws Throwable
{
- acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.READ, false, false, false, false, null, false);
+ acquireLocksWithTimeout(ctx, fqn, NodeLock.LockType.READ, false, false, false, false, null);
return nextInterceptor(ctx);
}
private boolean acquireLocksWithTimeout(InvocationContext ctx, Fqn fqn, NodeLock.LockType lockType,
boolean createIfNotExists, boolean zeroLockTimeout,
- boolean acquireLockOnParent, boolean reverseRemoveCheck, List<Fqn> createdNodes, boolean markNewNodesAsDeleted)
+ boolean acquireLockOnParent, boolean reverseRemoveCheck, List<Fqn> createdNodes)
throws InterruptedException
{
if (fqn == null || configuration.getIsolationLevel() == IsolationLevel.NONE) return false;
@@ -345,7 +345,7 @@
{
throw new TimeoutException("Unable to acquire lock on Fqn " + fqn + " after " + timeout + " millis");
}
- created = lock(ctx, fqn, lockType, createIfNotExists, timeout, acquireLockOnParent, reverseRemoveCheck, createdNodes, markNewNodesAsDeleted);
+ created = lock(ctx, fqn, lockType, createIfNotExists, timeout, acquireLockOnParent, reverseRemoveCheck, createdNodes);
firstTry = false;
}
while (createIfNotExists && peekNode(ctx, fqn, false, true, false) == null);// keep trying until we have the lock (fixes concurrent remove())
@@ -362,10 +362,9 @@
* reach a node that does not exists
* @param reverseRemoveCheck see {@link #manageReverseRemove(org.jboss.cache.transaction.GlobalTransaction, org.jboss.cache.NodeSPI, boolean)}
* @param createdNodes a list to which any nodes created can register their Fqns so that calling code is aware of which nodes have been newly created.
- * @param markNewNodesAsDeleted
*/
private boolean lock(InvocationContext ctx, Fqn fqn, NodeLock.LockType lockType, boolean createIfNotExists, long timeout,
- boolean acquireWriteLockOnParent, boolean reverseRemoveCheck, List<Fqn> createdNodes, boolean markNewNodesAsDeleted)
+ boolean acquireWriteLockOnParent, boolean reverseRemoveCheck, List<Fqn> createdNodes)
throws TimeoutException, LockingException, InterruptedException
{
Thread currentThread = Thread.currentThread();
@@ -391,11 +390,10 @@
if (createIfNotExists)
{
// if the new node is to be marked as deleted, do not notify!
- currentNode = parent.addChildDirect(new Fqn(childName), !markNewNodesAsDeleted);
+ currentNode = parent.addChildDirect(new Fqn(childName), true);
created = true;
- if (trace) log.trace("Child node was null, so created child node " + childName);
+ if (trace) log.trace("Child node was null, so created child node " + childName + System.identityHashCode(currentNode));
if (createdNodes != null) createdNodes.add(currentNode.getFqn());
- if (markNewNodesAsDeleted) currentNode.markAsDeleted(true);
}
else
{
@@ -414,11 +412,10 @@
lockTypeRequired = NodeLock.LockType.WRITE;
}
- manageReverseRemove(gtx, currentNode, reverseRemoveCheck);
-
// actually acquire the lock we need. This method blocks.
acquireNodeLock(currentNode, owner, gtx, lockTypeRequired, timeout);
+ manageReverseRemove(gtx, currentNode, reverseRemoveCheck);
// make sure the lock we acquired isn't on a deleted node/is an orphan!!
// look into invalidated nodes as well
NodeSPI repeek = peekNode(ctx, currentNode.getFqn(), true, true, true);
@@ -433,14 +430,14 @@
if (peekNode(ctx, parent.getFqn(), true, true, true) == null)
{
// crap!
- if (trace)
- log.trace("Parent has been deleted again. Go through the lock method all over again.");
+ if (trace) log.trace("Parent has been deleted again. Go through the lock method all over again.");
currentNode = rootNode;
parent = null;
}
else
{
- // do the loop again, but don't assign child_node to currentNode so that child_node is processed again.
+ currentNode = parent;
+ parent = null;
if (System.currentTimeMillis() > expiryTime)
{
throw new TimeoutException("Unable to acquire lock on child node " + new Fqn(currentNode.getFqn(), childName) + " after " + timeout + " millis.");
@@ -565,7 +562,7 @@
*/
private void manageReverseRemove(GlobalTransaction gtx, NodeSPI childNode, boolean reverseRemoveCheck)
{
- boolean needToReverseRemove = reverseRemoveCheck && childNode.isDeleted() && tx_table.get(gtx).getRemovedNodes().contains(childNode.getFqn());
+ boolean needToReverseRemove = reverseRemoveCheck && childNode.isDeleted() && tx_table.isNodeRemovedInTx(gtx,childNode.getFqn());
if (gtx != null && needToReverseRemove)
{
childNode.markAsDeleted(false);
Modified: core/trunk/src/main/java/org/jboss/cache/transaction/TransactionTable.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/transaction/TransactionTable.java 2008-01-22 13:30:36 UTC (rev 5189)
+++ core/trunk/src/main/java/org/jboss/cache/transaction/TransactionTable.java 2008-01-22 14:42:59 UTC (rev 5190)
@@ -9,6 +9,7 @@
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.jboss.cache.CacheException;
+import org.jboss.cache.Fqn;
import org.jboss.cache.lock.NodeLock;
import org.jboss.cache.marshall.MethodCall;
@@ -286,4 +287,10 @@
throw e;
}
}
+
+ public boolean isNodeRemovedInTx(GlobalTransaction gtx, Fqn fqn)
+ {
+ TransactionEntry te = get(gtx);
+ return te != null && te.getRemovedNodes().contains(fqn);
+ }
}
Modified: core/trunk/src/test/java/org/jboss/cache/lock/pessimistic/ConcurrentPutRemoveTest.java
===================================================================
--- core/trunk/src/test/java/org/jboss/cache/lock/pessimistic/ConcurrentPutRemoveTest.java 2008-01-22 13:30:36 UTC (rev 5189)
+++ core/trunk/src/test/java/org/jboss/cache/lock/pessimistic/ConcurrentPutRemoveTest.java 2008-01-22 14:42:59 UTC (rev 5190)
@@ -16,6 +16,7 @@
import javax.transaction.TransactionManager;
import java.util.ArrayList;
import java.util.List;
+import java.net.URL;
// This is disabled because the fix is not absolute and will require pretty bug architectural changes.
// There is an edge case where a NodeNotFoundException may occur, and this is due to parent nodes not being
@@ -24,7 +25,7 @@
// The problem is in the way READ_COMMITTED is implemented, i.e., writers are not blocked by readers and this
// allows a reader to hold a lock when a writer comes in and deletes the node in question.
- at Test(groups = {"functional"}, enabled = false)
+ at Test(groups = {"functional"}, enabled = true)
// Known issue - See JBCACHE-1164 and JBCACHE-1165
public class ConcurrentPutRemoveTest
{
@@ -60,7 +61,7 @@
}
}
- @Test(invocationCount = 50, enabled = false)
+ @Test(invocationCount = 10, enabled = true)
public void testLock() throws Exception
{
for (int x = 0; x < 2; x++)
More information about the jbosscache-commits
mailing list