[jboss-svn-commits] JBL Code SVN: r29063 - in labs/jbosstm/trunk/ArjunaCore/arjuna: tests/byteman-scripts and 1 other directories.
jboss-svn-commits at lists.jboss.org
jboss-svn-commits at lists.jboss.org
Wed Aug 26 08:42:22 EDT 2009
Author: adinn
Date: 2009-08-26 08:42:22 -0400 (Wed, 26 Aug 2009)
New Revision: 29063
Modified:
labs/jbosstm/trunk/ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/coordinator/TransactionReaper.java
labs/jbosstm/trunk/ArjunaCore/arjuna/tests/byteman-scripts/reaper.txt
labs/jbosstm/trunk/ArjunaCore/arjuna/tests/classes/com/hp/mwtests/ts/arjuna/reaper/ReaperTestCase2.java
Log:
Fixes for JBTM-611
reaper check() method now checks for and pulls in pending TXs each
time round the main loop in order to ensure it does not process any
entries out of order. it also locks the queue when it dequeues the
first entry, ensuring that it shares a consistent view of what is
first with any inserting threads
reaper now includes pending TXs when computing transactions count and
timeouts count.
reaper inserts now check the timeouts and pending timeouts maps before
adding an element to detect duplicate adds. This is a guard against
badly implemented hash and equals methods since our code will never
add the same instance twice. Note that a deferred add (to the pending
queue) may succeed but later fail when the TX is added to the sorted
list. This is a guard against badly implemented compareTo
methods. Both of these could be dropped if the performance hit is too
large. We don't really need to care about users who break the reaper
by failing to implement equality or trichotomy.
reaper test script changed to relocate the initial rendezvous point at
the top of the check loop.
ReaperTestCase2 made cosmetic changes so that indicies in symbolic
names for uids in text match indices for actual uids allocated in the
test.
Modified: labs/jbosstm/trunk/ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/coordinator/TransactionReaper.java
===================================================================
--- labs/jbosstm/trunk/ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/coordinator/TransactionReaper.java 2009-08-26 12:31:13 UTC (rev 29062)
+++ labs/jbosstm/trunk/ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/coordinator/TransactionReaper.java 2009-08-26 12:42:22 UTC (rev 29063)
@@ -203,6 +203,9 @@
FacilityCode.FAC_ATOMIC_ACTION, "TransactionReaper::check ()");
}
+ do
+ {
+ final ReaperElement e ;
synchronized (this)
{
@@ -221,42 +224,38 @@
}
}
}
- }
+ try
+ {
+ e = (ReaperElement)_transactions.first();
+ }
+ catch (final NoSuchElementException nsee)
+ {
+ return true ;
+ }
+ if (tsLogger.arjLoggerI18N.isDebugEnabled())
+ {
+ tsLogger.arjLoggerI18N
+ .debug(
+ DebugLevel.FUNCTIONS,
+ VisibilityLevel.VIS_PUBLIC,
+ FacilityCode.FAC_ATOMIC_ACTION,
+ "com.arjuna.ats.arjuna.coordinator.TransactionReaper_2",
+ new Object[]
+ { Long.toString(e.getAbsoluteTimeout()) });
+ }
- do
- {
- final ReaperElement e ;
- try
- {
- e = (ReaperElement)_transactions.first();
- }
- catch (final NoSuchElementException nsee)
- {
- return true ;
- }
+ final long now = System.currentTimeMillis();
- if (tsLogger.arjLoggerI18N.isDebugEnabled())
- {
- tsLogger.arjLoggerI18N
- .debug(
- DebugLevel.FUNCTIONS,
- VisibilityLevel.VIS_PUBLIC,
- FacilityCode.FAC_ATOMIC_ACTION,
- "com.arjuna.ats.arjuna.coordinator.TransactionReaper_2",
- new Object[]
- { Long.toString(e.getAbsoluteTimeout()) });
- }
+ if (now < e.getAbsoluteTimeout())
+ {
+ // go back to sleep
- final long now = System.currentTimeMillis();
+ break;
+ }
- if (now < e.getAbsoluteTimeout())
- {
- // go back to sleep
+ }
- break;
- }
-
if (tsLogger.arjLoggerI18N.isWarnEnabled())
{
tsLogger.arjLoggerI18N
@@ -757,7 +756,7 @@
public final long numberOfTransactions()
{
- return _transactions.size();
+ return _transactions.size() + _pendingInsertions.size();
}
/**
@@ -766,7 +765,7 @@
*/
public final long numberOfTimeouts()
{
- return _timeouts.size();
+ return _timeouts.size() + _pendingInsertions.size();
}
public final void addListener (ReaperMonitor listener)
@@ -814,8 +813,23 @@
// if the new element would timeout after the earliest one we already have,
// we can delay its insertion until that earlier timeout. Hopefully the new tx will
// complete before then and we'll never have to insert it at all.
- if(first != null && e.compareTo(first) > 0) {
- _pendingInsertions.put(control, e);
+ if (first != null && e.compareTo(first) > 0) {
+ // first make sure we have not seen this control already
+ if (_timeouts.containsKey(control)) {
+ // hmm, this probably means that the hash or equals implementation on the element has been
+ // coded wrong
+ return false;
+ }
+ // put it in in the pending list for later insertion but also
+ // check the return value in case this entry already exists
+ ReaperElement old = _pendingInsertions.put(control, e);
+ if (old != null) {
+ // hmm, this probably means that the hash or equals implementation on the element has been
+ // coded wrong -- restore the old entry and return false. n.b. checking for a duplicate
+ // this way avoids having to do a containsKey test in the normal case.
+ _pendingInsertions.put(control, old);
+ return false;
+ }
asyncInsert = true;
}
}
@@ -831,20 +845,21 @@
private final boolean synchronousInsert(ReaperElement elementToInsert)
{
-
- /**
- * Ignore if it's already in the list with a different timeout.
- * (This should never happen)
- */
- if (_timeouts.containsKey(elementToInsert._control)) {
- return false; // TODO remove this, rewrite put instead.
- }
-
synchronized (this)
{
TransactionReaper._lifetime += elementToInsert._timeout;
- _timeouts.put(elementToInsert._control, elementToInsert);
+ ReaperElement old = (ReaperElement)_timeouts.put(elementToInsert._control, elementToInsert);
+ if (old != null) {
+ // hmm, this probably means that the hash or equals implementation on the element has been
+ // coded wrong -- restore the old entry and return false. n.b. checking for a duplicate
+ // this way avoids having to do a containsKey test in the normal case.
+ _timeouts.put(elementToInsert._control, old);
+ return false;
+ }
+
+ // we should not get an error here unless the user has coded the compareTo test wrong
+
boolean rtn = _transactions.add(elementToInsert);
if(_dynamic && _transactions.first() == elementToInsert)
Modified: labs/jbosstm/trunk/ArjunaCore/arjuna/tests/byteman-scripts/reaper.txt
===================================================================
--- labs/jbosstm/trunk/ArjunaCore/arjuna/tests/byteman-scripts/reaper.txt 2009-08-26 12:31:13 UTC (rev 29062)
+++ labs/jbosstm/trunk/ArjunaCore/arjuna/tests/byteman-scripts/reaper.txt 2009-08-26 12:42:22 UTC (rev 29063)
@@ -18,7 +18,7 @@
RULE pause transaction reaper 1
CLASS com.arjuna.ats.arjuna.coordinator.TransactionReaper
METHOD check
-AT CALL first()
+AT SYNCHRONIZE
BIND NOTHING
IF isRendezvous("reaper1", 2)
DO debug("reaper1"),
Modified: labs/jbosstm/trunk/ArjunaCore/arjuna/tests/classes/com/hp/mwtests/ts/arjuna/reaper/ReaperTestCase2.java
===================================================================
--- labs/jbosstm/trunk/ArjunaCore/arjuna/tests/classes/com/hp/mwtests/ts/arjuna/reaper/ReaperTestCase2.java 2009-08-26 12:31:13 UTC (rev 29062)
+++ labs/jbosstm/trunk/ArjunaCore/arjuna/tests/classes/com/hp/mwtests/ts/arjuna/reaper/ReaperTestCase2.java 2009-08-26 12:42:22 UTC (rev 29063)
@@ -24,6 +24,7 @@
import com.arjuna.ats.arjuna.coordinator.Reapable;
import com.arjuna.ats.arjuna.coordinator.ActionStatus;
import com.arjuna.ats.arjuna.common.Uid;
+import com.arjuna.ats.arjuna.logging.tsLogger;
import org.junit.Test;
import static org.junit.Assert.*;
@@ -50,24 +51,24 @@
// the rendezvous for the reapables are keyed by the reapable's uid
+ Uid uid0 = new Uid();
Uid uid1 = new Uid();
Uid uid2 = new Uid();
Uid uid3 = new Uid();
- Uid uid4 = new Uid();
- // reapable1 will return CANCELLED from cancel and will rendezvous inside the cancel call
+ // reapable0 will return CANCELLED from cancel and will rendezvous inside the cancel call
// so we can delay it. prevent_commit should not get called so we don't care about the arguments
- TestReapable reapable1 = new TestReapable(uid1, true, true, false, false);
- // reapable2 will return CANCELLED from cancel and will not rendezvous inside the cancel call
+ TestReapable reapable0 = new TestReapable(uid0, true, true, false, false);
+ // reapable1 will return CANCELLED from cancel and will not rendezvous inside the cancel call
// prevent_commit should not get called so we don't care about the arguments
- TestReapable reapable2 = new TestReapable(uid2, true, false, false, false);
- // reapable3 will return RUNNING from cancel and will rendezvous inside the cancel call
+ TestReapable reapable1 = new TestReapable(uid1, true, false, false, false);
+ // reapable2 will return RUNNING from cancel and will rendezvous inside the cancel call
// the call will get delayed causing the worker to exit as a zombie
// prevent_commit will be called from the reaper thread and will fail but will not rendezvous
- TestReapable reapable3 = new TestReapable(uid3, false, true, false, false);
- // reapable4 will return RUNNING from cancel and will not rendezvous inside the cancel call
+ TestReapable reapable2 = new TestReapable(uid2, false, true, false, false);
+ // reapable3 will return RUNNING from cancel and will not rendezvous inside the cancel call
// prevent_commit should get called and should return true without a rendezvous
- TestReapable reapable4 = new TestReapable(uid4, false, false, true, false);
+ TestReapable reapable3 = new TestReapable(uid3, false, false, true, false);
// enable a repeatable rendezvous before checking the reapable queue
enableRendezvous("reaper1", true);
@@ -95,29 +96,29 @@
// enable a repeatable rendezvous for each of the test reapables which we have marked to
// perform a rendezvous
- enableRendezvous(uid1, true);
- enableRendezvous(uid3, true);
+ enableRendezvous(uid0, true);
+ enableRendezvous(uid2, true);
// STAGE I
// insert two reapables so they timeout at 1 second intervals then stall the first one and
// check progress of cancellations and rollbacks for both
+ assertTrue(reaper.insert(reapable0, 1));
+
assertTrue(reaper.insert(reapable1, 1));
- assertTrue(reaper.insert(reapable2, 1));
+ //assertTrue(reaper.insert(reapable2, 1));
//assertTrue(reaper.insert(reapable3, 1));
- //assertTrue(reaper.insert(reapable4, 1));
-
// latch the reaper before it tries to process the queue
triggerRendezvous("reaper1");
// make sure they were all registered
// the transactions queue should be
+ // UID0 RUNNING
// UID1 RUNNING
- // UID2 RUNNING
assertEquals(2, reaper.numberOfTransactions());
assertEquals(2, reaper.numberOfTimeouts());
@@ -140,8 +141,8 @@
triggerRendezvous("reaperworker1");
// the transactions queue should be
- // UID2 RUNNING
- // UID1 CANCEL
+ // UID1 RUNNING
+ // UID0 CANCEL
assertEquals(2, reaper.numberOfTransactions());
assertEquals(2, reaper.numberOfTimeouts());
@@ -152,7 +153,7 @@
// latch the first reapable inside cancel
- triggerRendezvous(uid1);
+ triggerRendezvous(uid0);
// now let the reaper check the queue for the second reapable, dequeue it and add it to the
// worker queue
@@ -164,8 +165,8 @@
triggerRendezvous("reaper1");
// the transactions queue should be
- // UID1 CANCEL
- // UID2 SCHEDULE_CANCEL
+ // UID0 CANCEL
+ // UID1 SCHEDULE_CANCEL
assertEquals(2, reaper.numberOfTransactions());
assertEquals(2, reaper.numberOfTimeouts());
@@ -184,14 +185,14 @@
// unlatch the first reapable inside cancel
- triggerRendezvous(uid1);
+ triggerRendezvous(uid0);
// latch the worker as it is about to process the queue again
triggerRendezvous("reaperworker1");
// the transactions queue should be
- // UID2 SCHEDULE_CANCEL
+ // UID1 SCHEDULE_CANCEL
assertEquals(1, reaper.numberOfTransactions());
assertEquals(1, reaper.numberOfTimeouts());
@@ -209,12 +210,12 @@
assertEquals(0, reaper.numberOfTransactions());
assertEquals(0, reaper.numberOfTimeouts());
- // ensure that cancel was tried on reapable2 and that set rollback only was not tried on either
- // we know cancel was tried on reapable1 because we got through the rendezvous
+ // ensure that cancel was tried on reapable1 and that set rollback only was not tried on either
+ // we know cancel was tried on reapable0 because we got through the rendezvous
- assertTrue(reapable2.getCancelTried());
+ assertTrue(reapable1.getCancelTried());
+ assertTrue(!reapable0.getRollbackTried());
assertTrue(!reapable1.getRollbackTried());
- assertTrue(!reapable2.getRollbackTried());
assertTrue(checkAndClearFlag("interrupted"));
// STAGE II
@@ -223,14 +224,14 @@
// insert reapables so they timeout at 1 second intervals then
// check progress of cancellations and rollbacks
+ assertTrue(reaper.insert(reapable2, 1));
+
assertTrue(reaper.insert(reapable3, 1));
- assertTrue(reaper.insert(reapable4, 1));
-
// make sure they were all registered
// the transactions queue should be
+ // UID2 RUNNING
// UID3 RUNNING
- // UID4 RUNNING
assertEquals(2, reaper.numberOfTransactions());
assertEquals(2, reaper.numberOfTimeouts());
@@ -249,8 +250,8 @@
triggerRendezvous("reaper1");
// the transactions queue should be
- // UID4 RUNNING
- // UID3 CANCEL
+ // UID3 RUNNING
+ // UID2 CANCEL
assertEquals(2, reaper.numberOfTransactions());
assertEquals(2, reaper.numberOfTimeouts());
@@ -261,7 +262,7 @@
// latch the third reapable inside cancel
- triggerRendezvous(uid3);
+ triggerRendezvous(uid2);
// now let the reaper check the queue for the fourth reapable, dequeue it and add it to the
// worker queue
@@ -273,8 +274,8 @@
triggerRendezvous("reaper1");
// the transactions queue should be
- // UID3 CANCEL
- // UID4 SCHEDULE_CANCEL
+ // UID2 CANCEL
+ // UID3 SCHEDULE_CANCEL
assertEquals(2, reaper.numberOfTransactions());
assertEquals(2, reaper.numberOfTimeouts());
@@ -298,8 +299,8 @@
triggerWait(500);
// the transactions queue should be
- // UID4 SCHEDULE_CANCEL
- // UID3 CANCEL_INTERRUPTED
+ // UID3 SCHEDULE_CANCEL
+ // UID2 CANCEL_INTERRUPTED
assertEquals(2, reaper.numberOfTransactions());
assertEquals(2, reaper.numberOfTimeouts());
@@ -313,8 +314,8 @@
triggerRendezvous("reaper1");
// the transactions queue should be
- // UID3 CANCEL_INTERRUPTED
- // UID4 SCHEDULE_CANCEL
+ // UID2 CANCEL_INTERRUPTED
+ // UID3 SCHEDULE_CANCEL
assertEquals(2, reaper.numberOfTransactions());
assertEquals(2, reaper.numberOfTimeouts());
@@ -332,14 +333,14 @@
assertTrue(checkAndClearFlag("zombied"));
// the transactions queue should be
- // UID4 SCHEDULE_CANCEL
+ // UID3 SCHEDULE_CANCEL
assertEquals(1, reaper.numberOfTransactions());
assertEquals(1, reaper.numberOfTimeouts());
// unlatch the third reapable inside cancel
- triggerRendezvous(uid3);
+ triggerRendezvous(uid2);
// latch the new worker as it is about to process the queue again
@@ -358,12 +359,12 @@
assertEquals(0, reaper.numberOfTransactions());
assertEquals(0, reaper.numberOfTimeouts());
- // ensure that cancel was tried on reapable4 and that set rollback only was tried on reapable3
- // and reapable4 we know cancel was tried on reapable3 because we got through the rendezvous
+ // ensure that cancel was tried on reapable3 and that set rollback only was tried on reapable2
+ // and reapable3 we know cancel was tried on reapable2 because we got through the rendezvous
- assertTrue(reapable4.getCancelTried());
+ assertTrue(reapable3.getCancelTried());
+ assertTrue(reapable2.getRollbackTried());
assertTrue(reapable3.getRollbackTried());
- assertTrue(reapable4.getRollbackTried());
}
private void enableRendezvous(Object o, boolean repeatable)
More information about the jboss-svn-commits
mailing list