Avoid requesting blocks that aren't needed by the ledger.

This isn't done for the sake of optimization - it's done so we won't have misleading warning messages in the log file due to frequently canceled contexts.

Author: tsachiherman

catchup/service.go

@@ -18,6 +18,7 @@ package catchup
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"sync"
 	"sync/atomic"
@@ -155,8 +156,19 @@ func (s *Service) SynchronizingTime() time.Duration {
 	return time.Duration(timeInNS - startNS)
 }
 
+// errLedgerAlreadyHasBlock is returned by innerFetch in case the local ledger already has the requested block.
+var errLedgerAlreadyHasBlock = errors.New("ledger already has block")
+
 // function scope to make a bunch of defer statements better
 func (s *Service) innerFetch(r basics.Round, peer network.Peer) (blk *bookkeeping.Block, cert *agreement.Certificate, ddur time.Duration, err error) {
+	ledgerWaitCh := s.ledger.Wait(r)
+	select {
+	case <-ledgerWaitCh:
+		// if our ledger already have this block, no need to attempt to fetch it.
+		return nil, nil, time.Duration(0), errLedgerAlreadyHasBlock
+	default:
+	}
+
 	ctx, cf := context.WithCancel(s.ctx)
 	fetcher := makeUniversalBlockFetcher(s.log, s.net, s.cfg)
 	defer cf()
@@ -165,7 +177,7 @@ func (s *Service) innerFetch(r basics.Round, peer network.Peer) (blk *bookkeepin
 	go func() {
 		select {
 		case <-stopWaitingForLedgerRound:
-		case <-s.ledger.Wait(r):
+		case <-ledgerWaitCh:
 			cf()
 		}
 	}()
@@ -218,6 +230,10 @@ func (s *Service) fetchAndWrite(r basics.Round, prevFetchCompleteChan chan bool,
 		block, cert, blockDownloadDuration, err := s.innerFetch(r, peer)
 
 		if err != nil {
+			if err == errLedgerAlreadyHasBlock {
+				// ledger already has the block, no need to request this block from anyone.
+				return true
+			}
 			s.log.Debugf("fetchAndWrite(%v): Could not fetch: %v (attempt %d)", r, err, i)
 			peerSelector.rankPeer(psp, peerRankDownloadFailed)
 			// we've just failed to retrieve a block; wait until the previous block is fetched before trying again