Merge lp:~pbeaman/akiban-persistit/test-plan-review-1 into lp:akiban-persistit

[Peter Beaman]

Several very small corrections/edits based on code-review:

Typos in the release notes.

Buffer#dump should use same IV format as journal.
synchronize around various clear() methods in Persistit#releaseAllResources to ensure visibility in other threads (eliminate a possible cause for occasional failure of unit tests to release all resources.

Remove a dead method.

Retry loop in Exchange now actually decrements the retry counter.

Remove return unnecessary return value from addOrCombineDelta.

[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)]

LBJ timeout.

[Akiban Build User (build-akiban)]

There were 2 failures during build/test:

* job persistit-build failed at build number 467: http://172.16.20.104:8080/job/persistit-build/467/

* view must-pass failed: persistit-build is yellow

review: Needs Fixing

[Peter Beaman (pbeaman)]

Very interesting. The failed test appears to have created a deadlock, which may be an instance of elusive bug I am looking for. I do not believe the changes in this proposal have anything to do with it, so I'm re-A-pproving this again, but will also try to reproduce the deadlock.

=== modified file 'doc/ReleaseNotes.rst'
--- doc/ReleaseNotes.rst	2012-10-15 21:22:32 +0000
+++ doc/ReleaseNotes.rst	2012-10-26 19:53:26 +0000
@@ -80,7 +80,7 @@
 Persistit 3.2.0 - Default Journal File Format Changed
 -----------------------------------------------------
 
-Version 3.2.0 fixes problems related to Volumes created and opened by the com.persistit#loadVolume 
+Version 3.2.0 fixes problems related to Volumes created and opened by the com.persistit.Persistit#loadVolume 
 method rather than being specified by the initial system configuration. In previous versions, journal files 
 contained insufficient information to properly recover such volumes, even during normal startup.
 
@@ -150,7 +150,7 @@
 volume from an existing Database, this change provides a configurable switch to optionally 
 allow pages from missing volumes to be skipped (with logged warning messages) during recovery 
 processing.  The switch can be enabled by setting the configuration parameter 
-com.persistit.Configuration#setIgnoreMissingViolumes to true.
+com.persistit.Configuration#setIgnoreMissingVolumes to true.
 
 
 Persistit 3.1.4 - Reduce KeyCoder Serialized Object Size

=== modified file 'src/main/java/com/persistit/Buffer.java'
--- src/main/java/com/persistit/Buffer.java	2012-10-03 14:43:25 +0000
+++ src/main/java/com/persistit/Buffer.java	2012-10-26 19:53:26 +0000
@@ -4268,7 +4268,11 @@
             IV.putHandle(bb, volumeHandle);
             IV.putVolumeId(bb, volume.getId());
             IV.putTimestamp(bb, 0);
-            IV.putVolumeSpecification(bb, volume.getName());
+            if (_persistit.getConfiguration().isUseOldVSpec()) {
+                IV.putVolumeSpecification(bb, volume.getName());
+            } else {
+                IV.putVolumeSpecification(bb, volume.getSpecification().toString());
+            }
             bb.position(bb.position() + IV.getLength(bb));
             identifiedVolumes.add(volume);
         }

=== modified file 'src/main/java/com/persistit/Exchange.java'
--- src/main/java/com/persistit/Exchange.java	2012-10-15 19:21:18 +0000
+++ src/main/java/com/persistit/Exchange.java	2012-10-26 19:53:26 +0000
@@ -585,14 +585,6 @@
             }
         }
 
-        /**
-         * Re-save the current buffer generation. Can only be used when all
-         * other data (key gens, foundAt, etc) is still valid.
-         */
-        private void updateBufferGeneration() {
-            _bufferGeneration = _buffer.getGeneration();
-        }
-
         private Sequence sequence(final int foundAt) {
             final int delta = ((foundAt & P_MASK) - (_lastInsertAt & P_MASK));
             if ((foundAt & EXACT_MASK) == 0 && delta == KEYBLOCK_LENGTH) {
@@ -1160,8 +1152,7 @@
                     pageAddress = buffer.getPointer(p);
 
                     Debug.$assert0.t(pageAddress > 0 && pageAddress < MAX_VALID_PAGE_ADDR);
-                }/** TODO -- dead code **/
-                else {
+                } else {
                     oldBuffer = buffer; // So it will be released
                     corrupt("Volume " + _volume + " level=" + currentLevel + " page=" + pageAddress + " key=<"
                             + key.toString() + ">" + " page type=" + buffer.getPageType() + " is invalid");
@@ -1346,7 +1337,7 @@
 
         final int maxSimpleValueSize = maxValueSize(key.getEncodedSize());
         final Value spareValue = _persistit.getThreadLocalValue();
-        assert !(doMVCC & value == spareValue || doFetch && value == _spareValue) : "storeInternal may be use the supplied Value: "
+        assert !(doMVCC & value == spareValue || doFetch && value == _spareValue) : "storeInternal may use the supplied Value: "
                 + value;
 
         //
@@ -1492,7 +1483,7 @@
                         if (doMVCC) {
                             valueToStore = spareValue;
                             final int valueSize = value.getEncodedSize();
-                            final int retries = VERSIONS_OUT_OF_ORDER_RETRY_COUNT;
+                            int retries = VERSIONS_OUT_OF_ORDER_RETRY_COUNT;
 
                             for (;;) {
                                 try {
@@ -1558,7 +1549,7 @@
                                     }
                                     break;
                                 } catch (final VersionsOutOfOrderException e) {
-                                    if (retries <= 0) {
+                                    if (--retries <= 0) {
                                         throw e;
                                     }
                                 }

=== modified file 'src/main/java/com/persistit/JournalManager.java'
--- src/main/java/com/persistit/JournalManager.java	2012-10-08 19:48:22 +0000
+++ src/main/java/com/persistit/JournalManager.java	2012-10-26 19:53:26 +0000
@@ -2221,10 +2221,6 @@
 
             return super.getPollInterval() / divisor;
         }
-
-        void setShouldStop(final boolean shouldStop) {
-            _shouldStop = shouldStop;
-        }
     }
 
     private class JournalFlusher extends IOTaskRunnable {

=== modified file 'src/main/java/com/persistit/Persistit.java'
--- src/main/java/com/persistit/Persistit.java	2012-10-10 16:06:49 +0000
+++ src/main/java/com/persistit/Persistit.java	2012-10-26 19:53:26 +0000
@@ -1837,20 +1837,35 @@
             _logFlusher.interrupt();
         }
         _logFlusher = null;
-        _accumulators.clear();
-        _volumes.clear();
-        _exchangePoolMap.clear();
-        _cleanupManager.clear();
-        _transactionSessionMap.clear();
-        _cliSessionMap.clear();
-        _sessionIdThreadLocal.remove();
-        _fatalErrors.clear();
-        _alertMonitors.clear();
-        _bufferPoolTable.clear();
-        _intArrayThreadLocal.set(null);
-        _keyThreadLocal.set(null);
-        _valueThreadLocal.set(null);
-        _initialized.set(false);
+        /*
+         * The following are synchronized to ensure visibility
+         */
+        synchronized (_accumulators) {
+            _accumulators.clear();
+        }
+        synchronized (this) {
+            _volumes.clear();
+            _alertMonitors.clear();
+            _bufferPoolTable.clear();
+            _intArrayThreadLocal.set(null);
+            _keyThreadLocal.set(null);
+            _valueThreadLocal.set(null);
+            _initialized.set(false);
+            _sessionIdThreadLocal.remove();
+            _cleanupManager.clear();
+        }
+        synchronized (_exchangePoolMap) {
+            _exchangePoolMap.clear();
+        }
+        synchronized (_transactionSessionMap) {
+            _transactionSessionMap.clear();
+        }
+        synchronized (_cliSessionMap) {
+            _cliSessionMap.clear();
+        }
+        synchronized (_fatalErrors) {
+            _fatalErrors.clear();
+        }
     }
 
     /**

=== modified file 'src/main/java/com/persistit/TransactionIndex.java'
--- src/main/java/com/persistit/TransactionIndex.java	2012-09-12 20:07:07 +0000
+++ src/main/java/com/persistit/TransactionIndex.java	2012-10-26 19:53:26 +0000
@@ -1142,17 +1142,15 @@
      *            Step value of modification.
      * @param value
      *            The value to add or combine.
-     * 
-     * @return Delta that was created or modified.
      */
-    Delta addOrCombineDelta(final TransactionStatus status, final Accumulator accumulator, final int step,
+    void addOrCombineDelta(final TransactionStatus status, final Accumulator accumulator, final int step,
             final long value) {
         // Check current deltas, no lock as status is single txn/thread
         Delta delta = status.getDelta();
         while (delta != null) {
             if (delta.canMerge(accumulator, step)) {
                 delta.merge(value);
-                return null;
+                return;
             }
             delta = delta.getNext();
         }
@@ -1161,7 +1159,6 @@
         delta.setAccumulator(accumulator);
         delta.setStep(step);
         delta.setValue(value);
-        return delta;
     }
 
     /*