From 236b29aeaf2c0f177ec707084764be673e3dcbfa Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Tue, 12 May 2020 16:39:15 +0200 Subject: [PATCH] eth: downloader + queue: more context in errors --- eth/downloader/downloader.go | 37 +++++++++++++++++++++++++----------- eth/downloader/queue.go | 18 ++++++++++-------- 2 files changed, 36 insertions(+), 19 deletions(-) diff --git a/eth/downloader/downloader.go b/eth/downloader/downloader.go index 2d08071948..0cff145447 100644 --- a/eth/downloader/downloader.go +++ b/eth/downloader/downloader.go @@ -321,13 +321,28 @@ func (d *Downloader) UnregisterPeer(id string) error { // adding various sanity checks as well as wrapping it with various log entries. func (d *Downloader) Synchronise(id string, head common.Hash, td *big.Int, mode SyncMode) error { err := d.synchronise(id, head, td, mode) - switch err { - case nil: - case errBusy, errCanceled: + switch err { + case nil, errBusy, errCanceled: + return err + } + + if errors.Is(err, errInvalidChain) { + log.Warn("Synchronisation failed, dropping peer", "peer", id, "err", err) + if d.dropPeer == nil { + // The dropPeer method is nil when `--copydb` is used for a local copy. + // Timeouts can occur if e.g. compaction hits at the wrong time, and can be ignored + log.Warn("Downloader wants to drop peer, but peerdrop-function is not set", "peer", id) + } else { + d.dropPeer(id) + } + return err + } + + switch err { case errTimeout, errBadPeer, errStallingPeer, errUnsyncedPeer, errEmptyHeaderSet, errPeersUnavailable, errTooOld, - errInvalidAncestor, errInvalidChain: + errInvalidAncestor: log.Warn("Synchronisation failed, dropping peer", "peer", id, "err", err) if d.dropPeer == nil { // The dropPeer method is nil when `--copydb` is used for a local copy. @@ -774,7 +789,7 @@ func (d *Downloader) findAncestor(p *peerConnection, remoteHeader *types.Header) expectNumber := from + int64(i)*int64(skip+1) if number := header.Number.Int64(); number != expectNumber { p.log.Warn("Head headers broke chain ordering", "index", i, "requested", expectNumber, "received", number) - return 0, errInvalidChain + return 0, fmt.Errorf("%w: %v", errInvalidChain, errors.New("head headers broke chain ordering")) } } // Check if a common ancestor was found @@ -988,7 +1003,7 @@ func (d *Downloader) fetchHeaders(p *peerConnection, from uint64, pivot uint64) filled, proced, err := d.fillHeaderSkeleton(from, headers) if err != nil { p.log.Debug("Skeleton chain invalid", "err", err) - return errInvalidChain + return fmt.Errorf("%w: %v", errInvalidChain, err) } headers = filled[proced:] from += uint64(proced) @@ -1207,13 +1222,13 @@ func (d *Downloader) fetchParts(deliveryCh chan dataPack, deliver func(dataPack) if peer := d.peers.Peer(packet.PeerId()); peer != nil { // Deliver the received chunk of data and check chain validity accepted, err := deliver(packet) - if err == errInvalidChain { + if errors.Is(err, errInvalidChain) { return err } // Unless a peer delivered something completely else than requested (usually // caused by a timed out request which came through in the end), set it to // idle. If the delivery's stale, the peer should have already been idled. - if err != errStaleDelivery { + if !errors.Is(err, errStaleDelivery) { setIdle(peer, accepted) } // Issue a log to the user to see what's going on @@ -1473,7 +1488,7 @@ func (d *Downloader) processHeaders(origin uint64, pivot uint64, td *big.Int) er rollback = append(rollback, chunk[:n]...) } log.Debug("Invalid header encountered", "number", chunk[n].Number, "hash", chunk[n].Hash(), "err", err) - return errInvalidChain + return fmt.Errorf("%w: %v", errInvalidChain, err) } // All verifications passed, store newly found uncertain headers rollback = append(rollback, unknown...) @@ -1565,7 +1580,7 @@ func (d *Downloader) importBlockResults(results []*fetchResult) error { // of the blocks delivered from the downloader, and the indexing will be off. log.Debug("Downloaded item processing failed on sidechain import", "index", index, "err", err) } - return errInvalidChain + return fmt.Errorf("%w: %v", errInvalidChain, err) } return nil } @@ -1706,7 +1721,7 @@ func (d *Downloader) commitFastSyncData(results []*fetchResult, stateSync *state } if index, err := d.blockchain.InsertReceiptChain(blocks, receipts, d.ancientLimit); err != nil { log.Debug("Downloaded item processing failed", "number", results[index].Header.Number, "hash", results[index].Header.Hash(), "err", err) - return errInvalidChain + return fmt.Errorf("%w: %v", errInvalidChain, err) } return nil } diff --git a/eth/downloader/queue.go b/eth/downloader/queue.go index 6ce0cbd515..9aea102539 100644 --- a/eth/downloader/queue.go +++ b/eth/downloader/queue.go @@ -509,7 +509,7 @@ func (q *queue) reserveHeaders(p *peerConnection, count int, taskPool map[common index := int(header.Number.Int64() - int64(q.resultOffset)) if index >= len(q.resultCache) || index < 0 { common.Report("index allocation went beyond available resultCache space") - return nil, false, errInvalidChain + return nil, false, fmt.Errorf("%w: index allocation went beyond available resultCache space", errInvalidChain) } if q.resultCache[index] == nil { components := 1 @@ -863,14 +863,16 @@ func (q *queue) deliver(id string, taskPool map[common.Hash]*types.Header, taskQ q.active.Signal() } // If none of the data was good, it's a stale delivery - switch { - case failure == nil || failure == errInvalidChain: - return accepted, failure - case useful: - return accepted, fmt.Errorf("partial failure: %v", failure) - default: - return accepted, errStaleDelivery + if failure == nil { + return accepted, nil } + if errors.Is(failure, errInvalidChain) { + return accepted, failure + } + if useful { + return accepted, fmt.Errorf("partial failure: %v", failure) + } + return accepted, fmt.Errorf("%w: %v", failure, errStaleDelivery) } // Prepare configures the result cache to allow accepting and caching inbound