[jbosscache-commits] JBoss Cache SVN: r4509 - in core/branches/1.4.X: src/org/jboss/cache/interceptors and 2 other directories.
jbosscache-commits at lists.jboss.org
jbosscache-commits at lists.jboss.org
Wed Sep 26 14:41:27 EDT 2007
Author: manik.surtani at jboss.com
Date: 2007-09-26 14:41:27 -0400 (Wed, 26 Sep 2007)
New Revision: 4509
Removed:
core/branches/1.4.X/src/org/jboss/cache/NodeCreationResult.java
core/branches/1.4.X/tests/functional/org/jboss/cache/lock/pessimistic/
core/branches/1.4.X/tests/functional/org/jboss/cache/transaction/AsyncRollbackTxLockTest.java
core/branches/1.4.X/tests/functional/org/jboss/cache/transaction/SimultaneousRollbackAndPutTest.java
Modified:
core/branches/1.4.X/src/org/jboss/cache/DataNode.java
core/branches/1.4.X/src/org/jboss/cache/Node.java
core/branches/1.4.X/src/org/jboss/cache/TransactionTable.java
core/branches/1.4.X/src/org/jboss/cache/interceptors/PessimisticLockInterceptor.java
Log:
Reversed fixes and tests for JBCACHE-1165, JBCACHE-1166, JBCACHE-1168, JBCACHE-1164, JBCACHE-1183, to release 1.4.1.SP5
Modified: core/branches/1.4.X/src/org/jboss/cache/DataNode.java
===================================================================
--- core/branches/1.4.X/src/org/jboss/cache/DataNode.java 2007-09-26 17:33:33 UTC (rev 4508)
+++ core/branches/1.4.X/src/org/jboss/cache/DataNode.java 2007-09-26 18:41:27 UTC (rev 4509)
@@ -51,14 +51,4 @@
void unmarkForRemoval(boolean deep);
void markForRemoval();
- /**
- * This method hs identical functionality as getOrCreateChild, but returns a NodeCreationResult.
- * If the node was created, NodeCreationResult#isCreated() returns true.
- * @param name name of child to create
- * @param tx transaction under which to create child
- * @param createIfNotExists if false don't attempt to create the node if it doesn't exist
- * @return a NodeCreationResult instance
- */
- NodeCreationResult getOrCreateChildWithCreateInformation(Object name, GlobalTransaction tx, boolean createIfNotExists);
-
}
Modified: core/branches/1.4.X/src/org/jboss/cache/Node.java
===================================================================
--- core/branches/1.4.X/src/org/jboss/cache/Node.java 2007-09-26 17:33:33 UTC (rev 4508)
+++ core/branches/1.4.X/src/org/jboss/cache/Node.java 2007-09-26 18:41:27 UTC (rev 4509)
@@ -318,64 +318,55 @@
return this.data().put(key, value);
}
}
-
- public NodeCreationResult getOrCreateChildWithCreateInformation(Object child_name, GlobalTransaction gtx, boolean createIfNotExists){
- NodeCreationResult result = new NodeCreationResult();
- DataNode child;
- if (child_name == null)
- throw new IllegalArgumentException("null child name");
- child = (DataNode) children().get(child_name);
- if (createIfNotExists && child == null)
- {
- if (log.isTraceEnabled()) log.trace("Children:"+children);
- // construct the new child outside the synchronized block to avoid
- // spending any more time than necessary in the synchronized section
- Fqn child_fqn = new Fqn(this.fqn, child_name);
- DataNode newChild = (DataNode) NodeFactory.getInstance().createNodeOfType(this, child_name, child_fqn, this, null, cache);
- if (newChild == null)
- throw new IllegalStateException();
- synchronized (this)
- {
- // check again to see if the child exists
- // after acquiring exclusive lock
- child = (Node) children().get(child_name);
- if (child == null)
- {
- child = newChild;
- children.put(child_name, child);
- if (gtx != null)
- {
- MethodCall undo_op = MethodCallFactory.create(MethodDeclarations.removeNodeMethodLocal,
- new Object[]{gtx, child_fqn, Boolean.FALSE});
- cache.addUndoOperation(gtx, undo_op);
- // add the node name to the list maintained for the current tx
- // (needed for abort/rollback of transaction)
- // cache.addNode(gtx, child.getFqn());
- }
- }
- }
-
- // notify if we actually created a new child
- if (newChild == child)
- {
- if (trace)
- {
- log.trace("created child: fqn=" + child_fqn);
- }
- result.setCreated(true);
- cache.notifyNodeCreated(child.getFqn());
- }else{
- result.setCreated(false);
- }
- }
- result.setTreeNode(child);
- return result;
- }
-
public TreeNode getOrCreateChild(Object child_name, GlobalTransaction gtx, boolean createIfNotExists)
{
- return getOrCreateChildWithCreateInformation(child_name, gtx, createIfNotExists).getTreeNode();
+ DataNode child;
+ if (child_name == null)
+ throw new IllegalArgumentException("null child name");
+
+ child = (DataNode) children().get(child_name);
+ if (createIfNotExists && child == null)
+ {
+ // construct the new child outside the synchronized block to avoid
+ // spending any more time than necessary in the synchronized section
+ Fqn child_fqn = new Fqn(this.fqn, child_name);
+ DataNode newChild = (DataNode) NodeFactory.getInstance().createNodeOfType(this, child_name, child_fqn, this, null, cache);
+ if (newChild == null)
+ throw new IllegalStateException();
+ synchronized (this)
+ {
+ // check again to see if the child exists
+ // after acquiring exclusive lock
+ child = (Node) children().get(child_name);
+ if (child == null)
+ {
+ child = newChild;
+ children.put(child_name, child);
+ if (gtx != null)
+ {
+ MethodCall undo_op = MethodCallFactory.create(MethodDeclarations.removeNodeMethodLocal,
+ new Object[]{gtx, child_fqn, Boolean.FALSE});
+ cache.addUndoOperation(gtx, undo_op);
+ // add the node name to the list maintained for the current tx
+ // (needed for abort/rollback of transaction)
+ // cache.addNode(gtx, child.getFqn());
+ }
+ }
+ }
+
+ // notify if we actually created a new child
+ if (newChild == child)
+ {
+ if (trace)
+ {
+ log.trace("created child: fqn=" + child_fqn);
+ }
+ cache.notifyNodeCreated(child.getFqn());
+ }
+ }
+ return child;
+
}
public TreeNode createChild(Object child_name, Fqn fqn, TreeNode parent)
@@ -516,13 +507,13 @@
if (trace)
{
log.trace(new StringBuffer("acquiring RL: fqn=").append(fqn).append(", caller=").append(caller).
- append(", lock=").append(lock_.toString(DataNode.PRINT_LOCK_DETAILS)).append("lock_hc="+lock_.hashCode()).append(", this._hc=").append(this.hashCode()));
+ append(", lock=").append(lock_.toString(DataNode.PRINT_LOCK_DETAILS)));
}
boolean flag = lock_.acquireReadLock(caller, timeout);
if (trace)
{
log.trace(new StringBuffer("acquired RL: fqn=").append(fqn).append(", caller=").append(caller).
- append(", lock=").append(lock_.toString(DataNode.PRINT_LOCK_DETAILS)).append("lock_hc="+lock_.hashCode()).append(", this._hc=").append(this.hashCode()));
+ append(", lock=").append(lock_.toString(DataNode.PRINT_LOCK_DETAILS)));
}
return flag;
}
@@ -533,13 +524,13 @@
if (trace)
{
log.trace(new StringBuffer("acquiring WL: fqn=").append(fqn).append(", caller=").append(caller).
- append(", lock=").append(lock_.toString(DataNode.PRINT_LOCK_DETAILS)).append("lock_hc="+lock_.hashCode()).append(", this._hc=").append(this.hashCode()));
+ append(", lock=").append(lock_.toString(DataNode.PRINT_LOCK_DETAILS)));
}
boolean flag = lock_.acquireWriteLock(caller, timeout);
if (trace)
{
log.trace(new StringBuffer("acquired WL: fqn=").append(fqn).append(", caller=").append(caller).
- append(", lock=").append(lock_.toString(DataNode.PRINT_LOCK_DETAILS)).append("lock_hc="+lock_.hashCode()).append(", this._hc=").append(this.hashCode()));
+ append(", lock=").append(lock_.toString(DataNode.PRINT_LOCK_DETAILS)));
}
return flag;
}
Deleted: core/branches/1.4.X/src/org/jboss/cache/NodeCreationResult.java
===================================================================
--- core/branches/1.4.X/src/org/jboss/cache/NodeCreationResult.java 2007-09-26 17:33:33 UTC (rev 4508)
+++ core/branches/1.4.X/src/org/jboss/cache/NodeCreationResult.java 2007-09-26 18:41:27 UTC (rev 4509)
@@ -1,19 +0,0 @@
-package org.jboss.cache;
-
-public class NodeCreationResult
-{
- private TreeNode treeNode;
- private boolean created;
- public boolean isCreated() {
- return created;
- }
- public void setCreated(boolean created) {
- this.created = created;
- }
- public TreeNode getTreeNode() {
- return treeNode;
- }
- public void setTreeNode(TreeNode treeNode) {
- this.treeNode = treeNode;
- }
-}
Modified: core/branches/1.4.X/src/org/jboss/cache/TransactionTable.java
===================================================================
--- core/branches/1.4.X/src/org/jboss/cache/TransactionTable.java 2007-09-26 17:33:33 UTC (rev 4508)
+++ core/branches/1.4.X/src/org/jboss/cache/TransactionTable.java 2007-09-26 18:41:27 UTC (rev 4509)
@@ -179,45 +179,39 @@
/**
* Adds a lock to the global transaction.
- * @return true if the lock was added
*/
- public boolean addLock(GlobalTransaction gtx, IdentityLock l) {
+ public void addLock(GlobalTransaction gtx, IdentityLock l) {
TransactionEntry entry=get(gtx);
if(entry == null) {
log.error("transaction entry not found for (gtx=" + gtx + ")");
- return false;
+ return;
}
entry.addLock(l);
- return true;
}
/**
* Adds a collection of locks to the global transaction.
- * @return true if the locks were added
*/
- public boolean addLocks(GlobalTransaction gtx, Collection locks) {
+ public void addLocks(GlobalTransaction gtx, Collection locks) {
TransactionEntry entry=get(gtx);
if(entry == null) {
log.error("transaction entry not found for (gtx=" + gtx + ")");
- return false;
+ return;
}
entry.addLocks(locks);
- return true;
}
/**
* Adds a node that has been removed to the global transaction
- * @return true if the removed node was added to the list of removed nodes
*/
- public boolean addRemovedNode(GlobalTransaction gtx, Fqn fqn)
+ public void addRemovedNode(GlobalTransaction gtx, Fqn fqn)
{
TransactionEntry entry=get(gtx);
if(entry == null) {
log.error("transaction entry not found for (gtx=" + gtx + ")");
- return false;
+ return;
}
entry.addRemovedNode(fqn);
- return true;
}
/**
Modified: core/branches/1.4.X/src/org/jboss/cache/interceptors/PessimisticLockInterceptor.java
===================================================================
--- core/branches/1.4.X/src/org/jboss/cache/interceptors/PessimisticLockInterceptor.java 2007-09-26 17:33:33 UTC (rev 4508)
+++ core/branches/1.4.X/src/org/jboss/cache/interceptors/PessimisticLockInterceptor.java 2007-09-26 18:41:27 UTC (rev 4509)
@@ -8,14 +8,11 @@
import org.jboss.cache.DataNode;
import org.jboss.cache.Fqn;
-import org.jboss.cache.NodeCreationResult;
import org.jboss.cache.GlobalTransaction;
import org.jboss.cache.InvocationContext;
-import org.jboss.cache.Node;
import org.jboss.cache.TransactionEntry;
import org.jboss.cache.TransactionTable;
import org.jboss.cache.TreeCache;
-import org.jboss.cache.TreeNode;
import org.jboss.cache.lock.IdentityLock;
import org.jboss.cache.lock.IsolationLevel;
import org.jboss.cache.lock.LockingException;
@@ -44,6 +41,7 @@
public class PessimisticLockInterceptor extends Interceptor
{
TransactionTable tx_table = null;
+
boolean writeLockOnChildInsertRemove = true;
/**
@@ -117,9 +115,6 @@
lock_timeout = ((Long) args[5]).longValue();
break;
case MethodDeclarations.removeNodeMethodLocal_id:
- // createIfNotExists is set to true to prevent the endless loop in JBCACHE-1165.
- // See http://jboss.org/index.html?module=bb&op=viewtopic&t=118186 for an explanation of this.
- createIfNotExists = true;
fqn = (Fqn) args[1];
lock_type = DataNode.LOCK_TYPE_WRITE;
recursive = true; // remove node and *all* child nodes
@@ -173,112 +168,25 @@
// If no TX: add each acquired lock to the list of locks for this method (locks)
// If TX: [merge code from TransactionInterceptor]: register with TxManager, on commit/rollback,
// release the locks for the given TX
- /*if (fqn != null)
+ if (fqn != null)
{
- // limit the time spent in the loop attempting to acquire locks.
- long startTime = System.currentTimeMillis();
- boolean finish;
-
- do
+ if (createIfNotExists)
{
- GlobalTransaction gtx = ctx.getGlobalTransaction();
- // first try and lock the node in question.
- DataNode lockedNode = lock(fqn, gtx, lock_type, recursive, zeroLockTimeout ? 0 : lock_timeout, createIfNotExists, storeLockedNode, isRemoveData);
-
- // test whether we need to retry getting the lock.
- finish = !createIfNotExists;
-
- // if we need to create the node, or the locked node was not null, we need to test if this is the *same* node that is in the cache
- // to prevent a race condition between concurrent creation and deletion.
- if (createIfNotExists || lockedNode != null)
+ do
{
- DataNode dataNode = getNodeFromCache(fqn, ctx.getGlobalTransaction());// fetch node from cache
- finish = finish || dataNode != null;
- // compare the two nodes. These *should* be the same object instances. If not, we have a problem - try and re-acquire the lock.
- if (dataNode != lockedNode)
- {
- finish = false;
- if (log.isDebugEnabled()) log.debug("Lock was acquired but node changed (createIfNotExists=" + createIfNotExists + ")");
- releaseLock(lockedNode.getLock(), gtx == null ? (Object) Thread.currentThread() : gtx);
- }
+ lock(fqn, ctx.getGlobalTransaction(), lock_type, recursive, zeroLockTimeout ? 0 : lock_timeout, createIfNotExists, storeLockedNode, isRemoveData);
}
-
- // additional timeout check - if we haven't finished and have taken > lock_timeout milliseconds, throw a TimeoutException.
- // if zeroLockTimeout is specified, there is no time for retries!
- if (!finish && (zeroLockTimeout || System.currentTimeMillis() - startTime > lock_timeout))
- {
- String msg = "PessimicticLockInterceptor can't acquire lock after [" + lock_timeout + "] ms";
- log.error(msg);
- throw new TimeoutException(msg);
- }
-
- } while (!finish); // keep trying until we have the lock, or we time out.
+ while(!cache.exists(fqn)); // keep trying until we have the lock (fixes concurrent remove())
+ // terminates successfully, or with (Timeout)Exception
+ }
+ else
+ lock(fqn, ctx.getGlobalTransaction(), lock_type, recursive, zeroLockTimeout ? 0 : lock_timeout, createIfNotExists, storeLockedNode, isRemoveData);
}
else
{
if (log.isTraceEnabled())
log.trace("bypassed locking as method " + m.getName() + "() doesn't require locking");
- }*/
-
-
- if (fqn != null)
- {
- long startTime = System.currentTimeMillis();//we can;t spend in this loop more than lock_timeout!
- if (createIfNotExists)
- {
- boolean finish = false;
- do
- {
- if (!zeroLockTimeout && (System.currentTimeMillis()-startTime>lock_timeout)){
- String msg = "PessimicticLockInterceptor can't acquire lock after ["+lock_timeout+"] ms.";
- log.error(msg);
- throw new TimeoutException(msg);
- }
- DataNode lockedNode = lock(fqn, ctx.getGlobalTransaction(), lock_type, recursive, zeroLockTimeout ? 0 : lock_timeout, createIfNotExists, storeLockedNode, isRemoveData);
- DataNode dataNode = getNodeFromCache(fqn,ctx.getGlobalTransaction());
- finish = dataNode!=null;
- if (finish && lockedNode!=null){
- if (dataNode!=lockedNode){
- finish = false;
- log.trace("Lock was acquired but node changed (createIfNotExists=true)");
- }
- }
- }
- while(!finish); // keep trying until we have the lock (fixes concurrent remove())
- // terminates successfully, or with (Timeout)Exception
- }
- else{
- boolean finish = false;
- do
- {
- if (!zeroLockTimeout && (System.currentTimeMillis()-startTime>lock_timeout)){
- String msg = "PessimicticLockInterceptor can't acquire lock after ["+lock_timeout+"] ms";
- log.error(msg);
- throw new TimeoutException(msg);
- }
- DataNode lockedNode = lock(fqn, ctx.getGlobalTransaction(), lock_type, recursive, zeroLockTimeout ? 0 : lock_timeout, createIfNotExists, storeLockedNode, isRemoveData);
- if (lockedNode!=null){
- DataNode dataNode = getNodeFromCache(fqn,ctx.getGlobalTransaction());
- if (dataNode!=lockedNode){
- finish = false;
- log.trace("Lock was acquired but node changed (createIfNotExists=false)");
- }else{
- finish=true;
- }
- }else{
- finish = true;
- }
- }
- while(!finish); // keep trying until we have the lock (fixes concurrent remove())
- }
}
- else
- {
- if (log.isTraceEnabled())
- log.trace("bypassed locking as method " + m.getName() + "() doesn't require locking");
- }
-
-
if (m.getMethodId() == MethodDeclarations.lockMethodLocal_id)
return null;
@@ -301,44 +209,9 @@
return o;
}
- /**
- * Return node from cache, include nodes marked as removal only if they are removed by this transaction
- *
- * @param fqn fqn
- * @return DataNode
- */
- private DataNode getNodeFromCache(Fqn fqn, GlobalTransaction gtx)
- {
- DataNode root = cache.getRoot();
- if (fqn == null || fqn.size() == 0) return root;
- TreeNode n = root;
- int fqnSize = fqn.size();
- for (int i = 0; i < fqnSize; i++)
- {
- Object obj = fqn.get(i);
- n = n.getChild(obj);
- if (n == null)
- return null;
- else if (((DataNode) n).isMarkedForRemoval())
- {
- if (gtx != null && !tx_table.get(gtx).getRemovedNodes().contains(n.getFqn()))
- {
- return null;
- }
- }
- }
- return (Node) n;
- }
-
-
private void cleanup(GlobalTransaction gtx)
{
TransactionEntry entry = tx_table.get(gtx);
- if (entry == null)
- {
- if (log.isInfoEnabled()) log.info("Unable to clean up since transaction entry for tx " + gtx + " is null. Perhaps the transaction has timed out?");
- return;
- }
// Let's do it in stack style, LIFO
entry.releaseAllLocksLIFO(gtx);
@@ -358,11 +231,10 @@
* @param gtx
* @param lock_type DataNode.LOCK_TYPE_READ, DataNode.LOCK_TYPE_WRITE or DataNode.LOCK_TYPE_NONE
* @param recursive Lock children recursively
- * @return node on which lock was acquired or null if lock was not acquired
*/
- private DataNode lock(Fqn fqn, GlobalTransaction gtx, int lock_type, boolean recursive,
- long lock_timeout, boolean createIfNotExists, boolean isRemoveNodeOperation, boolean isRemoveDataOperation)
- throws TimeoutException, LockingException, InterruptedException
+ private void lock(Fqn fqn, GlobalTransaction gtx, int lock_type, boolean recursive,
+ long lock_timeout, boolean createIfNotExists, boolean isRemoveNodeOperation, boolean isRemoveDataOperation)
+ throws TimeoutException, LockingException, InterruptedException
{
DataNode n;
DataNode child_node;
@@ -378,11 +250,11 @@
if (fqn == null)
{
log.error("fqn is null - this should not be the case");
- return null;
+ return;
}
if ((treeNodeSize = fqn.size()) == 0)
- return null;
+ return;
if (cache.getIsolationLevelClass() == IsolationLevel.NONE)
lock_type = DataNode.LOCK_TYPE_NONE;
@@ -390,7 +262,6 @@
n = cache.getRoot();
for (int i = -1; i < treeNodeSize; i++)
{
- boolean nodeCreated = false;
if (i == -1)
{
child_name = Fqn.ROOT.getName();
@@ -399,17 +270,14 @@
else
{
child_name = fqn.get(i);
-// child_node = (DataNode) n.getOrCreateChild(child_name, gtx, createIfNotExists);
- NodeCreationResult result = n.getOrCreateChildWithCreateInformation(child_name, gtx, createIfNotExists);
- child_node = (DataNode) result.getTreeNode();
- nodeCreated = result.isCreated();
+ child_node = (DataNode) n.getOrCreateChild(child_name, gtx, createIfNotExists);
}
if (child_node == null)
{
if (log.isTraceEnabled())
log.trace("failed to find or create child " + child_name + " of node " + n.getFqn());
- return null;
+ return;
}
if (lock_type == DataNode.LOCK_TYPE_NONE)
@@ -420,7 +288,7 @@
}
else
{
- if (nodeCreated || writeLockNeeded(lock_type, i, treeNodeSize, isRemoveNodeOperation, createIfNotExists, isRemoveDataOperation, fqn, child_node.getFqn()))
+ if (writeLockNeeded(lock_type, i, treeNodeSize, isRemoveNodeOperation, createIfNotExists, isRemoveDataOperation, fqn, child_node.getFqn()))
{
currentLockType = DataNode.LOCK_TYPE_WRITE;
}
@@ -429,12 +297,14 @@
currentLockType = DataNode.LOCK_TYPE_READ;
}
}
+
// reverse the "remove" if the node has been previously removed in the same tx, if this operation is a put()
if (gtx != null && needToReverseRemove(child_node, tx_table.get(gtx), lock_type, isRemoveNodeOperation, createIfNotExists))
{
reverseRemove(child_node);
}
+
// Try to acquire the lock; recording that we did if successful
acquireNodeLock(child_node, owner, gtx, currentLockType, lock_timeout);
@@ -446,12 +316,7 @@
{
if (gtx != null)
{
- boolean reallyAdded = tx_table.addLocks(gtx, acquired_locks);
- if (!reallyAdded)
- {
- releaseLocks(acquired_locks, owner);
- throw new LockingException("Locks can't be added to current transaction table (transaction timed out?)");
- }
+ cache.getTransactionTable().addLocks(gtx, acquired_locks);
}
else
{
@@ -465,21 +330,13 @@
}
// Add the Fqn to be removed to the transaction entry so we can clean up after ourselves during commit/rollback
- if (isRemoveNodeOperation && gtx != null)
- {
- if (!tx_table.addRemovedNode(gtx, fqn))
- {
- throw new LockingException("Removed node can't be added to current transaction table (transaction timed out?)");
- }
- }
- return n;
+ if (isRemoveNodeOperation && gtx != null) cache.getTransactionTable().get(gtx).addRemovedNode(fqn);
}
-
private boolean needToReverseRemove(DataNode n, TransactionEntry te, int lockTypeRequested, boolean isRemoveOperation, boolean createIfNotExists)
{
return !isRemoveOperation && createIfNotExists && lockTypeRequested == DataNode.LOCK_TYPE_WRITE && n.isMarkedForRemoval()
- && hasBeenRemovedInCurrentTx(te, n.getFqn());
+ && hasBeenRemovedInCurrentTx(te, n.getFqn());
}
private boolean hasBeenRemovedInCurrentTx(TransactionEntry te, Fqn f)
@@ -525,50 +382,17 @@
if (acquired)
{
// Record the lock for release on method return or tx commit/rollback
- recordNodeLock(gtx, node.getLock(), owner);
+ recordNodeLock(gtx, node.getLock());
}
}
- private void releaseLocks(Set acquired_locks, Object owner)
+ private void recordNodeLock(GlobalTransaction gtx, IdentityLock lock)
{
- if (acquired_locks != null && !acquired_locks.isEmpty())
- {
- Iterator iter = acquired_locks.iterator();
- while (iter.hasNext())
- {
- IdentityLock identityLock = (IdentityLock) iter.next();
- releaseLock(identityLock, owner);
- }
- }
- }
-
- private void releaseLock(IdentityLock lock, Object owner)
- {
- if (lock != null)
- {
- if (log.isDebugEnabled())
- {
- log.debug("Releasing lock [" + lock + "]");
- }
- lock.release(owner);
- }
- }
-
- private void recordNodeLock(GlobalTransaction gtx, IdentityLock lock, Object owner) throws LockingException
- {
if (gtx != null)
{
- // test that the gtx is associated with a valid tx.
- Transaction tx = tx_table.getLocalTransaction(gtx);
- if (!isValid(tx)) throw new LockingException("Locks can't be added to current transaction (transaction timed out?)");
// add the lock to the list of locks maintained for this transaction
// (needed for release of locks on commit or rollback)
- boolean reallyAdded = tx_table.addLock(gtx, lock);
- if (!reallyAdded)
- {
- releaseLock(lock, owner);
- throw new LockingException("Locks can't be added to current transaction (transaction timed out?)");
- }
+ cache.getTransactionTable().addLock(gtx, lock);
}
else
{
@@ -578,6 +402,7 @@
}
}
+
private List getLocks(Thread currentThread)
{
// This sort of looks like a get/put race condition, but
@@ -632,7 +457,7 @@
TransactionEntry entry = tx_table.get(gtx);
if (entry == null)
{
- if (log.isInfoEnabled()) log.info("entry for transaction " + gtx + " not found (transaction timed out?)");
+ log.error("entry for transaction " + gtx + " not found (maybe already committed)");
return;
}
@@ -666,7 +491,7 @@
if (entry == null)
{
- if (log.isInfoEnabled()) log.info("entry for transaction " + tx + " not found (transaction timed out?)");
+ log.error("entry for transaction " + tx + " not found (transaction has possibly already been rolled back)");
return;
}
@@ -683,4 +508,3 @@
}
}
-
Deleted: core/branches/1.4.X/tests/functional/org/jboss/cache/transaction/AsyncRollbackTxLockTest.java
===================================================================
--- core/branches/1.4.X/tests/functional/org/jboss/cache/transaction/AsyncRollbackTxLockTest.java 2007-09-26 17:33:33 UTC (rev 4508)
+++ core/branches/1.4.X/tests/functional/org/jboss/cache/transaction/AsyncRollbackTxLockTest.java 2007-09-26 18:41:27 UTC (rev 4509)
@@ -1,109 +0,0 @@
-package org.jboss.cache.transaction;
-
-import junit.framework.TestCase;
-import org.jboss.cache.TreeCache;
-import org.jboss.cache.lock.LockingException;
-
-import javax.transaction.Synchronization;
-import javax.transaction.SystemException;
-import javax.transaction.Transaction;
-import javax.transaction.TransactionManager;
-
-/**
- * Test behaviour of async rollback of a tx that times out and subsequent operations on that transaction.
- * <p/>
- * NOTE: To run this test, add the following lines:
- * <p/>
- * <tt>
- * <p/>
- * <p/>
- * if ("Thread-0".equals(Thread.currentThread().getName())) Thread.sleep(1500);
- * <p/>
- * </tt>
- * <p/>
- * to the first line of PessimistickLockInterceptor#recordNodeLock()
- *
- * @author <a href="mailto:jhalat at infovide.pl">Jacek Halat</a>
- */
-public class AsyncRollbackTxLockTest extends TestCase
-{
- private TreeCache cache;
- private TransactionManager tm;
-
- protected void setUp() throws Exception
- {
- cache = new TreeCache();
-
- cache.setTransactionManagerLookupClass("org.jboss.cache.transaction.AsyncRollbackTransactionManagerLookup");
- cache.startService();
- tm = cache.getTransactionManager();
- tm.setTransactionTimeout(2);
- }
-
- protected void tearDown()
- {
- try
- {
- if (tm != null && tm.getTransaction() != null)
- {
- try
- {
- tm.rollback();
- }
- catch (SystemException e)
- {
- // do nothing
- }
- }
- }
- catch (SystemException e)
- {
- // do nothing
- }
- if (cache != null) cache.stopService();
- cache = null;
- tm = null;
- }
-
-
- public void testTxTimeoutAndRemovePutAfter() throws Exception
- {
- Thread.currentThread().setName("Thread-0");
- System.out.println("Main Thread:" + Thread.currentThread());
- assertEquals(0, cache.getNumberOfLocksHeld());
- tm.setTransactionTimeout(1);//short transaction timeout
- cache.put("/a/b/c/d", "k", "v");
- tm.begin();
- Transaction transaction = tm.getTransaction();
- transaction.registerSynchronization(new Synchronization()
- {
-
- public void afterCompletion(int arg0)
- {
- System.out.println("Synchronization Thread:" + Thread.currentThread());
- }
-
- public void beforeCompletion()
- {
- }
-
- });
- assertNotNull(tm.getTransaction());
- Thread.sleep(500);//transaction should be rolledback in another thread
- try
- {
- cache.put("/a", "k", "v");
- }
- catch (LockingException expected)
- {
- // this is normal; the tx would have timed out and obtaining locks for this tx should fail.
- }
-
- tm.rollback();
- assertNull(tm.getTransaction());
- assertEquals(0, cache.getNumberOfLocksHeld());
- cache.put("/a", "k", "v");
- }
-
-}
-
Deleted: core/branches/1.4.X/tests/functional/org/jboss/cache/transaction/SimultaneousRollbackAndPutTest.java
===================================================================
--- core/branches/1.4.X/tests/functional/org/jboss/cache/transaction/SimultaneousRollbackAndPutTest.java 2007-09-26 17:33:33 UTC (rev 4508)
+++ core/branches/1.4.X/tests/functional/org/jboss/cache/transaction/SimultaneousRollbackAndPutTest.java 2007-09-26 18:41:27 UTC (rev 4509)
@@ -1,105 +0,0 @@
-package org.jboss.cache.transaction;
-
-import junit.framework.TestCase;
-import org.jboss.cache.DummyTransactionManagerLookup;
-import org.jboss.cache.Fqn;
-import org.jboss.cache.TreeCache;
-
-import javax.transaction.RollbackException;
-import javax.transaction.Transaction;
-import javax.transaction.TransactionManager;
-import java.util.ArrayList;
-import java.util.List;
-
-/**
- * To test JBCACHE-923
- *
- * @author <a href="mailto:manik at jboss.org">Manik Surtani</a>
- */
-public class SimultaneousRollbackAndPutTest extends TestCase
-{
- private TreeCache cache;
- private TransactionManager tm;
- private Fqn A = Fqn.fromString("/a"), B = Fqn.fromString("/b");
-
- protected void setUp() throws Exception
- {
- cache = new TreeCache();
- cache.setTransactionManagerLookupClass(AsyncRollbackTransactionManagerLookup.class.getName());
- cache.start();
- tm = cache.getTransactionManager();
- cache.put(A, "k", "v");
- }
-
- protected void tearDown()
- {
- cache.stop();
- }
-
- public void testStaleLocks() throws Exception
- {
- // repeat this test several times.
- for (int i = 0; i < 1000; i++)
- {
- System.out.println("In loop " + i);
- doTest();
- cache.remove(B);
- cache.removeData(A);
- cache.put(A, "k", "v");
- }
-
- }
-
-
- private void doTest() throws Exception
- {
- // scenario:
- // Thread starts tx in cache. E.g., create and put into B
- tm.begin();
- final Transaction t = tm.getTransaction();
- final List exceptions = new ArrayList();
-
- cache.put(B, "k", "v");
-
- // now the container should attempt to rollback the tx in a separate thread.
- Thread rollbackThread = new Thread()
- {
- public void run()
- {
- try
- {
- t.rollback();
- }
- catch (Exception e)
- {
- exceptions.add(e);
- }
- }
- };
- rollbackThread.start();
-
- try
- {
- // now try and put stuff in the main thread again
- cache.put(A, "k2", "v2");
- tm.commit();
- }
- catch (Exception expected)
- {
- // this is expected.
- }
-
- // make sure the rollback thread has completed
- rollbackThread.join();
-
- int nL = cache.getNumberOfLocksHeld();
- if (nL > 0)
- {
- System.out.println(cache.printLockInfo());
- fail("Should be no stale locks around!");
- }
- //assertEquals("No stale locks should be around", 0, cache.getNumberOfLocksHeld());
-
- if (exceptions.size() > 0) throw ((Exception) exceptions.get(0));
- }
-}
More information about the jbosscache-commits
mailing list