Synchroniser to wait for new peer if best is up to date #1161
Conversation
When chain downloading, the SyncTargetManager continues to attempt to download headers/blocks from the current syncTarget, even if the system is "insync". This is due to the fact that the "insync" check is only performed at initial peer selection, rather than on each loop of the chain downloader.
| .map(CompletableFuture::completedFuture) // Return an existing sync target if present | ||
| .orElseGet(this::selectNewSyncTarget); | ||
| if (syncState.syncTarget().isPresent() && !syncState.isInSync()) { | ||
| return CompletableFuture.completedFuture(syncState.syncTarget().get()); |
There was a problem hiding this comment.
Overall this looks like the right direction.
The isInSync check has a small tolerance to avoid rapidly toggling between inSync/outOfSync when a new block is first published but we don't want that behaviour here. It's common to want different tolerances in different situations so we probably should introduce a isInSync(long tolerance) variant and use isInSync(0) here.
We could also remove the syncState.syncTarget().isPresent() condition because isInSync will always be true if there is no sync target.
|
|
||
| @Override | ||
| public boolean isCaughtUpWithPeer(final EthPeer peer) { | ||
| return peer.chainState().getEstimatedHeight() < pivotBlockHeader.getNumber(); |
There was a problem hiding this comment.
This isn't the right thing to extract here. It is the right condition for where it was (when fast syncing we can select any peer as our target as long as their chain height is above the pivot block). We've caught up with that peer when our chain height >= pivotBlockHeader.getNumber().
| @@ -144,4 +144,15 @@ public void clearSyncTarget(final SyncTarget syncTarget) { | |||
| public abstract boolean shouldSwitchSyncTarget(final SyncTarget currentTarget); | |||
|
|
|||
| public abstract boolean shouldContinueDownloading(); | |||
|
|
|||
| public abstract boolean isCaughtUpWithPeer(final EthPeer peer); | |||
There was a problem hiding this comment.
This (and implementations) could probably be protected.
When chain downloading, the SyncTargetManager continues to
attempt to download headers/blocks from the current syncTarget,
even if the system is "insync". This is due to the fact that
the "insync" check is only performed at initial peer selection,
rather than on each loop of the chain downloader.