Merge lp:~pbeaman/akiban-persistit/fix_912514_again into lp:akiban-persistit

[Peter Beaman]

Fix two small bugs detected while running the stress tests.

(a) start-stop-plan reuses a Persistit instance; releaseAllResources() now sets the initialized flag to false so that this will work correctly.

(b) fetchAndRemove is still broken - some bits need to be masked in two conditionals that test whether the from and to key of a remove operation are adjacent.

This branch fixes both bugs and adds trivial unit tests.

(Note - this is the second merge proposal for this branch. We pulled the first one because of uncertainty over some other changes.)

[Nathan Williams (nwilliams)]

Looks good.

review: Approve

[Akiban Build User (build-akiban)]

There was one failure during build/test:

* unknown exception (check log)

review: Needs Fixing

[Nathan Williams (nwilliams)]

Unrelated LBJ timeout waiting for dumbo.

=== modified file 'src/main/java/com/persistit/Exchange.java'
--- src/main/java/com/persistit/Exchange.java	2012-05-25 18:50:59 +0000
+++ src/main/java/com/persistit/Exchange.java	2012-05-29 21:03:19 +0000
@@ -3559,13 +3559,13 @@
 
     private void removeFetchFirst(Buffer buffer1, int foundAt1, Buffer buffer2, int foundAt2) throws PersistitException {
         if (buffer1 == buffer2) {
-            if (buffer1.nextKeyBlock(foundAt1) == foundAt2) {
+            if (buffer1.nextKeyBlock(foundAt1) == (foundAt2 & P_MASK)) {
                 buffer1.fetch(foundAt1 | EXACT_MASK, _spareValue);
             }
         } else {
             if (buffer1.getRightSibling() == buffer2.getPageAddress() && buffer1.nextKeyBlock(foundAt1) == -1) {
                 foundAt1 = buffer2.toKeyBlock(0);
-                if (buffer2.nextKeyBlock(foundAt1) == foundAt2) {
+                if (buffer2.nextKeyBlock(foundAt1) == (foundAt2 & P_MASK)) {
                     buffer2.fetch(foundAt1 | EXACT_MASK, _spareValue);
                 }
             }

=== modified file 'src/main/java/com/persistit/Persistit.java'
--- src/main/java/com/persistit/Persistit.java	2012-05-25 18:50:59 +0000
+++ src/main/java/com/persistit/Persistit.java	2012-05-29 21:03:19 +0000
@@ -1794,6 +1794,7 @@
         _intArrayThreadLocal.set(null);
         _keyThreadLocal.set(null);
         _valueThreadLocal.set(null);
+        _initialized.set(false);
     }
 
     /**

=== modified file 'src/test/java/com/persistit/ConfigurationTest.java'
--- src/test/java/com/persistit/ConfigurationTest.java	2012-05-25 18:50:59 +0000
+++ src/test/java/com/persistit/ConfigurationTest.java	2012-05-29 21:03:19 +0000
@@ -222,6 +222,14 @@
         assertTrue("Should contain expected text 'hwdemo'", configuration.getJournalPath().contains("hwdemo"));
     }
 
+    @Test
+    public void canReinitialize() throws Exception {
+        _persistit.close();
+        assertTrue("Should have closed all volumes", _persistit.getVolumes().isEmpty());
+        _persistit.initialize(_persistit.getConfiguration());
+        assertTrue("Should have reopened volumes", !_persistit.getVolumes().isEmpty());
+    }
+
     private Configuration testLoadPropertiesBufferSpecificationsHelper(final Properties properties) throws Exception {
         Configuration configuration = new Configuration();
         configuration.merge(properties);

=== modified file 'src/test/java/com/persistit/unit/ExchangeTest.java'
--- src/test/java/com/persistit/unit/ExchangeTest.java	2012-05-25 18:50:59 +0000
+++ src/test/java/com/persistit/unit/ExchangeTest.java	2012-05-29 21:03:19 +0000
@@ -21,6 +21,7 @@
 package com.persistit.unit;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import java.util.Random;
@@ -30,6 +31,7 @@
 import com.persistit.Exchange;
 import com.persistit.Key;
 import com.persistit.KeyFilter;
+import com.persistit.Transaction;
 import com.persistit.Volume;
 import com.persistit.exception.ConversionException;
 import com.persistit.exception.PersistitException;
@@ -281,6 +283,40 @@
         assertEquals(1, ex.getValue().getInt());
     }
 
+    @Test
+    public void testRemoveAndFetch() throws Exception {
+        testRemoveAndFetch(false);
+        testRemoveAndFetch(true);
+    }
+
+    private void testRemoveAndFetch(boolean inTransaction) throws Exception {
+        Exchange ex = _persistit.getExchange("persistit", "gogo", true);
+        Transaction txn = ex.getTransaction();
+        ex.getValue().put(RED_FOX);
+        for (int i = 1; i < 10000; i++) {
+            if (inTransaction) {
+                txn.begin();
+            }
+            ex.to(i).store();
+            if (inTransaction) {
+                txn.commit();
+                txn.end();
+            }
+        }
+        for (int i = 1; i < 10000; i++) {
+            if (inTransaction) {
+                txn.begin();
+            }
+            ex.getValue().clear();
+            ex.to(i).fetchAndRemove();
+            assertTrue("A value was fetched", ex.getValue().isDefined());
+            if (inTransaction) {
+                txn.commit();
+                txn.end();
+            }
+        }
+    }
+
     /*
      * This function is "borrowed" from the YCSB benchmark framework developed
      * by Yahoo Research.