Merge lp:~pbeaman/akiban-persistit/fix-1043536-deadlock-under-clover into lp:akiban-persistit

[Peter Beaman]

Fix bug 1043536 (we hope). The bug appears to occur when an assert caused by a timeout occurs in JournalManagerTest, leaving the ThreadSequencer state in an unexpectedly enabled state. The fix ensures that ThreadSequencer is disabled after every test, and it lengthens the timeouts by a factor of 5.

The theory behind lengthening the timeouts is that execution of the instrumented Clover code is much slower and therefore threads that normally complete in much less than the sanity timeout are slow enough under Clover to exceed the timeout. There's no proof that 5x is sufficient or necessary, but there's also no proof that the original timeouts were reasonable; they are chosen as a sanity outer bound, and the objective of the timeout should be to eventually signal a failure-to-complete problem within a scale that's reasonable given that the test machines need to perform other tasks.

I was unable to reproduce this phenomenon and therefore have no unit test for it. The test will be whether persistit-coverage continues to fail in the same way.

[Nathan Williams (nwilliams)]

Looks reasonable to me.

review: Approve

=== modified file 'src/test/java/com/persistit/AccumulatorTest.java'
--- src/test/java/com/persistit/AccumulatorTest.java	2012-08-24 13:57:19 +0000
+++ src/test/java/com/persistit/AccumulatorTest.java	2012-08-30 16:32:18 +0000
@@ -479,7 +479,7 @@
 
     @Test
     public void testDeltasCombineMultiAccumMultiThread() throws Exception {
-        final long RUN_TIME_MAX = 10000;
+        final long RUN_TIME_MAX = 50000;
         final int THREAD_COUNT = 20;
         final int ACCUM_COUNT = 10;
         final int STEP_COUNT = 5;

=== modified file 'src/test/java/com/persistit/JournalManagerTest.java'
--- src/test/java/com/persistit/JournalManagerTest.java	2012-08-24 13:57:19 +0000
+++ src/test/java/com/persistit/JournalManagerTest.java	2012-08-30 16:32:18 +0000
@@ -677,7 +677,7 @@
          */
         enableSequencer(true);
         addSchedules(PAGE_MAP_READ_INVALIDATE_SCHEDULE);
-        startAndJoinAssertSuccess(5000, thread1, thread2);
+        startAndJoinAssertSuccess(25000, thread1, thread2);
         disableSequencer();
     }
 

=== modified file 'src/test/java/com/persistit/MVCCConcurrentTest.java'
--- src/test/java/com/persistit/MVCCConcurrentTest.java	2012-08-24 13:57:19 +0000
+++ src/test/java/com/persistit/MVCCConcurrentTest.java	2012-08-30 16:32:18 +0000
@@ -74,7 +74,7 @@
             }
         });
 
-        startAndJoinAssertSuccess(5000, readThread, writeThread, removeThread);
+        startAndJoinAssertSuccess(25000, readThread, writeThread, removeThread);
     }
 
     //

=== modified file 'src/test/java/com/persistit/PersistitUnitTestCase.java'
--- src/test/java/com/persistit/PersistitUnitTestCase.java	2012-08-15 16:12:34 +0000
+++ src/test/java/com/persistit/PersistitUnitTestCase.java	2012-08-30 16:32:18 +0000
@@ -24,6 +24,7 @@
 
 import com.persistit.exception.PersistitException;
 import com.persistit.unit.UnitTestProperties;
+import com.persistit.util.ThreadSequencer;
 
 public abstract class PersistitUnitTestCase {
 
@@ -58,6 +59,8 @@
 
     @After
     public void tearDown() throws Exception {
+        // Ensure sequencer is disabled after each test.
+        ThreadSequencer.disableSequencer();
         final WeakReference<Persistit> ref = new WeakReference<Persistit>(_persistit);
         _persistit.close(false);
         _persistit = null;