Merge lp:~pbeaman/akiban-persistit/fix-several-small-bugs into lp:akiban-persistit

[Peter Beaman]

Fixes several small unrelated bugs:

https://bugs.launchpad.net/akiban-persistit/+bug/1023549 - as written, is invalid, and this proposal includes a new unit test to prove it.

https://bugs.launchpad.net/akiban-persistit/+bug/1013259 - cleans up Persistit shut-down handling. Changes ensure that concurrent transactions are interrupted and closed properly. The IllegalMonitorStateException occurred because two threads were racing to close a transaction on shutdown; this condition is now eliminated. A new unit test was added to TransactionTest2 to verify correct shutdown behavior.

https://bugs.launchpad.net/akiban-persistit/+bug/1029942 - incorrect constant was used in the log statement. Constants NS_PER_S, NS_PER_MS and MS_PER_S were consolidated into the com.persistit.util.Util class.

Bug #1062315: Assertion failure in TransactionIndexBucket#allocateTransactionStatus - a small change to avoid asserting in the case of a benign race condition was added.

[Nathan Williams (nwilliams)]

VolumeStorageV2#updateMetaData() - including timestamp changed in result will make meta data always "dirty" and then written. Is that what we want?

ExchangeTest#traverseEQfalse0 - The bug has has 'false' in the title and 'true' in the description. Could we add cases for 'true' as well? Additionally, the server use case probably did it inside of a transaction. It should work as is if another test method just calls the new one, but inside of a transaction block. I have no reason to suspect a failure, but simple to add.

Persistit#close() - _journalManager.stopCopier() is no longer called. Should it be? The rest of the related changes look plausible and the new test is good.

review: Needs Information

[Peter Beaman (pbeaman)]

VolumeStorageV2#updateMetaData - good catch - forgot to change it back after an experiment.

ExchangeTest#traverseEQfalse0 - good catch on not testing the transaction case. Changed the names, added a transactional version, and also tests for both true and false values of the 'deep' parameter.

Persistit#close() no longer calls stopCopier - yes, there is a change. The JOURNAL_COPIER thread used to be responsible for various cleanup activities and therefore had to be stopped early. That is no longer the case, so JournalManager#close() is where the copier now gets stopped.

[Nathan Williams (nwilliams)]

Thanks for the tweaks and clarifications.

review: Approve

[Peter Beaman (pbeaman)]

Has a conflict. Marked Rejected. Will fix and re-approve.

[Peter Beaman (pbeaman)]

Merge from trunk (now with branch fix-dynamic-volumes) and fix the conflict in JournalManager.

[Peter Beaman (pbeaman)]

Approving this since the only changes are mechanical.

=== modified file 'src/main/java/com/persistit/BufferPool.java'
--- src/main/java/com/persistit/BufferPool.java	2012-09-27 18:22:25 +0000
+++ src/main/java/com/persistit/BufferPool.java	2012-10-08 19:12:23 +0000
@@ -15,6 +15,8 @@
 
 package com.persistit;
 
+import static com.persistit.util.Util.NS_PER_S;
+
 import java.io.DataOutputStream;
 import java.io.IOException;
 import java.nio.ByteBuffer;
@@ -97,11 +99,10 @@
      */
     private final static int INVENTORY_VERSIONS = 3;
 
-    private final static long NS_PER_SEC = 1000000000;
     /**
      * Preload log message interval, in seconds
      */
-    private final static long INVENTORY_PRELOAD_LOG_MESSAGE_NS = 60L * NS_PER_SEC;
+    private final static long INVENTORY_PRELOAD_LOG_MESSAGE_NS = 60L * NS_PER_S;
 
     /**
      * The Persistit instance that references this BufferPool.
@@ -1514,8 +1515,8 @@
                     count++;
                     final long now = System.nanoTime();
                     if (now - reportTime >= INVENTORY_PRELOAD_LOG_MESSAGE_NS) {
-                        _persistit.getLogBase().bufferInventoryProgress.log(count, total, (now - reportTime)
-                                / NS_PER_SEC);
+                        _persistit.getLogBase().bufferInventoryProgress
+                                .log(count, total, (now - reportTime) / NS_PER_S);
                         reportTime = now;
                     }
                     if (count >= _bufferCount) {
@@ -1533,7 +1534,7 @@
             _persistit.getLogBase().bufferInventoryException.log(e);
         } finally {
             final long now = System.nanoTime();
-            _persistit.getLogBase().bufferInventoryProgress.log(count, total, (now - reportTime) / NS_PER_SEC);
+            _persistit.getLogBase().bufferInventoryProgress.log(count, total, (now - reportTime) / NS_PER_S);
         }
     }
 

=== modified file 'src/main/java/com/persistit/CheckpointManager.java'
--- src/main/java/com/persistit/CheckpointManager.java	2012-08-24 14:00:17 +0000
+++ src/main/java/com/persistit/CheckpointManager.java	2012-10-08 19:12:23 +0000
@@ -15,6 +15,8 @@
 
 package com.persistit;
 
+import static com.persistit.util.Util.NS_PER_S;
+
 import java.text.SimpleDateFormat;
 import java.util.ArrayList;
 import java.util.Date;
@@ -86,8 +88,6 @@
         }
     }
 
-    private final static long NS_PER_S = 1000000000L;
-
     /**
      * Default interval in nanoseconds between checkpoints - two minutes.
      */

=== modified file 'src/main/java/com/persistit/JournalManager.java'
--- src/main/java/com/persistit/JournalManager.java	2012-10-03 18:19:49 +0000
+++ src/main/java/com/persistit/JournalManager.java	2012-10-08 19:12:23 +0000
@@ -19,6 +19,7 @@
 import static com.persistit.util.SequencerConstants.PAGE_MAP_READ_INVALIDATE_A;
 import static com.persistit.util.SequencerConstants.RECOVERY_PRUNING_B;
 import static com.persistit.util.ThreadSequencer.sequence;
+import static com.persistit.util.Util.NS_PER_MS;
 
 import java.io.File;
 import java.io.IOException;
@@ -79,7 +80,6 @@
     final static int HALF_URGENT = 5;
     final static int URGENT_COMMIT_DELAY_MILLIS = 50;
     final static int GENTLE_COMMIT_DELAY_MILLIS = 12;
-    private final static long NS_PER_MS = 1000000L;
     private final static int IO_MEASUREMENT_CYCLES = 8;
     private final static int TOO_MANY_WARN_THRESHOLD = 5;
     private final static int TOO_MANY_ERROR_THRESHOLD = 10;
@@ -1271,11 +1271,6 @@
         return address % _blockSize;
     }
 
-    void stopCopier() {
-        _copier.setShouldStop(true);
-        _persistit.waitForIOTaskStop(_copier);
-    }
-
     void setWriteBufferSize(final int size) {
         if (size < MINIMUM_BUFFER_SIZE || size > MAXIMUM_BUFFER_SIZE) {
             throw new IllegalArgumentException("Invalid write buffer size: " + size);
@@ -1284,11 +1279,7 @@
     }
 
     public void close() throws PersistitException {
-
-        synchronized (this) {
-            _closed.set(true);
-        }
-
+        _closed.set(true);
         rollover();
 
         final JournalCopier copier = _copier;
@@ -2456,10 +2447,7 @@
         int handle = -1;
 
         for (final Iterator<PageNode> iterator = list.iterator(); iterator.hasNext();) {
-            if (_closed.get() && !_copyFast.get() || _appendOnly.get()) {
-                list.clear();
-                break;
-            }
+
             final PageNode pageNode = iterator.next();
             if (pageNode.isInvalid()) {
                 iterator.remove();
@@ -2520,11 +2508,6 @@
         final Set<Volume> volumes = new HashSet<Volume>();
 
         for (final Iterator<PageNode> iterator = list.iterator(); iterator.hasNext();) {
-            if (_closed.get() && !_copyFast.get() || _appendOnly.get()) {
-                list.clear();
-                break;
-            }
-
             final PageNode pageNode = iterator.next();
 
             if (pageNode.getVolumeHandle() != handle) {

=== modified file 'src/main/java/com/persistit/JournalManagerBench.java'
--- src/main/java/com/persistit/JournalManagerBench.java	2012-09-12 20:36:27 +0000
+++ src/main/java/com/persistit/JournalManagerBench.java	2012-10-08 19:12:23 +0000
@@ -15,6 +15,8 @@
 
 package com.persistit;
 
+import static com.persistit.util.Util.NS_PER_S;
+
 import java.io.File;
 import java.io.RandomAccessFile;
 import java.nio.ByteBuffer;
@@ -44,7 +46,6 @@
  */
 public class JournalManagerBench {
 
-    private final long NS_PER_S = 1000000000L;
     private final byte[] NULLS = new byte[65536];
 
     private final String[] ARG_TEMPLATE = new String[] { "duration|int:10:10:86400|Duration of test in seconds",

=== modified file 'src/main/java/com/persistit/Persistit.java'
--- src/main/java/com/persistit/Persistit.java	2012-10-03 16:04:16 +0000
+++ src/main/java/com/persistit/Persistit.java	2012-10-08 19:12:23 +0000
@@ -15,6 +15,8 @@
 
 package com.persistit;
 
+import static com.persistit.util.Util.NS_PER_S;
+
 import java.io.BufferedReader;
 import java.io.File;
 import java.io.FileReader;
@@ -1514,7 +1516,7 @@
         return _bufferPoolTable;
     }
 
-    public void cleanup() {
+    void cleanup() {
         final Set<SessionId> sessionIds;
         synchronized (_transactionSessionMap) {
             sessionIds = new HashSet<SessionId>(_transactionSessionMap.keySet());
@@ -1659,12 +1661,8 @@
             }
             recordBufferPoolInventory();
 
-            /*
-             * The copier is responsible for background pruning of aborted
-             * transactions. Halt it so Transaction#close() can be called
-             * without being concerned about its state changing.
-             */
-            _journalManager.stopCopier();
+            _cleanupManager.close(flush);
+            waitForIOTaskStop(_cleanupManager);
 
             getTransaction().close();
             cleanup();
@@ -1681,9 +1679,6 @@
                 }
             }
 
-            _cleanupManager.close(flush);
-            waitForIOTaskStop(_cleanupManager);
-
             _checkpointManager.close(flush);
             waitForIOTaskStop(_checkpointManager);
 
@@ -1693,26 +1688,13 @@
                 pool.close();
             }
 
-            /*
-             * Close (and abort) all remaining transactions.
-             */
-            Set<Transaction> transactions;
-            synchronized (_transactionSessionMap) {
-                transactions = new HashSet<Transaction>(_transactionSessionMap.values());
-                _transactionSessionMap.clear();
-            }
-            for (final Transaction txn : transactions) {
-                try {
-                    txn.close();
-                } catch (final PersistitException e) {
-                    _logBase.exception.log(e);
-                }
-            }
-
             _journalManager.close();
             final IOTaskRunnable task = _transactionIndex.close();
             waitForIOTaskStop(task);
 
+            interruptActiveThreads(SHORT_DELAY);
+            closeZombieTransactions();
+
             for (final Volume volume : volumes) {
                 volume.close();
             }
@@ -1730,6 +1712,50 @@
         releaseAllResources();
     }
 
+    private void closeZombieTransactions() {
+        final Set<SessionId> sessionIds;
+        synchronized (_transactionSessionMap) {
+            sessionIds = new HashSet<SessionId>(_transactionSessionMap.keySet());
+        }
+        for (final SessionId sessionId : sessionIds) {
+            Transaction transaction = null;
+            synchronized (_transactionSessionMap) {
+                transaction = _transactionSessionMap.remove(sessionId);
+            }
+            if (!sessionId.isAlive()) {
+                if (transaction != null) {
+                    try {
+                        transaction.close();
+                    } catch (final Exception e) {
+                        _logBase.exception.log(e);
+                    }
+                }
+            }
+        }
+    }
+
+    private void interruptActiveThreads(final long timeout) throws PersistitInterruptedException {
+        final long expires = System.currentTimeMillis() + timeout;
+        boolean remaining = false;
+        do {
+            final Set<SessionId> sessionIds;
+            synchronized (_transactionSessionMap) {
+                sessionIds = new HashSet<SessionId>(_transactionSessionMap.keySet());
+            }
+            for (final SessionId sessionId : sessionIds) {
+                if (sessionId.isAlive()) {
+                    if (sessionId.interrupt()) {
+                        _logBase.interruptedAtClose.log(sessionId.ownerName());
+                    }
+                    remaining = true;
+                }
+            }
+            if (remaining) {
+                Util.spinSleep();
+            }
+        } while (remaining && System.currentTimeMillis() < expires);
+    }
+
     /**
      * Abruptly stop (using {@link Thread#stop()}) the writer and collector
      * processes. This method should be used only by tests.
@@ -1885,7 +1911,7 @@
             }
             final long now = System.currentTimeMillis();
             if (now > _nextCloseTime) {
-                _logBase.waitForClose.log((_nextCloseTime - _beginCloseTime) / 1000);
+                _logBase.waitForClose.log((_nextCloseTime - _beginCloseTime) / NS_PER_S);
                 _nextCloseTime += CLOSE_LOG_INTERVAL;
             }
         }

=== modified file 'src/main/java/com/persistit/SessionId.java'
--- src/main/java/com/persistit/SessionId.java	2012-09-26 13:31:13 +0000
+++ src/main/java/com/persistit/SessionId.java	2012-10-08 19:12:23 +0000
@@ -77,6 +77,16 @@
         _owner.set(Thread.currentThread());
     }
 
+    boolean interrupt() {
+        final Thread t = _owner.get();
+        if (t != null && t != Thread.currentThread()) {
+            t.interrupt();
+            return true;
+        } else {
+            return false;
+        }
+    }
+
     public String ownerName() {
         final Thread t = _owner.get();
         if (t == null) {

=== modified file 'src/main/java/com/persistit/Transaction.java'
--- src/main/java/com/persistit/Transaction.java	2012-08-24 13:57:19 +0000
+++ src/main/java/com/persistit/Transaction.java	2012-10-08 19:12:23 +0000
@@ -725,7 +725,6 @@
                 _persistit.getTransactionIndex().notifyCompleted(_transactionStatus,
                         _persistit.getTimestampAllocator().getCurrentTimestamp());
                 _rollbackCompleted = true;
-
             }
         }
     }

=== modified file 'src/main/java/com/persistit/TransactionIndexBucket.java'
--- src/main/java/com/persistit/TransactionIndexBucket.java	2012-08-24 13:57:19 +0000
+++ src/main/java/com/persistit/TransactionIndexBucket.java	2012-10-08 19:12:23 +0000
@@ -170,10 +170,13 @@
         _lock.unlock();
     }
 
-    TransactionStatus allocateTransactionStatus() {
+    TransactionStatus allocateTransactionStatus() throws InterruptedException {
         assert _lock.isHeldByCurrentThread();
         final TransactionStatus status = _free;
         if (status != null) {
+            if (status.isLocked()) {
+                status.briefLock();
+            }
             assert !status.isLocked();
             _free = status.getNext();
             _freeCount--;
@@ -569,14 +572,7 @@
                     }
                 }
             } else if (tc < 0 && tc != ABORTED && -tc < timestamp) {
-                boolean locked = false;
-                try {
-                    locked = status.wwLock(TransactionIndex.SHORT_TIMEOUT);
-                } finally {
-                    if (locked) {
-                        status.wwUnlock();
-                    }
-                }
+                status.briefLock();
                 _transactionIndex.incrementAccumulatorSnapshotRetryCounter();
                 throw RetryException.SINGLE;
             }

=== modified file 'src/main/java/com/persistit/TransactionStatus.java'
--- src/main/java/com/persistit/TransactionStatus.java	2012-08-24 13:57:19 +0000
+++ src/main/java/com/persistit/TransactionStatus.java	2012-10-08 19:12:23 +0000
@@ -327,6 +327,26 @@
     }
 
     /**
+     * Block briefly until another thread transiently holding the wwLock
+     * vacates. Times out and returns after
+     * {@value TransactionIndex#SHORT_TIMEOUT} milliseconds.
+     * 
+     * @throws InterruptedException
+     */
+    void briefLock() throws InterruptedException {
+        boolean locked = false;
+        try {
+            locked = wwLock(TransactionIndex.SHORT_TIMEOUT);
+        } catch (final InterruptedException ie) {
+            Thread.currentThread().interrupt();
+        } finally {
+            if (locked) {
+                wwUnlock();
+            }
+        }
+    }
+
+    /**
      * <p>
      * Acquire a lock on this TransactionStatus. This supports the
      * {@link TransactionIndex#wwDependency(long, long, long)} method. While a

=== modified file 'src/main/java/com/persistit/VolumeHeader.java'
--- src/main/java/com/persistit/VolumeHeader.java	2012-08-24 13:57:19 +0000
+++ src/main/java/com/persistit/VolumeHeader.java	2012-10-08 19:12:23 +0000
@@ -287,11 +287,12 @@
      */
     public static boolean verifyVolumeHeader(final VolumeSpecification specification, final long systemTimestamp)
             throws CorruptVolumeException, InvalidVolumeSpecificationException, PersistitIOException {
+        FileInputStream stream = null;
         try {
             final File file = new File(specification.getPath());
             if (file.exists()) {
                 if (file.isFile()) {
-                    final FileInputStream stream = new FileInputStream(file);
+                    stream = new FileInputStream(file);
                     final byte[] bytes = new byte[SIZE];
                     final int readSize = stream.read(bytes);
                     if (readSize < SIZE) {
@@ -336,6 +337,13 @@
             }
         } catch (final IOException ioe) {
             throw new PersistitIOException(ioe);
+        } finally {
+            if (stream != null) {
+                try {
+                    stream.close();
+                } catch (final IOException e) {
+                }
+            }
         }
     }
 

=== modified file 'src/main/java/com/persistit/logging/LogBase.java'
--- src/main/java/com/persistit/logging/LogBase.java	2012-09-06 20:48:31 +0000
+++ src/main/java/com/persistit/logging/LogBase.java	2012-10-08 19:12:23 +0000
@@ -253,6 +253,9 @@
     @Message("WARNING|Exception while writing buffer pool inventory %s")
     public final LogItem bufferInventoryException = PersistitLogMessage.empty();
 
+    @Message("WARNING|Thread %s interrupted due to shutdown")
+    public final LogItem interruptedAtClose = PersistitLogMessage.empty();
+
     public static String recurring(final String message, final int count, final long duration) {
         return String.format(RECURRING, message, count, duration);
     }

=== modified file 'src/main/java/com/persistit/util/Util.java'
--- src/main/java/com/persistit/util/Util.java	2012-08-24 13:57:19 +0000
+++ src/main/java/com/persistit/util/Util.java	2012-10-08 19:12:23 +0000
@@ -38,6 +38,10 @@
     public final static String NEW_LINE = System.getProperty("line.separator");
     private final static String REGEX_QUOTE = "^$*+?()[].";
 
+    public final static long NS_PER_S = 1000000000L;
+    public final static long MS_PER_S = 1000L;
+    public final static long NS_PER_MS = 1000000L;
+
     public final static char[] HEX_DIGITS = { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D',
             'E', 'F' };
 

=== modified file 'src/test/java/com/persistit/IOFailureTest.java'
--- src/test/java/com/persistit/IOFailureTest.java	2012-08-24 14:00:17 +0000
+++ src/test/java/com/persistit/IOFailureTest.java	2012-10-08 19:12:23 +0000
@@ -308,6 +308,8 @@
         final long size0 = channel0.size();
         channel0.truncate(100);
 
+        channel0.close();
+
         final File file1 = jman.addressToFile(currentAddress);
         final FileChannel channel1 = new RandomAccessFile(file1, "rw").getChannel();
         final long size1 = channel1.size();

=== modified file 'src/test/java/com/persistit/TransactionTest2.java'
--- src/test/java/com/persistit/TransactionTest2.java	2012-08-24 13:57:19 +0000
+++ src/test/java/com/persistit/TransactionTest2.java	2012-10-08 19:12:23 +0000
@@ -29,6 +29,7 @@
 import org.junit.Test;
 
 import com.persistit.Transaction.CommitPolicy;
+import com.persistit.exception.PersistitClosedException;
 import com.persistit.exception.PersistitException;
 import com.persistit.exception.PersistitIOException;
 import com.persistit.exception.PersistitInterruptedException;
@@ -47,15 +48,17 @@
 
     final static CommitPolicy policy = CommitPolicy.SOFT;
 
-    final static long TIMEOUT = 20000; // 20 seconds
+    final static long TIMEOUT = 10000; // 10 seconds
+    final static int ITERATIONS_PER_THREAD = 25000;
 
     static int _threadCount = 8;
-    static int _iterationsPerThread = 25000;
+    static int _iterationsPerThread = ITERATIONS_PER_THREAD;
     static int _accounts = 5000;
 
     static AtomicInteger _retriedTransactionCount = new AtomicInteger();
     static AtomicInteger _completedTransactionCount = new AtomicInteger();
     static AtomicInteger _failedTransactionCount = new AtomicInteger();
+    static AtomicInteger _strandedThreads = new AtomicInteger();
 
     static int _threadCounter = 0;
 
@@ -118,7 +121,7 @@
             threadArray[index] = new Thread(new Runnable() {
                 @Override
                 public void run() {
-                    runIt();
+                    runIt(_iterationsPerThread);
                 }
             }, "TransactionThread_" + index);
 
@@ -182,7 +185,7 @@
                 final Thread thread = new Thread(new Runnable() {
                     @Override
                     public void run() {
-                        runIt();
+                        runIt(Integer.MAX_VALUE);
                     }
                 }, "TransactionThread_" + ++index);
                 threads.add(thread);
@@ -238,19 +241,44 @@
         txn.begin();
     }
 
-    public void runIt() {
+    @Test
+    public void transactionsConcurrentWithPersistitClose() throws Exception {
+        new Thread(new Runnable() {
+            @Override
+            public void run() {
+                final Thread[] threadArray = new Thread[_threadCount];
+                for (int index = 0; index < _threadCount; index++) {
+                    threadArray[index] = new Thread(new Runnable() {
+                        @Override
+                        public void run() {
+                            runIt(Integer.MAX_VALUE);
+                        }
+                    }, "TransactionThread_" + index);
+                }
+                for (int index = 0; index < _threadCount; index++) {
+                    threadArray[index].start();
+                }
+            }
+        }).start();
+        /*
+         * Let the threads crank up
+         */
+        Thread.sleep(1000);
+        _persistit.close();
+        assertEquals("All threads should have exited correctly", 0, _strandedThreads.get());
+    }
+
+    public void runIt(final int limit) {
+        _strandedThreads.incrementAndGet();
         try {
             final Exchange accountEx = _persistit.getExchange("persistit", "account", true);
             //
             final Random random = new Random();
-            for (int iterations = 1; iterations <= _iterationsPerThread; iterations++) {
+            for (int iterations = 1; iterations <= limit; iterations++) {
                 final int accountNo1 = random.nextInt(_accounts);
-                // int accountNo2 = random.nextInt(_accounts - 1);
-                // if (accountNo2 == accountNo1) accountNo2++;
                 final int accountNo2 = random.nextInt(_accounts);
 
                 final int delta = random.nextInt(10000);
-                // final int delta = 1;
 
                 transfer(accountEx, accountNo1, accountNo2, delta);
                 _completedTransactionCount.incrementAndGet();
@@ -260,10 +288,16 @@
                     System.out.flush();
                 }
             }
+            _strandedThreads.decrementAndGet();
         } catch (final PersistitInterruptedException exception) {
+            _strandedThreads.decrementAndGet();
+            // expected
+        } catch (final PersistitClosedException exception) {
+            _strandedThreads.decrementAndGet();
             // expected
         } catch (final PersistitIOException exception) {
             if (InterruptedIOException.class.equals(exception.getCause().getClass())) {
+                _strandedThreads.decrementAndGet();
                 // expected
             } else {
                 exception.printStackTrace();

=== modified file 'src/test/java/com/persistit/stress/AbstractStressTest.java'
--- src/test/java/com/persistit/stress/AbstractStressTest.java	2012-08-24 13:57:19 +0000
+++ src/test/java/com/persistit/stress/AbstractStressTest.java	2012-10-08 19:12:23 +0000
@@ -15,6 +15,8 @@
 
 package com.persistit.stress;
 
+import static com.persistit.util.Util.NS_PER_S;
+
 import com.persistit.Persistit;
 
 /**
@@ -128,7 +130,7 @@
             _result = new TestResult(false, throwable);
         }
         forceStop();
-        final long elapsed = (System.nanoTime() - _startTime) / AbstractSuite.NS_PER_S;
+        final long elapsed = (System.nanoTime() - _startTime) / NS_PER_S;
         System.err.printf("\n%s at %,d seconds: %s\n", this, elapsed, _result);
     }
 

=== modified file 'src/test/java/com/persistit/stress/AbstractSuite.java'
--- src/test/java/com/persistit/stress/AbstractSuite.java	2012-08-24 14:00:17 +0000
+++ src/test/java/com/persistit/stress/AbstractSuite.java	2012-10-08 19:12:23 +0000
@@ -15,6 +15,10 @@
 
 package com.persistit.stress;
 
+import static com.persistit.util.Util.MS_PER_S;
+import static com.persistit.util.Util.NS_PER_MS;
+import static com.persistit.util.Util.NS_PER_S;
+
 import java.io.File;
 import java.io.FileWriter;
 import java.io.IOException;
@@ -42,10 +46,6 @@
 
     protected final static long PROGRESS_LOG_INTERVAL = 600000;
 
-    protected final static long NS_PER_MS = 1000000;
-    protected final static long MS_PER_S = 1000;
-    protected final static long NS_PER_S = NS_PER_MS * MS_PER_S;
-
     private long _nextReport;
     private long _accumulatedWork;
 

=== modified file 'src/test/java/com/persistit/stress/StartStop.java'
--- src/test/java/com/persistit/stress/StartStop.java	2012-08-24 13:57:19 +0000
+++ src/test/java/com/persistit/stress/StartStop.java	2012-10-08 19:12:23 +0000
@@ -15,6 +15,8 @@
 
 package com.persistit.stress;
 
+import static com.persistit.util.Util.NS_PER_S;
+
 import com.persistit.IntegrityCheck;
 import com.persistit.Persistit;
 import com.persistit.Transaction.CommitPolicy;

=== modified file 'src/test/java/com/persistit/unit/ExchangeTest.java'
--- src/test/java/com/persistit/unit/ExchangeTest.java	2012-08-24 13:57:19 +0000
+++ src/test/java/com/persistit/unit/ExchangeTest.java	2012-10-08 19:12:23 +0000
@@ -368,4 +368,120 @@
         assertTrue("Not enough methods were tested: " + tested, tested > 10);
     }
 
+    /**
+     * Test for https://bugs.launchpad.net/akiban-persistit/+bug/1023549:
+     * 
+     * traverse(EQ, false, 0) returns incorrect result
+     * 
+     * This method returns true even when the tree is empty. traverse(EQ, true,
+     * 0) returns the correct value.
+     * 
+     * @throws Exception
+     */
+    @Test
+    public void traverse_EQ_false_0__IsCorrect() throws Exception {
+        traverseCases(false);
+        traverseCases(true);
+    }
+
+    /**
+     * Test for https://bugs.launchpad.net/akiban-persistit/+bug/1023549:
+     * 
+     * traverse(EQ, false, 0) returns incorrect result
+     * 
+     * This method returns true even when the tree is empty. traverse(EQ, true,
+     * 0) returns the correct value.
+     * 
+     * @throws Exception
+     */
+    @Test
+    public void traverse_EQ_false_0__IsCorrect_Txn() throws Exception {
+        final Transaction txn = _persistit.getTransaction();
+        txn.begin();
+        traverseCases(false);
+        txn.commit();
+        txn.end();
+
+        txn.begin();
+        traverseCases(true);
+        txn.commit();
+        txn.end();
+    }
+
+    private void traverseCases(final boolean deep) throws Exception {
+        final Exchange ex = _persistit.getExchange("persistit", "gogo", true);
+
+        ex.removeAll();
+        ex.clear();
+        assertEquals("Should be false", false, ex.traverse(Key.EQ, deep, -1));
+        ex.clear();
+        assertEquals("Should be false", false, ex.traverse(Key.GTEQ, deep, -1));
+        ex.clear();
+        assertEquals("Should be false", false, ex.traverse(Key.GT, deep, -1));
+        ex.clear();
+        assertEquals("Should be false", false, ex.traverse(Key.LTEQ, deep, -1));
+        ex.clear();
+        assertEquals("Should be false", false, ex.traverse(Key.LT, deep, -1));
+        ex.clear();
+
+        ex.append(1).append(2).store();
+        ex.clear().append(Key.BEFORE);
+        assertEquals("Should be false", false, ex.traverse(Key.EQ, deep, -1));
+        assertEquals("Should be true", true, ex.traverse(Key.GTEQ, deep, -1));
+        assertEquals("Should be true", true, ex.traverse(Key.GTEQ, deep, -1));
+        ex.clear().append(1);
+        assertEquals("Should be " + !deep, !deep, ex.traverse(Key.EQ, deep, -1));
+
+        ex.clear().append(Key.AFTER);
+        assertEquals("Should be false", false, ex.traverse(Key.EQ, deep, -1));
+        ex.clear().append(Key.AFTER);
+        assertEquals("Should be true", true, ex.traverse(Key.LTEQ, deep, -1));
+        assertEquals("Should be true", true, ex.traverse(Key.LTEQ, deep, -1));
+
+        ex.removeAll();
+        ex.clear();
+        assertEquals("Should be false", false, ex.traverse(Key.EQ, deep, 0));
+        keyCheck(ex, "{{before}}");
+        ex.clear();
+        assertEquals("Should be false", false, ex.traverse(Key.GTEQ, deep, 0));
+        keyCheck(ex, "{{before}}");
+        ex.clear();
+        assertEquals("Should be false", false, ex.traverse(Key.GT, deep, 0));
+        keyCheck(ex, "{{before}}");
+        ex.clear();
+        assertEquals("Should be false", false, ex.traverse(Key.LTEQ, deep, 0));
+        keyCheck(ex, "{{after}}");
+        ex.clear();
+        assertEquals("Should be false", false, ex.traverse(Key.LT, deep, 0));
+        keyCheck(ex, "{{after}}");
+        ex.clear();
+
+        ex.append(1).append(2).store();
+        ex.clear().append(Key.BEFORE);
+        assertEquals("Should be false", false, ex.traverse(Key.EQ, deep, 0));
+        keyCheck(ex, "{{before}}");
+        assertEquals("Should be true", true, ex.traverse(Key.GTEQ, deep, 0));
+        keyCheck(ex, deep ? "{1,2}" : "{1}");
+        assertEquals("Should be true", true, ex.traverse(Key.GTEQ, deep, 0));
+        keyCheck(ex, deep ? "{1,2}" : "{1}");
+        assertEquals("Should be true", true, ex.traverse(Key.EQ, deep, 0));
+        keyCheck(ex, deep ? "{1,2}" : "{1}");
+
+        ex.clear().append(Key.AFTER);
+        assertEquals("Should be false", false, ex.traverse(Key.EQ, deep, 0));
+        keyCheck(ex, "{{before}}");
+        ex.clear().append(Key.AFTER);
+        assertEquals("Should be true", true, ex.traverse(Key.LTEQ, deep, 0));
+        keyCheck(ex, deep ? "{1,2}" : "{1}");
+        assertEquals("Should be true", true, ex.traverse(Key.LTEQ, deep, 0));
+        keyCheck(ex, deep ? "{1,2}" : "{1}");
+        assertEquals("Should be true", true, ex.traverse(Key.EQ, deep, 0));
+        keyCheck(ex, deep ? "{1,2}" : "{1}");
+
+    }
+
+    private void keyCheck(final Exchange ex, final String expected) {
+        assertEquals("Key should be " + expected, expected, ex.getKey().toString());
+    }
+
 }