Merge lp:~pbeaman/akiban-persistit/fix-1076517-page-in-use2 into lp:akiban-persistit

[Peter Beaman]

Contains fixes for two observed deadlock scenarios described in https://bugs.launchpad.net/akiban-persistit/+bug/1076517.

1. Avoid calling deallocChain in the quick delete branch because a possible race which is otherwise harmless can lead to a deadlock.

2. In the event of a WWRetryException, the non-exclusive claim on the Tree is released as well as the claim on the buffer.

With these changes stress tests ran successfully 8 x 8 hours.

As mentioned in another merge proposal, the code to avoid a hypothetical livelock has been dropped for now until there is evidence of this actually occurring.

[Nathan Williams (nwilliams)]

Simple enough.

review: Approve

=== modified file 'src/main/java/com/persistit/Exchange.java'
--- src/main/java/com/persistit/Exchange.java	2012-11-28 20:04:20 +0000
+++ src/main/java/com/persistit/Exchange.java	2012-11-28 20:04:21 +0000
@@ -44,6 +44,7 @@
 import com.persistit.MVV.PrunedVersion;
 import com.persistit.ValueHelper.MVVValueWriter;
 import com.persistit.ValueHelper.RawValueWriter;
+import com.persistit.VolumeStructure.Chain;
 import com.persistit.exception.CorruptVolumeException;
 import com.persistit.exception.InUseException;
 import com.persistit.exception.PersistitException;
@@ -1662,10 +1663,14 @@
                         buffer.releaseTouched();
                         buffer = null;
                     }
+                    if (treeClaimAcquired) {
+                        _treeHolder.release();
+                        treeClaimAcquired = false;
+                    }
                     try {
                         sequence(WRITE_WRITE_STORE_A);
+                        final long depends = _persistit.getTransactionIndex().wwDependency(re.getVersionHandle(),
                         // TODO - timeout?
-                        final long depends = _persistit.getTransactionIndex().wwDependency(re.getVersionHandle(),
                                 _transaction.getTransactionStatus(), SharedResource.DEFAULT_MAX_WAIT_TIME);
                         if (depends != 0 && depends != TransactionStatus.ABORTED) {
                             // version is from concurrent txn that already
@@ -3260,6 +3265,7 @@
                     // First try for a quick delete from a single data page.
                     //
                     if (tryQuickDelete) {
+                        final List<Chain> chains = new ArrayList<Chain>();
                         Buffer buffer = null;
                         try {
                             final int foundAt1 = search(key1, true) & P_MASK;
@@ -3290,7 +3296,7 @@
 
                                         final long timestamp = timestamp();
                                         buffer.writePageOnCheckpoint(timestamp);
-                                        _volume.getStructure().harvestLongRecords(buffer, foundAt1, foundAt2);
+                                        _volume.getStructure().harvestLongRecords(buffer, foundAt1, foundAt2, chains);
 
                                         final boolean removed = buffer.removeKeys(foundAt1, foundAt2, _spareKey1);
                                         if (removed) {
@@ -3311,6 +3317,7 @@
                                 buffer = null;
                             }
                         }
+                        _volume.getStructure().deallocateGarbageChain(chains);
                     }
 
                     /*

=== modified file 'src/main/java/com/persistit/VolumeStructure.java'
--- src/main/java/com/persistit/VolumeStructure.java	2012-11-28 20:04:20 +0000
+++ src/main/java/com/persistit/VolumeStructure.java	2012-11-28 20:04:21 +0000
@@ -57,7 +57,7 @@
     private final Map<String, WeakReference<Tree>> _treeNameHashMap = new HashMap<String, WeakReference<Tree>>();
     private Tree _directoryTree;
 
-    private static class Chain {
+    static class Chain {
         final long _left;
         final long _right;
 
@@ -526,8 +526,7 @@
         deallocateGarbageChain(list);
     }
 
-    private void deallocateGarbageChain(final List<Chain> chains) throws PersistitException {
-
+    void deallocateGarbageChain(final List<Chain> chains) throws PersistitException {
         _volume.getStorage().claimHeadBuffer();
         try {
             while (!chains.isEmpty()) {
@@ -608,7 +607,7 @@
         deallocateGarbageChain(chains);
     }
 
-    private void harvestLongRecords(final Buffer buffer, final int start, final int end, final List<Chain> chains)
+    void harvestLongRecords(final Buffer buffer, final int start, final int end, final List<Chain> chains)
             throws PersistitException {
         assert buffer.isOwnedAsWriterByMe() : "Harvesting from page owned by another thread: " + buffer;
         if (buffer.isDataPage()) {

=== modified file 'src/test/java/com/persistit/stress/unit/AccumulatorRestart.java'
--- src/test/java/com/persistit/stress/unit/AccumulatorRestart.java	2012-11-27 14:42:49 +0000
+++ src/test/java/com/persistit/stress/unit/AccumulatorRestart.java	2012-11-28 20:04:21 +0000
@@ -106,16 +106,16 @@
                         seq.update(1, txn);
                         sum.update(r, txn);
                         seqValue++;
-                        long minWas = getLong(_ex.to("min"), Long.MAX_VALUE);
+                        final long minWas = getLong(_ex.to("min"), Long.MAX_VALUE);
                         _ex.getValue().put(Math.min(bsum(minValue, r), minWas));
                         _ex.store();
-                        long maxWas = getLong(_ex.to("max"), Long.MIN_VALUE);
+                        final long maxWas = getLong(_ex.to("max"), Long.MIN_VALUE);
                         _ex.getValue().put(Math.max(bsum(maxValue, r), maxWas));
                         _ex.store();
-                        long seqWas = getLong(_ex.to("seq"), 0);
+                        final long seqWas = getLong(_ex.to("seq"), 0);
                         _ex.getValue().put(Math.max(seqValue, seqWas));
                         _ex.store();
-                        long sumWas = getLong(_ex.to("sum"), 0);
+                        final long sumWas = getLong(_ex.to("sum"), 0);
                         _ex.getValue().put(Math.min(sumValue + r, sumWas));
                         _ex.store();
                         if (!a) {
@@ -150,7 +150,7 @@
         } catch (final Exception ex) {
             handleThrowable(ex);
         } finally {
-            Persistit db = getPersistit();
+            final Persistit db = getPersistit();
             try {
                 db.close();
             } catch (final PersistitException pe) {