From 143b04d16961a1dd6e45c0dcba366874566935d1 Mon Sep 17 00:00:00 2001 From: "lukasz.rozmej" Date: Wed, 15 Feb 2023 15:32:57 +0100 Subject: [PATCH 1/4] Don't disconnect peers on fast sync criteria --- .../SyncPeerPoolTests.cs | 52 -------- .../Peers/SyncPeerPool.cs | 118 ------------------ 2 files changed, 170 deletions(-) diff --git a/src/Nethermind/Nethermind.Synchronization.Test/SyncPeerPoolTests.cs b/src/Nethermind/Nethermind.Synchronization.Test/SyncPeerPoolTests.cs index 4c107587dc9..f111ce801b7 100644 --- a/src/Nethermind/Nethermind.Synchronization.Test/SyncPeerPoolTests.cs +++ b/src/Nethermind/Nethermind.Synchronization.Test/SyncPeerPoolTests.cs @@ -169,16 +169,6 @@ public async Task Cannot_add_when_not_started() } } - [Test] - public async Task Will_disconnect_one_when_at_max() - { - await using Context ctx = new(); - var peers = await SetupPeers(ctx, 25); - await WaitForPeersInitialization(ctx); - ctx.Pool.DropUselessPeers(true); - Assert.True(peers.Any(p => p.DisconnectRequested)); - } - [TestCase(true, false)] [TestCase(false, true)] public async Task Will_disconnect_when_refresh_exception_is_not_cancelled(bool isExceptionOperationCanceled, bool isDisconnectRequested) @@ -192,48 +182,6 @@ public async Task Will_disconnect_when_refresh_exception_is_not_cancelled(bool i peer.DisconnectRequested.Should().Be(isDisconnectRequested); } - [TestCase(0)] - [TestCase(10)] - [TestCase(24)] - public async Task Will_not_disconnect_any_priority_peer_if_their_amount_is_lower_than_max(byte number) - { - const int peersMaxCount = 25; - const int priorityPeersMaxCount = 25; - await using Context ctx = new(); - ctx.Pool = new SyncPeerPool(ctx.BlockTree, ctx.Stats, ctx.PeerStrategy, peersMaxCount, priorityPeersMaxCount, 50, LimboLogs.Instance); - var peers = await SetupPeers(ctx, peersMaxCount); - - // setting priority to all peers except one - peers[number] - for (int i = 0; i < priorityPeersMaxCount; i++) - { - if (i != number) - { - ctx.Pool.SetPeerPriority(peers[i].Id); - } - } - await WaitForPeersInitialization(ctx); - ctx.Pool.DropUselessPeers(true); - Assert.True(peers[number].DisconnectRequested); - } - - [Test] - public async Task Can_disconnect_priority_peer_if_their_amount_is_max() - { - const int peersMaxCount = 25; - const int priorityPeersMaxCount = 25; - await using Context ctx = new(); - ctx.Pool = new SyncPeerPool(ctx.BlockTree, ctx.Stats, ctx.PeerStrategy, peersMaxCount, priorityPeersMaxCount, 50, LimboLogs.Instance); - var peers = await SetupPeers(ctx, peersMaxCount); - - foreach (SimpleSyncPeerMock peer in peers) - { - ctx.Pool.SetPeerPriority(peer.Id); - } - await WaitForPeersInitialization(ctx); - ctx.Pool.DropUselessPeers(true); - Assert.True(peers.Any(p => p.DisconnectRequested)); - } - [Test] public async Task Should_increment_PriorityPeerCount_when_added_priority_peer_and_decrement_after_removal() { diff --git a/src/Nethermind/Nethermind.Synchronization/Peers/SyncPeerPool.cs b/src/Nethermind/Nethermind.Synchronization/Peers/SyncPeerPool.cs index 9c93ccf46cc..001cec339d6 100644 --- a/src/Nethermind/Nethermind.Synchronization/Peers/SyncPeerPool.cs +++ b/src/Nethermind/Nethermind.Synchronization/Peers/SyncPeerPool.cs @@ -477,123 +477,6 @@ private bool CanBeUsefulForFastBlocks(long blockNumber) lowestInsertedHeader > 1 && lowestInsertedHeader < blockNumber; } - internal void DropUselessPeers(bool force = false) - { - if (!force && DateTime.UtcNow - _lastUselessPeersDropTime < TimeSpan.FromSeconds(30)) - // give some time to monitoring nodes - // (monitoring nodes are nodes that are investigating the network but are not synced themselves) - return; - - if (_logger.IsTrace) _logger.Trace($"Reviewing {PeerCount} peer usefulness"); - - int peersDropped = 0; - _lastUselessPeersDropTime = DateTime.UtcNow; - - long ourNumber = _blockTree.BestSuggestedHeader?.Number ?? 0L; - UInt256 ourDifficulty = _blockTree.BestSuggestedHeader?.TotalDifficulty ?? UInt256.Zero; - foreach (PeerInfo peerInfo in NonStaticPeers) - { - if (peerInfo.HeadNumber == 0 - && peerInfo.IsInitialized - && ourNumber != 0 - && peerInfo.PeerClientType != NodeClientType.Nethermind - && peerInfo.PeerClientType != NodeClientType.Trinity) - // we know that Nethermind reports 0 HeadNumber when it is in sync (and it can still serve a lot of data to other nodes) - { - if (!CanBeUsefulForFastBlocks(peerInfo.HeadNumber)) - { - peersDropped++; - peerInfo.SyncPeer.Disconnect(InitiateDisconnectReason.UselessInFastBlocks, "PEER REVIEW / HEAD 0"); - } - } - else if (peerInfo.HeadNumber == 1920000 && _blockTree.NetworkId == BlockchainIds.Mainnet) // mainnet, stuck Geth nodes - { - if (!CanBeUsefulForFastBlocks(peerInfo.HeadNumber)) - { - peersDropped++; - peerInfo.SyncPeer.Disconnect(InitiateDisconnectReason.UselessInFastBlocks, "PEER REVIEW / 1920000"); - } - } - else if (peerInfo.HeadNumber == 7280022 && _blockTree.NetworkId == BlockchainIds.Mainnet) // mainnet, stuck Geth nodes - { - if (!CanBeUsefulForFastBlocks(peerInfo.HeadNumber)) - { - peersDropped++; - peerInfo.SyncPeer.Disconnect(InitiateDisconnectReason.UselessInFastBlocks, "PEER REVIEW / 7280022"); - } - } - else if (peerInfo.HeadNumber > ourNumber + 1024L && _betterPeerStrategy.IsLowerThanTerminalTotalDifficulty(peerInfo.TotalDifficulty) && peerInfo.TotalDifficulty < ourDifficulty) - { - if (!CanBeUsefulForFastBlocks(MainnetSpecProvider.Instance.DaoBlockNumber ?? 0)) - { - // probably Ethereum Classic nodes tht remain connected after we went pass the DAO - // worth to find a better way to discard them at the right time - peersDropped++; - peerInfo.SyncPeer.Disconnect(InitiateDisconnectReason.UselessInFastBlocks, "STRAY PEER"); - } - } - } - - if (PeerCount == PeerMaxCount) - { - peersDropped += DropWorstPeer(); - } - - if (_logger.IsDebug) _logger.Debug($"Dropped {peersDropped} useless peers"); - } - - private int DropWorstPeer() - { - string? IsPeerWorstWithReason(PeerInfo currentPeer, PeerInfo toCompare) - { - if (toCompare.HeadNumber < currentPeer.HeadNumber) - { - return "LOWEST NUMBER"; - } - - if (toCompare.TotalDifficulty < currentPeer.TotalDifficulty) - { - return "LOWEST DIFFICULTY"; - } - - if ((_stats.GetOrAdd(toCompare.SyncPeer.Node).GetAverageTransferSpeed(TransferSpeedType.Latency) ?? long.MaxValue) > - (_stats.GetOrAdd(currentPeer.SyncPeer.Node).GetAverageTransferSpeed(TransferSpeedType.Latency) ?? long.MaxValue)) - { - return "HIGHEST PING"; - } - - return null; - } - - bool canDropPriorityPeer = PriorityPeerCount >= PriorityPeerMaxCount; - - PeerInfo? worstPeer = null; - string? worstReason = "DEFAULT"; - - foreach (PeerInfo peerInfo in NonStaticPeers) - { - if (peerInfo.SyncPeer.IsPriority && !canDropPriorityPeer) - { - continue; - } - - if (worstPeer is null) - { - worstPeer = peerInfo; - } - - string? peerWorstReason = IsPeerWorstWithReason(worstPeer, peerInfo); - if (peerWorstReason is not null) - { - worstPeer = peerInfo; - worstReason = peerWorstReason; - } - } - - worstPeer?.SyncPeer.Disconnect(InitiateDisconnectReason.DropWorstPeer, $"PEER REVIEW / {worstReason}"); - return 1; - } - public void SignalPeersChanged() { if (!_signals.SafeWaitHandle.IsClosed) @@ -674,7 +557,6 @@ await firstToComplete.ContinueWith( /// Thrown if an irreplaceable allocation is being replaced by this method (internal implementation error). private void UpgradeAllocations() { - DropUselessPeers(); WakeUpPeerThatSleptEnough(); foreach ((SyncPeerAllocation allocation, _) in _replaceableAllocations) { From 1c33fe5a28b16e0be11f1519d8adb029557ecaf2 Mon Sep 17 00:00:00 2001 From: "lukasz.rozmej" Date: Wed, 15 Feb 2023 15:35:17 +0100 Subject: [PATCH 2/4] Remove obsolete flag --- .../Nethermind.Network.Stats/Model/InitiateDisconnectReason.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Nethermind/Nethermind.Network.Stats/Model/InitiateDisconnectReason.cs b/src/Nethermind/Nethermind.Network.Stats/Model/InitiateDisconnectReason.cs index 57c8b431fb9..a8b2d5b6a4c 100644 --- a/src/Nethermind/Nethermind.Network.Stats/Model/InitiateDisconnectReason.cs +++ b/src/Nethermind/Nethermind.Network.Stats/Model/InitiateDisconnectReason.cs @@ -21,7 +21,6 @@ public enum InitiateDisconnectReason : byte TxFlooding, NoCapabilityMatched, - UselessInFastBlocks, DropWorstPeer, PeerRefreshFailed, @@ -72,8 +71,6 @@ public static DisconnectReason ToDisconnectReason(this InitiateDisconnectReason case InitiateDisconnectReason.NoCapabilityMatched: return DisconnectReason.UselessPeer; - case InitiateDisconnectReason.UselessInFastBlocks: - return DisconnectReason.UselessPeer; case InitiateDisconnectReason.DropWorstPeer: return DisconnectReason.TooManyPeers; case InitiateDisconnectReason.PeerRefreshFailed: From eb35da6d6a3ee220fdbca886738201152a972146 Mon Sep 17 00:00:00 2001 From: "lukasz.rozmej" Date: Wed, 15 Feb 2023 15:43:25 +0100 Subject: [PATCH 3/4] remove unused method --- .../Nethermind.Synchronization/Peers/SyncPeerPool.cs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/Nethermind/Nethermind.Synchronization/Peers/SyncPeerPool.cs b/src/Nethermind/Nethermind.Synchronization/Peers/SyncPeerPool.cs index 001cec339d6..ed968b5159f 100644 --- a/src/Nethermind/Nethermind.Synchronization/Peers/SyncPeerPool.cs +++ b/src/Nethermind/Nethermind.Synchronization/Peers/SyncPeerPool.cs @@ -469,14 +469,6 @@ private void StartUpgradeTimer() upgradeTimer.Start(); } - private bool CanBeUsefulForFastBlocks(long blockNumber) - { - long lowestInsertedBody = _blockTree.LowestInsertedBodyNumber ?? long.MaxValue; - long lowestInsertedHeader = _blockTree.LowestInsertedHeader?.Number ?? long.MaxValue; - return lowestInsertedBody > 1 && lowestInsertedBody < blockNumber || - lowestInsertedHeader > 1 && lowestInsertedHeader < blockNumber; - } - public void SignalPeersChanged() { if (!_signals.SafeWaitHandle.IsClosed) From 48ea9940d34c8a3dded3c57b04f9eca40d245f5a Mon Sep 17 00:00:00 2001 From: "lukasz.rozmej" Date: Wed, 8 Mar 2023 14:54:36 +0100 Subject: [PATCH 4/4] Keep DropWorstPeer --- .../Steps/InitializeNetwork.cs | 2 +- .../StateSyncDispatcherTests.cs | 2 +- .../FastSync/StateSyncFeedTestsBase.cs | 2 +- .../OldStyleFullSynchronizerTests.cs | 2 +- .../SyncPeerPoolTests.cs | 58 ++++++++++- .../SyncPeersReportTests.cs | 8 +- .../SyncThreadTests.cs | 2 +- .../SynchronizerTests.cs | 19 +--- .../Peers/SyncPeerPool.cs | 96 ++++++++++++++----- 9 files changed, 141 insertions(+), 50 deletions(-) diff --git a/src/Nethermind/Nethermind.Init/Steps/InitializeNetwork.cs b/src/Nethermind/Nethermind.Init/Steps/InitializeNetwork.cs index 094b03c00bf..37704aedc3e 100644 --- a/src/Nethermind/Nethermind.Init/Steps/InitializeNetwork.cs +++ b/src/Nethermind/Nethermind.Init/Steps/InitializeNetwork.cs @@ -117,7 +117,7 @@ private async Task Initialize(CancellationToken cancellationToken) int maxPeersCount = _networkConfig.ActivePeersMaxCount; int maxPriorityPeersCount = _networkConfig.PriorityPeersMaxCount; Network.Metrics.PeerLimit = maxPeersCount; - SyncPeerPool apiSyncPeerPool = new(_api.BlockTree!, _api.NodeStatsManager!, _api.BetterPeerStrategy, maxPeersCount, maxPriorityPeersCount, SyncPeerPool.DefaultUpgradeIntervalInMs, _api.LogManager); + SyncPeerPool apiSyncPeerPool = new(_api.BlockTree!, _api.NodeStatsManager!, _api.BetterPeerStrategy, _api.LogManager, maxPeersCount, maxPriorityPeersCount); _api.SyncPeerPool = apiSyncPeerPool; _api.PeerDifficultyRefreshPool = apiSyncPeerPool; diff --git a/src/Nethermind/Nethermind.Synchronization.Test/FastSync/SnapProtocolTests/StateSyncDispatcherTests.cs b/src/Nethermind/Nethermind.Synchronization.Test/FastSync/SnapProtocolTests/StateSyncDispatcherTests.cs index a986a457d3c..ebc28835808 100644 --- a/src/Nethermind/Nethermind.Synchronization.Test/FastSync/SnapProtocolTests/StateSyncDispatcherTests.cs +++ b/src/Nethermind/Nethermind.Synchronization.Test/FastSync/SnapProtocolTests/StateSyncDispatcherTests.cs @@ -43,7 +43,7 @@ public void Setup() BlockTree blockTree = Build.A.BlockTree().OfChainLength((int)BlockTree.BestSuggestedHeader!.Number).TestObject; ITimerFactory timerFactory = Substitute.For(); - _pool = new SyncPeerPool(blockTree, new NodeStatsManager(timerFactory, LimboLogs.Instance), new TotalDifficultyBetterPeerStrategy(LimboLogs.Instance), 25, LimboLogs.Instance); + _pool = new SyncPeerPool(blockTree, new NodeStatsManager(timerFactory, LimboLogs.Instance), new TotalDifficultyBetterPeerStrategy(LimboLogs.Instance), LimboLogs.Instance, 25); _pool.Start(); var feed = Substitute.For>(); diff --git a/src/Nethermind/Nethermind.Synchronization.Test/FastSync/StateSyncFeedTestsBase.cs b/src/Nethermind/Nethermind.Synchronization.Test/FastSync/StateSyncFeedTestsBase.cs index 3b762736876..bd165d52136 100644 --- a/src/Nethermind/Nethermind.Synchronization.Test/FastSync/StateSyncFeedTestsBase.cs +++ b/src/Nethermind/Nethermind.Synchronization.Test/FastSync/StateSyncFeedTestsBase.cs @@ -97,7 +97,7 @@ protected SafeContext PrepareDownloaderWithPeer(DbContext dbContext, params ISyn ctx = new SafeContext(); BlockTree blockTree = Build.A.BlockTree().OfChainLength((int)BlockTree.BestSuggestedHeader.Number).TestObject; ITimerFactory timerFactory = Substitute.For(); - ctx.Pool = new SyncPeerPool(blockTree, new NodeStatsManager(timerFactory, LimboLogs.Instance), new TotalDifficultyBetterPeerStrategy(LimboLogs.Instance), 25, LimboLogs.Instance); + ctx.Pool = new SyncPeerPool(blockTree, new NodeStatsManager(timerFactory, LimboLogs.Instance), new TotalDifficultyBetterPeerStrategy(LimboLogs.Instance), LimboLogs.Instance, 25); ctx.Pool.Start(); for (int i = 0; i < syncPeers.Length; i++) diff --git a/src/Nethermind/Nethermind.Synchronization.Test/OldStyleFullSynchronizerTests.cs b/src/Nethermind/Nethermind.Synchronization.Test/OldStyleFullSynchronizerTests.cs index 206df0a7a40..933533886c2 100644 --- a/src/Nethermind/Nethermind.Synchronization.Test/OldStyleFullSynchronizerTests.cs +++ b/src/Nethermind/Nethermind.Synchronization.Test/OldStyleFullSynchronizerTests.cs @@ -51,7 +51,7 @@ public async Task Setup() ITimerFactory timerFactory = Substitute.For(); NodeStatsManager stats = new(timerFactory, LimboLogs.Instance); - _pool = new SyncPeerPool(_blockTree, stats, new TotalDifficultyBetterPeerStrategy(LimboLogs.Instance), 25, LimboLogs.Instance); + _pool = new SyncPeerPool(_blockTree, stats, new TotalDifficultyBetterPeerStrategy(LimboLogs.Instance), LimboLogs.Instance, 25); SyncConfig syncConfig = new(); ProgressTracker progressTracker = new(_blockTree, dbProvider.StateDb, LimboLogs.Instance); SnapProvider snapProvider = new(progressTracker, dbProvider, LimboLogs.Instance); diff --git a/src/Nethermind/Nethermind.Synchronization.Test/SyncPeerPoolTests.cs b/src/Nethermind/Nethermind.Synchronization.Test/SyncPeerPoolTests.cs index f111ce801b7..8a6895c5704 100644 --- a/src/Nethermind/Nethermind.Synchronization.Test/SyncPeerPoolTests.cs +++ b/src/Nethermind/Nethermind.Synchronization.Test/SyncPeerPoolTests.cs @@ -40,7 +40,7 @@ public Context() BlockTree = Substitute.For(); Stats = Substitute.For(); PeerStrategy = new TotalDifficultyBetterPeerStrategy(LimboLogs.Instance); - Pool = new SyncPeerPool(BlockTree, Stats, PeerStrategy, 25, 50, LimboLogs.Instance); + Pool = new SyncPeerPool(BlockTree, Stats, PeerStrategy, LimboLogs.Instance, 25, 50); } public async ValueTask DisposeAsync() @@ -169,6 +169,16 @@ public async Task Cannot_add_when_not_started() } } + [Test] + public async Task Will_disconnect_one_when_at_max() + { + await using Context ctx = new(); + var peers = await SetupPeers(ctx, 25); + await WaitForPeersInitialization(ctx); + ctx.Pool.DropUselessPeers(true); + Assert.True(peers.Any(p => p.DisconnectRequested)); + } + [TestCase(true, false)] [TestCase(false, true)] public async Task Will_disconnect_when_refresh_exception_is_not_cancelled(bool isExceptionOperationCanceled, bool isDisconnectRequested) @@ -182,13 +192,55 @@ public async Task Will_disconnect_when_refresh_exception_is_not_cancelled(bool i peer.DisconnectRequested.Should().Be(isDisconnectRequested); } + [TestCase(0)] + [TestCase(10)] + [TestCase(24)] + public async Task Will_not_disconnect_any_priority_peer_if_their_amount_is_lower_than_max(byte number) + { + const int peersMaxCount = 25; + const int priorityPeersMaxCount = 25; + await using Context ctx = new(); + ctx.Pool = new SyncPeerPool(ctx.BlockTree, ctx.Stats, ctx.PeerStrategy, LimboLogs.Instance, peersMaxCount, priorityPeersMaxCount, 50); + var peers = await SetupPeers(ctx, peersMaxCount); + + // setting priority to all peers except one - peers[number] + for (int i = 0; i < priorityPeersMaxCount; i++) + { + if (i != number) + { + ctx.Pool.SetPeerPriority(peers[i].Id); + } + } + await WaitForPeersInitialization(ctx); + ctx.Pool.DropUselessPeers(true); + Assert.True(peers[number].DisconnectRequested); + } + + [Test] + public async Task Can_disconnect_priority_peer_if_their_amount_is_max() + { + const int peersMaxCount = 25; + const int priorityPeersMaxCount = 25; + await using Context ctx = new(); + ctx.Pool = new SyncPeerPool(ctx.BlockTree, ctx.Stats, ctx.PeerStrategy, LimboLogs.Instance, peersMaxCount, priorityPeersMaxCount, 50); + var peers = await SetupPeers(ctx, peersMaxCount); + + foreach (SimpleSyncPeerMock peer in peers) + { + ctx.Pool.SetPeerPriority(peer.Id); + } + await WaitForPeersInitialization(ctx); + ctx.Pool.DropUselessPeers(true); + Assert.True(peers.Any(p => p.DisconnectRequested)); + } + [Test] public async Task Should_increment_PriorityPeerCount_when_added_priority_peer_and_decrement_after_removal() { const int peersMaxCount = 1; const int priorityPeersMaxCount = 1; await using Context ctx = new(); - ctx.Pool = new SyncPeerPool(ctx.BlockTree, ctx.Stats, ctx.PeerStrategy, peersMaxCount, priorityPeersMaxCount, 50, LimboLogs.Instance); + ctx.Pool = new SyncPeerPool(ctx.BlockTree, ctx.Stats, ctx.PeerStrategy, LimboLogs.Instance, peersMaxCount, priorityPeersMaxCount, 50); SimpleSyncPeerMock peer = new(TestItem.PublicKeyA) { IsPriority = true }; ctx.Pool.Start(); @@ -206,7 +258,7 @@ public async Task Should_increment_PriorityPeerCount_when_called_SetPriorityPeer const int peersMaxCount = 1; const int priorityPeersMaxCount = 1; await using Context ctx = new(); - ctx.Pool = new SyncPeerPool(ctx.BlockTree, ctx.Stats, ctx.PeerStrategy, peersMaxCount, priorityPeersMaxCount, 50, LimboLogs.Instance); + ctx.Pool = new SyncPeerPool(ctx.BlockTree, ctx.Stats, ctx.PeerStrategy, LimboLogs.Instance, peersMaxCount, priorityPeersMaxCount, 50); SimpleSyncPeerMock peer = new(TestItem.PublicKeyA) { IsPriority = false }; ctx.Pool.Start(); diff --git a/src/Nethermind/Nethermind.Synchronization.Test/SyncPeersReportTests.cs b/src/Nethermind/Nethermind.Synchronization.Test/SyncPeersReportTests.cs index 1447638b280..60083511519 100644 --- a/src/Nethermind/Nethermind.Synchronization.Test/SyncPeersReportTests.cs +++ b/src/Nethermind/Nethermind.Synchronization.Test/SyncPeersReportTests.cs @@ -161,10 +161,10 @@ public void PeerFormatIsCorrect() syncPeerPool.AllPeers.Returns(peers); string expectedResult = - "== Header ==\n" + - "===[Active][Sleep ][Peer(ProtocolVersion/Head/Host:Port/Direction)][Transfer Speeds (L/H/B/R/N/S) ][Client Info (Name/Version/Operating System/Language) ]\n" + - "--------------------------------------------------------------------------------------------------------------------------------------------------------------\n" + - " [HBRNSW][ ][Peer|eth99| 9999| 127.0.0.1: 3030| Out][ | | | | | ][]\n" + + "== Header ==" + Environment.NewLine + + "===[Active][Sleep ][Peer(ProtocolVersion/Head/Host:Port/Direction)][Transfer Speeds (L/H/B/R/N/S) ][Client Info (Name/Version/Operating System/Language) ]" + Environment.NewLine + + "--------------------------------------------------------------------------------------------------------------------------------------------------------------" + Environment.NewLine + + " [HBRNSW][ ][Peer|eth99| 9999| 127.0.0.1: 3030| Out][ | | | | | ][]" + Environment.NewLine + " [ ][HBRNSW][Peer|eth99| 9999| 127.0.0.1: 3030| In][ | | | | | ][]"; SyncPeersReport report = new(syncPeerPool, Substitute.For(), NoErrorLimboLogs.Instance); diff --git a/src/Nethermind/Nethermind.Synchronization.Test/SyncThreadTests.cs b/src/Nethermind/Nethermind.Synchronization.Test/SyncThreadTests.cs index 1bea43b3505..bb9bc3d5a65 100644 --- a/src/Nethermind/Nethermind.Synchronization.Test/SyncThreadTests.cs +++ b/src/Nethermind/Nethermind.Synchronization.Test/SyncThreadTests.cs @@ -313,7 +313,7 @@ private SyncTestContext CreateSyncManager(int index) ITimerFactory timerFactory = Substitute.For(); NodeStatsManager nodeStatsManager = new(timerFactory, logManager); - SyncPeerPool syncPeerPool = new(tree, nodeStatsManager, new TotalDifficultyBetterPeerStrategy(LimboLogs.Instance), 25, logManager); + SyncPeerPool syncPeerPool = new(tree, nodeStatsManager, new TotalDifficultyBetterPeerStrategy(LimboLogs.Instance), logManager, 25); StateProvider devState = new(trieStore, codeDb, logManager); StorageProvider devStorage = new(trieStore, devState, logManager); diff --git a/src/Nethermind/Nethermind.Synchronization.Test/SynchronizerTests.cs b/src/Nethermind/Nethermind.Synchronization.Test/SynchronizerTests.cs index db2daf17a29..48b68b64597 100644 --- a/src/Nethermind/Nethermind.Synchronization.Test/SynchronizerTests.cs +++ b/src/Nethermind/Nethermind.Synchronization.Test/SynchronizerTests.cs @@ -331,24 +331,13 @@ ISyncConfig GetSyncConfig() => syncConfig, _logManager); - if (IsMerge(synchronizerType)) - SyncPeerPool = new SyncPeerPool(BlockTree, stats, - new MergeBetterPeerStrategy( - new TotalDifficultyBetterPeerStrategy(LimboLogs.Instance), poSSwitcher, beaconPivot, LimboLogs.Instance), 25, _logManager); - else - SyncPeerPool = new SyncPeerPool(BlockTree, stats, - new TotalDifficultyBetterPeerStrategy(LimboLogs.Instance), 25, - _logManager); - TotalDifficultyBetterPeerStrategy totalDifficultyBetterPeerStrategy = new(LimboLogs.Instance); - IBetterPeerStrategy bestPeerStrategy; - bestPeerStrategy = IsMerge(synchronizerType) - ? new MergeBetterPeerStrategy(totalDifficultyBetterPeerStrategy, - poSSwitcher, beaconPivot, LimboLogs.Instance) + IBetterPeerStrategy bestPeerStrategy = IsMerge(synchronizerType) + ? new MergeBetterPeerStrategy(totalDifficultyBetterPeerStrategy, poSSwitcher, beaconPivot, LimboLogs.Instance) : totalDifficultyBetterPeerStrategy; - MultiSyncModeSelector syncModeSelector = new(syncProgressResolver, SyncPeerPool, - syncConfig, No.BeaconSync, bestPeerStrategy, _logManager); + SyncPeerPool = new SyncPeerPool(BlockTree, stats, bestPeerStrategy, _logManager, 25); + MultiSyncModeSelector syncModeSelector = new(syncProgressResolver, SyncPeerPool, syncConfig, No.BeaconSync, bestPeerStrategy, _logManager); Pivot pivot = new(syncConfig); IInvalidChainTracker invalidChainTracker = new NoopInvalidChainTracker(); diff --git a/src/Nethermind/Nethermind.Synchronization/Peers/SyncPeerPool.cs b/src/Nethermind/Nethermind.Synchronization/Peers/SyncPeerPool.cs index ed968b5159f..2bd8cc43f37 100644 --- a/src/Nethermind/Nethermind.Synchronization/Peers/SyncPeerPool.cs +++ b/src/Nethermind/Nethermind.Synchronization/Peers/SyncPeerPool.cs @@ -21,6 +21,7 @@ using Nethermind.Specs; using Nethermind.Stats; using Nethermind.Stats.Model; +using Nethermind.Synchronization.ParallelSync; using Nethermind.Synchronization.Peers.AllocationStrategies; using Timer = System.Timers.Timer; @@ -64,29 +65,10 @@ public class SyncPeerPool : ISyncPeerPool, IPeerDifficultyRefreshPool public SyncPeerPool(IBlockTree blockTree, INodeStatsManager nodeStatsManager, IBetterPeerStrategy betterPeerStrategy, - int peersMaxCount, - ILogManager logManager) - : this(blockTree, nodeStatsManager, betterPeerStrategy, peersMaxCount, DefaultUpgradeIntervalInMs, logManager) - { - } - - public SyncPeerPool(IBlockTree blockTree, - INodeStatsManager nodeStatsManager, - IBetterPeerStrategy betterPeerStrategy, - int peersMaxCount, - int allocationsUpgradeIntervalInMs, - ILogManager logManager) - : this(blockTree, nodeStatsManager, betterPeerStrategy, peersMaxCount, 0, allocationsUpgradeIntervalInMs, logManager) - { - } - - public SyncPeerPool(IBlockTree blockTree, - INodeStatsManager nodeStatsManager, - IBetterPeerStrategy betterPeerStrategy, - int peersMaxCount, - int priorityPeerMaxCount, - int allocationsUpgradeIntervalInMsInMs, - ILogManager logManager) + ILogManager logManager, + int peersMaxCount = 100, + int priorityPeerMaxCount = 0, + int allocationsUpgradeIntervalInMsInMs = DefaultUpgradeIntervalInMs) { _blockTree = blockTree ?? throw new ArgumentNullException(nameof(blockTree)); _stats = nodeStatsManager ?? throw new ArgumentNullException(nameof(nodeStatsManager)); @@ -469,6 +451,73 @@ private void StartUpgradeTimer() upgradeTimer.Start(); } + internal void DropUselessPeers(bool force = false) + { + if (!force && DateTime.UtcNow - _lastUselessPeersDropTime < TimeSpan.FromSeconds(30)) + return; + + if (_logger.IsTrace) _logger.Trace($"Reviewing {PeerCount} peer usefulness"); + + int peersDropped = 0; + _lastUselessPeersDropTime = DateTime.UtcNow; + + if (PeerCount == PeerMaxCount) + { + peersDropped += DropWorstPeer(); + } + + if (_logger.IsDebug) _logger.Debug($"Dropped {peersDropped} useless peers"); + } + + private int DropWorstPeer() + { + string? IsPeerWorstWithReason(PeerInfo currentPeer, PeerInfo toCompare) + { + if (toCompare.HeadNumber < currentPeer.HeadNumber) + { + return "LOWEST NUMBER"; + } + + if (toCompare.TotalDifficulty < currentPeer.TotalDifficulty) + { + return "LOWEST DIFFICULTY"; + } + + if ((_stats.GetOrAdd(toCompare.SyncPeer.Node).GetAverageTransferSpeed(TransferSpeedType.Latency) ?? long.MaxValue) > + (_stats.GetOrAdd(currentPeer.SyncPeer.Node).GetAverageTransferSpeed(TransferSpeedType.Latency) ?? long.MaxValue)) + { + return "HIGHEST PING"; + } + + return null; + } + + bool canDropPriorityPeer = PriorityPeerCount >= PriorityPeerMaxCount; + + PeerInfo? worstPeer = null; + string? worstReason = "DEFAULT"; + + foreach (PeerInfo peerInfo in NonStaticPeers) + { + if (peerInfo.SyncPeer.IsPriority && !canDropPriorityPeer) + { + continue; + } + + worstPeer ??= peerInfo; + + string? peerWorstReason = IsPeerWorstWithReason(worstPeer, peerInfo); + if (peerWorstReason is not null) + { + worstPeer = peerInfo; + worstReason = peerWorstReason; + } + } + + worstPeer?.SyncPeer.Disconnect(InitiateDisconnectReason.DropWorstPeer, $"PEER REVIEW / {worstReason}"); + return 1; + } + public void SignalPeersChanged() { if (!_signals.SafeWaitHandle.IsClosed) @@ -549,6 +598,7 @@ await firstToComplete.ContinueWith( /// Thrown if an irreplaceable allocation is being replaced by this method (internal implementation error). private void UpgradeAllocations() { + DropUselessPeers(); WakeUpPeerThatSleptEnough(); foreach ((SyncPeerAllocation allocation, _) in _replaceableAllocations) {