Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

[NC-1273] Consider peer count insufficient until minimum peers for fast sync are connected #629

Merged
merged 34 commits into from
Jan 23, 2019

Conversation

ajsutton
Copy link
Contributor

PR description

When fast sync is enabled, consider the peer count insufficient until we have enough to start fast sync. PeerDiscoveryController will then refresh the peer table more often to find extra peers more quickly.

ajsutton and others added 19 commits January 21, 2019 15:34
… full sync once that completes.

Currently the FastSyncDownloader immediately fails with FAST_SYNC_UNSUPPORTED.
…ad of having to check the return value is success constantly.
… it can return a single header, handle retrying and provide a place to do additional validation.
private List<Capability> supportedCapabilities;
private final Blockchain blockchain;

EthProtocolManager(
final Blockchain blockchain,
final WorldStateArchive worldStateArchive,
final int networkId,
final boolean fastSyncEnabled,
final SynchronizerConfiguration syncConfig,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Originally, I intended EthProtocolManager to be something like a generic service provider - a base layer for handling communication over the network related to the eth subprotocol without holding any process-specific code. So, I tried to keep the sync package dependent on the management layer without the management layer depending on sync-specific code. Not a huge deal, but I try to keep an eye on circular dependencies between packages.

One alternative idea would be to expose something like a PeerRefreshRequests object on EthContext. And the manager could then check and clear that when hasSufficientPeers() is invoked.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, looking at it whether we have enough peers or not is actually a property of Synchronizer not EthProtocolManager so I've moved it there which simplifies a bunch of stuff.

@ajsutton ajsutton merged commit 25f2420 into PegaSysEng:master Jan 23, 2019
@ajsutton ajsutton deleted the sufficient-peers branch January 23, 2019 23:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants