Merge lp:~pbeaman/akiban-persistit/fix-JournalManagerTest into lp:akiban-persistit

[Peter Beaman]

Fixes a subtle timing-related failure in JournalManagerTest#testJournalRecords. At one point this test needs to clear the handle for a Tree instance; it does so too early in the code, however, since there is a later call to Persistit#getExchange that can repopulate the handle. If the later call does reinstantiate the tree handle, then subsequent operations fail and an assertion fires to fail the test.

Apparently this problem has been present but undetected for many months.

The problem is timing-related because the getExchange method attempts to look up a cached copy of the Tree from a WeakHashMap. If that lookup is successful then the test succeeds; if it fails then a new Tree object with a freshly instantiated tree handle is created. It appears that GC is more aggressive on the Mac version of the JDK which is where we started to see the failures. This interaction is created by the very special internal twiddling done by this test and is not a general problem.

The modified version of the test passes on Mac and Cent OS.

Since this is a bug in a test, we have no unit test; the fact that the original test passes is confirmation of the fix.

[Nathan Williams (nwilliams)]

Makes sense.

review: Approve

=== modified file 'src/test/java/com/persistit/JournalManagerTest.java'
--- src/test/java/com/persistit/JournalManagerTest.java	2012-07-25 19:32:11 +0000
+++ src/test/java/com/persistit/JournalManagerTest.java	2012-08-01 23:18:19 +0000
@@ -72,27 +72,27 @@
 
         final Volume volume = _persistit.getVolume(_volumeName);
         volume.resetHandle();
+
+        final JournalManager jman = new JournalManager(_persistit);
+        final String path = UnitTestProperties.DATA_PATH + "/JournalManagerTest_journal_";
+        jman.init(null, path, 100 * 1000 * 1000);
+        final BufferPool pool = _persistit.getBufferPool(16384);
+        final long pages = Math.min(1000, volume.getStorage().getNextAvailablePage() - 1);
+        for (int i = 0; i < 1000; i++) {
+            final Buffer buffer = pool.get(volume, i % pages, true, true);
+            if ((i % 400) == 0) {
+                jman.rollover();
+            }
+            buffer.setDirtyAtTimestamp(_persistit.getTimestampAllocator().updateTimestamp());
+            jman.writePageToJournal(buffer);
+            buffer.clearDirty();
+            buffer.releaseTouched();
+        }
+
+        final Checkpoint checkpoint1 = _persistit.getCheckpointManager().createCheckpoint();
+        jman.writeCheckpointToJournal(checkpoint1);
+        final Exchange exchange = _persistit.getExchange(_volumeName, "JournalManagerTest1", false);
         volume.getTree("JournalManagerTest1", false).resetHandle();
-
-        final JournalManager jman = new JournalManager(_persistit);
-        final String path = UnitTestProperties.DATA_PATH + "/JournalManagerTest_journal_";
-        jman.init(null, path, 100 * 1000 * 1000);
-        final BufferPool pool = _persistit.getBufferPool(16384);
-        final long pages = Math.min(1000, volume.getStorage().getNextAvailablePage() - 1);
-        for (int i = 0; i < 1000; i++) {
-            final Buffer buffer = pool.get(volume, i % pages, true, true);
-            if ((i % 400) == 0) {
-                jman.rollover();
-            }
-            buffer.setDirtyAtTimestamp(_persistit.getTimestampAllocator().updateTimestamp());
-            jman.writePageToJournal(buffer);
-            buffer.clearDirty();
-            buffer.releaseTouched();
-        }
-
-        final Checkpoint checkpoint1 = _persistit.getCheckpointManager().createCheckpoint();
-        jman.writeCheckpointToJournal(checkpoint1);
-        final Exchange exchange = _persistit.getExchange(_volumeName, "JournalManagerTest1", false);
         assertTrue(exchange.next(true));
         final long[] timestamps = new long[100];