PeerGroup: sync improvements round two. Move chain download speed monitoring out of AbstractBlockChain and fix it so it doesn't sometimes print garbage. Add a stall detector. Next step is to force switch download peers when there's a long enough stall.

core/src/main/java/org/bitcoinj/core/AbstractBlockChain.java

@@ -350,24 +350,13 @@ public abstract class AbstractBlockChain {
      */
     protected abstract TransactionOutputChanges connectTransactions(StoredBlock newBlock) throws VerificationException, BlockStoreException, PrunedException;    
     
-    // Stat counters.
-    private long statsLastTime = System.currentTimeMillis();
-    private long statsBlocksAdded;
-
     // filteredTxHashList contains all transactions, filteredTxn just a subset
     private boolean add(Block block, boolean tryConnecting,
                         @Nullable List<Sha256Hash> filteredTxHashList, @Nullable Map<Sha256Hash, Transaction> filteredTxn)
             throws BlockStoreException, VerificationException, PrunedException {
+        // TODO: Use read/write locks to ensure that during chain download properties are still low latency.
         lock.lock();
         try {
-            // TODO: Use read/write locks to ensure that during chain download properties are still low latency.
-            if (System.currentTimeMillis() - statsLastTime > 1000) {
-                // More than a second passed since last stats logging.
-                if (statsBlocksAdded > 1)
-                    log.info("{} blocks per second", statsBlocksAdded);
-                statsLastTime = System.currentTimeMillis();
-                statsBlocksAdded = 0;
-            }
             // Quick check for duplicates to avoid an expensive check further down (in findSplit). This can happen a lot
             // when connecting orphan transactions due to the dumb brute force algorithm we use.
             if (block.equals(getChainHead().getHeader())) {
@@ -429,7 +418,6 @@ public abstract class AbstractBlockChain {
             if (tryConnecting)
                 tryConnectingOrphans();
 
-            statsBlocksAdded++;
             return true;
         } finally {
             lock.unlock();

core/src/main/java/org/bitcoinj/core/PeerGroup.java

@@ -100,7 +100,7 @@ public class PeerGroup implements TransactionBroadcaster {
 
     // The peer that has been selected for the purposes of downloading announced data.
     @GuardedBy("lock") private Peer downloadPeer;
-    // Callback for events related to chain download
+    // Callback for events related to chain download.
     @Nullable @GuardedBy("lock") private PeerEventListener downloadListener;
     // Callbacks for events related to peer connection/disconnection
     private final CopyOnWriteArrayList<ListenerRegistration<PeerEventListener>> peerEventListeners;
@@ -1461,10 +1461,46 @@ public class PeerGroup implements TransactionBroadcaster {
         }
     }
 
+    private class ChainDownloadSpeedCalculator extends AbstractPeerEventListener implements Runnable {
+        private int blocksInLastSecond, stallWarning;
+
+        @Override
+        public synchronized void onBlocksDownloaded(Peer peer, Block block, int blocksLeft) {
+            blocksInLastSecond++;
+        }
+
+        @Override
+        public synchronized void run() {
+            if (blocksInLastSecond > 1) {
+                log.info("{} blocks per second", blocksInLastSecond);
+                stallWarning = 0;
+            }
+            if (chain != null && chain.getBestChainHeight() < getMostCommonChainHeight() && blocksInLastSecond == 0 && stallWarning > -1) {
+                stallWarning++;
+                final int STALL_PERIOD_SECONDS = 3;
+                if (stallWarning == STALL_PERIOD_SECONDS) {
+                    stallWarning = -1;
+                    log.warn("Chain download stalled: no progress for {} seconds", STALL_PERIOD_SECONDS);
+                    // TODO: Consider disconnecting the stalled peer here.
+                }
+            }
+            blocksInLastSecond = 0;
+        }
+    }
+    @Nullable private ChainDownloadSpeedCalculator chainDownloadSpeedCalculator;
+
     private void startBlockChainDownloadFromPeer(Peer peer) {
         lock.lock();
         try {
             setDownloadPeer(peer);
+
+            if (chainDownloadSpeedCalculator == null) {
+                // Every second, run the calculator which will log how fast we are downloading the chain.
+                chainDownloadSpeedCalculator = new ChainDownloadSpeedCalculator();
+                executor.scheduleAtFixedRate(chainDownloadSpeedCalculator, 1, 1, TimeUnit.SECONDS);
+                peer.addEventListener(chainDownloadSpeedCalculator, Threading.SAME_THREAD);
+            }
+
             // startBlockChainDownload will setDownloadData(true) on itself automatically.
             peer.startBlockChainDownload();
         } finally {

core/src/test/java/org/bitcoinj/core/PeerGroupTest.java

@@ -486,9 +486,10 @@ public class PeerGroupTest extends TestWithPeerGroup {
         assertEquals(a, peerGroup.getDownloadPeer());  // No change yet.
         connectPeer(4, versionMessage3);
         assertEquals(3, peerGroup.getMostCommonChainHeight());
-        assertEquals(c, peerGroup.getDownloadPeer());  // Switch to first peer advertising new height.
+        assertEquals(a, peerGroup.getDownloadPeer());  // Still no change.
+
         // New peer with a higher protocol version but same chain height.
-        //TODO: When PeerGroup.selectDownloadPeer.PREFERRED_VERSION is not equal to vMinRequiredProtocolVersion,
+        // TODO: When PeerGroup.selectDownloadPeer.PREFERRED_VERSION is not equal to vMinRequiredProtocolVersion,
         // reenable this test
         /*VersionMessage versionMessage4 = new VersionMessage(params, 3);
         versionMessage4.clientVersion = 100000;