From 8b760858fe0eab09f703ba919ebfa4ffe84765b9 Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Thu, 25 May 2023 15:37:55 +0200 Subject: [PATCH 01/23] Use AbstractRetryingSwitchingPeerTask whenever it makes sense Signed-off-by: Fabio Di Fabio --- .../RetryingGetAccountRangeFromPeerTask.java | 24 ++++++++------ .../snap/RetryingGetBytecodeFromPeerTask.java | 22 +++++++------ .../RetryingGetStorageRangeFromPeerTask.java | 31 +++++++++++++------ .../snap/RetryingGetTrieNodeFromPeerTask.java | 22 +++++++------ .../task/RetryingGetNodeDataFromPeerTask.java | 20 ++++++------ .../ethereum/eth/sync/DownloadBodiesStep.java | 7 ++++- .../fastsync/worldstate/RequestDataStep.java | 6 +++- .../eth/sync/snapsync/RequestDataStep.java | 19 +++++++++--- .../RetryingGetNodeDataFromPeerTaskTest.java | 2 +- 9 files changed, 97 insertions(+), 56 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java index 7512bdd03a9..f6ff473c561 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java @@ -17,18 +17,17 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; -import org.hyperledger.besu.ethereum.eth.manager.task.AbstractRetryingPeerTask; +import org.hyperledger.besu.ethereum.eth.manager.task.AbstractRetryingSwitchingPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.EthTask; import org.hyperledger.besu.ethereum.eth.messages.snap.AccountRangeMessage; import org.hyperledger.besu.plugin.services.MetricsSystem; -import java.util.Optional; import java.util.concurrent.CompletableFuture; import org.apache.tuweni.bytes.Bytes32; public class RetryingGetAccountRangeFromPeerTask - extends AbstractRetryingPeerTask { + extends AbstractRetryingSwitchingPeerTask { private final EthContext ethContext; private final Bytes32 startKeyHash; @@ -41,9 +40,13 @@ private RetryingGetAccountRangeFromPeerTask( final Bytes32 startKeyHash, final Bytes32 endKeyHash, final BlockHeader blockHeader, - final MetricsSystem metricsSystem) { + final MetricsSystem metricsSystem, + final int maxRetries) { super( - ethContext, 4, data -> data.accounts().isEmpty() && data.proofs().isEmpty(), metricsSystem); + ethContext, + metricsSystem, + data -> data.accounts().isEmpty() && data.proofs().isEmpty(), + maxRetries); this.ethContext = ethContext; this.startKeyHash = startKeyHash; this.endKeyHash = endKeyHash; @@ -56,18 +59,19 @@ public static EthTask forAccountRange( final Bytes32 startKeyHash, final Bytes32 endKeyHash, final BlockHeader blockHeader, - final MetricsSystem metricsSystem) { + final MetricsSystem metricsSystem, + final int maxRetries) { return new RetryingGetAccountRangeFromPeerTask( - ethContext, startKeyHash, endKeyHash, blockHeader, metricsSystem); + ethContext, startKeyHash, endKeyHash, blockHeader, metricsSystem, maxRetries); } @Override - protected CompletableFuture executePeerTask( - final Optional assignedPeer) { + protected CompletableFuture executeTaskOnCurrentPeer( + final EthPeer assignedPeer) { final GetAccountRangeFromPeerTask task = GetAccountRangeFromPeerTask.forAccountRange( ethContext, startKeyHash, endKeyHash, blockHeader, metricsSystem); - assignedPeer.ifPresent(task::assignPeer); + task.assignPeer(assignedPeer); return executeSubTask(task::run) .thenApply( peerResult -> { diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetBytecodeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetBytecodeFromPeerTask.java index 7d23ec944d3..4b55c53b5ba 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetBytecodeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetBytecodeFromPeerTask.java @@ -17,19 +17,19 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; -import org.hyperledger.besu.ethereum.eth.manager.task.AbstractRetryingPeerTask; +import org.hyperledger.besu.ethereum.eth.manager.task.AbstractRetryingSwitchingPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.EthTask; import org.hyperledger.besu.plugin.services.MetricsSystem; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.concurrent.CompletableFuture; import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.bytes.Bytes32; -public class RetryingGetBytecodeFromPeerTask extends AbstractRetryingPeerTask> { +public class RetryingGetBytecodeFromPeerTask + extends AbstractRetryingSwitchingPeerTask> { private final EthContext ethContext; private final List codeHashes; @@ -40,8 +40,9 @@ private RetryingGetBytecodeFromPeerTask( final EthContext ethContext, final List codeHashes, final BlockHeader blockHeader, - final MetricsSystem metricsSystem) { - super(ethContext, 4, Map::isEmpty, metricsSystem); + final MetricsSystem metricsSystem, + final int maxRetries) { + super(ethContext, metricsSystem, Map::isEmpty, maxRetries); this.ethContext = ethContext; this.codeHashes = codeHashes; this.blockHeader = blockHeader; @@ -52,16 +53,17 @@ public static EthTask> forByteCode( final EthContext ethContext, final List codeHashes, final BlockHeader blockHeader, - final MetricsSystem metricsSystem) { - return new RetryingGetBytecodeFromPeerTask(ethContext, codeHashes, blockHeader, metricsSystem); + final MetricsSystem metricsSystem, + final int maxRetries) { + return new RetryingGetBytecodeFromPeerTask( + ethContext, codeHashes, blockHeader, metricsSystem, maxRetries); } @Override - protected CompletableFuture> executePeerTask( - final Optional assignedPeer) { + protected CompletableFuture> executeTaskOnCurrentPeer(final EthPeer peer) { final GetBytecodeFromPeerTask task = GetBytecodeFromPeerTask.forBytecode(ethContext, codeHashes, blockHeader, metricsSystem); - assignedPeer.ifPresent(task::assignPeer); + task.assignPeer(peer); return executeSubTask(task::run) .thenApply( peerResult -> { diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetStorageRangeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetStorageRangeFromPeerTask.java index 7a095de9292..4ef09a81de5 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetStorageRangeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetStorageRangeFromPeerTask.java @@ -17,19 +17,18 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; -import org.hyperledger.besu.ethereum.eth.manager.task.AbstractRetryingPeerTask; +import org.hyperledger.besu.ethereum.eth.manager.task.AbstractRetryingSwitchingPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.EthTask; import org.hyperledger.besu.ethereum.eth.messages.snap.StorageRangeMessage; import org.hyperledger.besu.plugin.services.MetricsSystem; import java.util.List; -import java.util.Optional; import java.util.concurrent.CompletableFuture; import org.apache.tuweni.bytes.Bytes32; public class RetryingGetStorageRangeFromPeerTask - extends AbstractRetryingPeerTask { + extends AbstractRetryingSwitchingPeerTask { private final EthContext ethContext; private final List accountHashes; @@ -44,8 +43,13 @@ private RetryingGetStorageRangeFromPeerTask( final Bytes32 startKeyHash, final Bytes32 endKeyHash, final BlockHeader blockHeader, - final MetricsSystem metricsSystem) { - super(ethContext, 4, data -> data.proofs().isEmpty() && data.slots().isEmpty(), metricsSystem); + final MetricsSystem metricsSystem, + final int maxRetries) { + super( + ethContext, + metricsSystem, + data -> data.proofs().isEmpty() && data.slots().isEmpty(), + maxRetries); this.ethContext = ethContext; this.accountHashes = accountHashes; this.startKeyHash = startKeyHash; @@ -60,18 +64,25 @@ public static EthTask forStorageRange( final Bytes32 startKeyHash, final Bytes32 endKeyHash, final BlockHeader blockHeader, - final MetricsSystem metricsSystem) { + final MetricsSystem metricsSystem, + final int maxRetries) { return new RetryingGetStorageRangeFromPeerTask( - ethContext, accountHashes, startKeyHash, endKeyHash, blockHeader, metricsSystem); + ethContext, + accountHashes, + startKeyHash, + endKeyHash, + blockHeader, + metricsSystem, + maxRetries); } @Override - protected CompletableFuture executePeerTask( - final Optional assignedPeer) { + protected CompletableFuture executeTaskOnCurrentPeer( + final EthPeer peer) { final GetStorageRangeFromPeerTask task = GetStorageRangeFromPeerTask.forStorageRange( ethContext, accountHashes, startKeyHash, endKeyHash, blockHeader, metricsSystem); - assignedPeer.ifPresent(task::assignPeer); + task.assignPeer(peer); return executeSubTask(task::run) .thenApply( peerResult -> { diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java index ebfd8856898..115bd28836c 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java @@ -17,18 +17,18 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; -import org.hyperledger.besu.ethereum.eth.manager.task.AbstractRetryingPeerTask; +import org.hyperledger.besu.ethereum.eth.manager.task.AbstractRetryingSwitchingPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.EthTask; import org.hyperledger.besu.plugin.services.MetricsSystem; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.concurrent.CompletableFuture; import org.apache.tuweni.bytes.Bytes; -public class RetryingGetTrieNodeFromPeerTask extends AbstractRetryingPeerTask> { +public class RetryingGetTrieNodeFromPeerTask + extends AbstractRetryingSwitchingPeerTask> { private final EthContext ethContext; private final Map> paths; @@ -39,8 +39,9 @@ private RetryingGetTrieNodeFromPeerTask( final EthContext ethContext, final Map> paths, final BlockHeader blockHeader, - final MetricsSystem metricsSystem) { - super(ethContext, 4, Map::isEmpty, metricsSystem); + final MetricsSystem metricsSystem, + final int maxRetries) { + super(ethContext, metricsSystem, Map::isEmpty, maxRetries); this.ethContext = ethContext; this.paths = paths; this.blockHeader = blockHeader; @@ -51,16 +52,17 @@ public static EthTask> forTrieNodes( final EthContext ethContext, final Map> paths, final BlockHeader blockHeader, - final MetricsSystem metricsSystem) { - return new RetryingGetTrieNodeFromPeerTask(ethContext, paths, blockHeader, metricsSystem); + final MetricsSystem metricsSystem, + final int maxRetries) { + return new RetryingGetTrieNodeFromPeerTask( + ethContext, paths, blockHeader, metricsSystem, maxRetries); } @Override - protected CompletableFuture> executePeerTask( - final Optional assignedPeer) { + protected CompletableFuture> executeTaskOnCurrentPeer(final EthPeer peer) { final GetTrieNodeFromPeerTask task = GetTrieNodeFromPeerTask.forTrieNodes(ethContext, paths, blockHeader, metricsSystem); - assignedPeer.ifPresent(task::assignPeer); + task.assignPeer(peer); return executeSubTask(task::run) .thenApply( peerResult -> { diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetNodeDataFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetNodeDataFromPeerTask.java index 16040e963ae..2bf3db7b72a 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetNodeDataFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetNodeDataFromPeerTask.java @@ -22,13 +22,13 @@ import java.util.Collection; import java.util.HashSet; import java.util.Map; -import java.util.Optional; import java.util.Set; import java.util.concurrent.CompletableFuture; import org.apache.tuweni.bytes.Bytes; -public class RetryingGetNodeDataFromPeerTask extends AbstractRetryingPeerTask> { +public class RetryingGetNodeDataFromPeerTask + extends AbstractRetryingSwitchingPeerTask> { private final EthContext ethContext; private final Set hashes; @@ -39,8 +39,9 @@ private RetryingGetNodeDataFromPeerTask( final EthContext ethContext, final Collection hashes, final long pivotBlockNumber, - final MetricsSystem metricsSystem) { - super(ethContext, 4, data -> false, metricsSystem); + final MetricsSystem metricsSystem, + final int maxRetries) { + super(ethContext, metricsSystem, data -> false, maxRetries); this.ethContext = ethContext; this.hashes = new HashSet<>(hashes); this.pivotBlockNumber = pivotBlockNumber; @@ -51,16 +52,17 @@ public static RetryingGetNodeDataFromPeerTask forHashes( final EthContext ethContext, final Collection hashes, final long pivotBlockNumber, - final MetricsSystem metricsSystem) { - return new RetryingGetNodeDataFromPeerTask(ethContext, hashes, pivotBlockNumber, metricsSystem); + final MetricsSystem metricsSystem, + final int maxRetries) { + return new RetryingGetNodeDataFromPeerTask( + ethContext, hashes, pivotBlockNumber, metricsSystem, maxRetries); } @Override - protected CompletableFuture> executePeerTask( - final Optional assignedPeer) { + protected CompletableFuture> executeTaskOnCurrentPeer(final EthPeer peer) { final GetNodeDataFromPeerTask task = GetNodeDataFromPeerTask.forHashes(ethContext, hashes, pivotBlockNumber, metricsSystem); - assignedPeer.ifPresent(task::assignPeer); + task.assignPeer(peer); return executeSubTask(task::run) .thenApply( peerResult -> { diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DownloadBodiesStep.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DownloadBodiesStep.java index 26fc7b544ce..c3a946f46ef 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DownloadBodiesStep.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DownloadBodiesStep.java @@ -43,7 +43,12 @@ public DownloadBodiesStep( @Override public CompletableFuture> apply(final List blockHeaders) { - return CompleteBlocksTask.forHeaders(protocolSchedule, ethContext, blockHeaders, metricsSystem) + return CompleteBlocksTask.forHeaders( + protocolSchedule, + ethContext, + blockHeaders, + ethContext.getEthPeers().peerCount(), + metricsSystem) .run(); } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/worldstate/RequestDataStep.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/worldstate/RequestDataStep.java index e4325b967f5..31815da1c43 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/worldstate/RequestDataStep.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/worldstate/RequestDataStep.java @@ -46,7 +46,11 @@ public RequestDataStep(final EthContext ethContext, final MetricsSystem metricsS this( (hashes, pivotBlockNumber) -> RetryingGetNodeDataFromPeerTask.forHashes( - ethContext, hashes, pivotBlockNumber, metricsSystem)); + ethContext, + hashes, + pivotBlockNumber, + metricsSystem, + ethContext.getEthPeers().peerCount())); } RequestDataStep( diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/RequestDataStep.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/RequestDataStep.java index 7422efdd274..6891d552ab3 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/RequestDataStep.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/RequestDataStep.java @@ -78,7 +78,8 @@ public CompletableFuture> requestAccount( accountDataRequest.getStartKeyHash(), accountDataRequest.getEndKeyHash(), blockHeader, - metricsSystem); + metricsSystem, + ethContext.getEthPeers().peerCount()); downloadState.addOutstandingTask(getAccountTask); return getAccountTask .run() @@ -113,7 +114,13 @@ public CompletableFuture>> requestStorage( : RangeManager.MAX_RANGE; final EthTask getStorageRangeTask = RetryingGetStorageRangeFromPeerTask.forStorageRange( - ethContext, accountHashes, minRange, maxRange, blockHeader, metricsSystem); + ethContext, + accountHashes, + minRange, + maxRange, + blockHeader, + metricsSystem, + ethContext.getEthPeers().peerCount()); downloadState.addOutstandingTask(getStorageRangeTask); return getStorageRangeTask .run() @@ -148,7 +155,11 @@ public CompletableFuture>> requestCode( final BlockHeader blockHeader = fastSyncState.getPivotBlockHeader().get(); final EthTask> getByteCodeTask = RetryingGetBytecodeFromPeerTask.forByteCode( - ethContext, codeHashes, blockHeader, metricsSystem); + ethContext, + codeHashes, + blockHeader, + metricsSystem, + ethContext.getEthPeers().peerCount()); downloadState.addOutstandingTask(getByteCodeTask); return getByteCodeTask .run() @@ -187,7 +198,7 @@ public CompletableFuture>> requestTrieNodeByPath( }); final EthTask> getTrieNodeFromPeerTask = RetryingGetTrieNodeFromPeerTask.forTrieNodes( - ethContext, message, blockHeader, metricsSystem); + ethContext, message, blockHeader, metricsSystem, ethContext.getEthPeers().peerCount()); downloadState.addOutstandingTask(getTrieNodeFromPeerTask); return getTrieNodeFromPeerTask .run() diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetNodeDataFromPeerTaskTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetNodeDataFromPeerTaskTest.java index 586bf2aa22d..21a2473d852 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetNodeDataFromPeerTaskTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetNodeDataFromPeerTaskTest.java @@ -53,7 +53,7 @@ protected Map generateDataToBeRequested() { protected EthTask> createTask(final Map requestedData) { final List hashes = Lists.newArrayList(requestedData.keySet()); return RetryingGetNodeDataFromPeerTask.forHashes( - ethContext, hashes, GENESIS_BLOCK_NUMBER, metricsSystem); + ethContext, hashes, GENESIS_BLOCK_NUMBER, metricsSystem, ethPeers.peerCount()); } @Test From 784b08f9a65ed814a749311476b456628da64522 Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Thu, 25 May 2023 18:15:03 +0200 Subject: [PATCH 02/23] Introduce a PoS friendly fast sync target manager Signed-off-by: Fabio Di Fabio --- .../CheckpointSyncChainDownloader.java | 3 +- .../fastsync/FastSyncChainDownloader.java | 2 +- .../fastsync/PoSFastSyncTargetManager.java | 89 +++++++++++++++++++ 3 files changed, 92 insertions(+), 2 deletions(-) create mode 100644 ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PoSFastSyncTargetManager.java diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointSyncChainDownloader.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointSyncChainDownloader.java index 854972c39fd..54ddf4734d0 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointSyncChainDownloader.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointSyncChainDownloader.java @@ -21,6 +21,7 @@ import org.hyperledger.besu.ethereum.eth.sync.fastsync.FastSyncChainDownloader; import org.hyperledger.besu.ethereum.eth.sync.fastsync.FastSyncState; import org.hyperledger.besu.ethereum.eth.sync.fastsync.FastSyncTargetManager; +import org.hyperledger.besu.ethereum.eth.sync.fastsync.PoSFastSyncTargetManager; import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.ethereum.worldstate.WorldStateStorage; @@ -39,7 +40,7 @@ public static ChainDownloader create( final FastSyncState fastSyncState) { final FastSyncTargetManager syncTargetManager = - new FastSyncTargetManager( + new PoSFastSyncTargetManager( config, worldStateStorage, protocolSchedule, diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncChainDownloader.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncChainDownloader.java index c4034cb87a0..db29e4429c7 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncChainDownloader.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncChainDownloader.java @@ -39,7 +39,7 @@ public static ChainDownloader create( final FastSyncState fastSyncState) { final FastSyncTargetManager syncTargetManager = - new FastSyncTargetManager( + new PoSFastSyncTargetManager( config, worldStateStorage, protocolSchedule, diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PoSFastSyncTargetManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PoSFastSyncTargetManager.java new file mode 100644 index 00000000000..583f5890e87 --- /dev/null +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PoSFastSyncTargetManager.java @@ -0,0 +1,89 @@ +/* + * Copyright Hyperledger Besu Contributors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ +package org.hyperledger.besu.ethereum.eth.sync.fastsync; + +import org.hyperledger.besu.ethereum.ProtocolContext; +import org.hyperledger.besu.ethereum.core.BlockHeader; +import org.hyperledger.besu.ethereum.eth.manager.EthContext; +import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.eth.manager.EthPeers; +import org.hyperledger.besu.ethereum.eth.sync.SyncTargetManager; +import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; +import org.hyperledger.besu.ethereum.eth.sync.tasks.RetryingGetHeaderFromPeerByHashTask; +import org.hyperledger.besu.ethereum.eth.sync.tasks.RetryingGetHeaderFromPeerByNumberTask; +import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; +import org.hyperledger.besu.ethereum.p2p.rlpx.wire.messages.DisconnectMessage.DisconnectReason; +import org.hyperledger.besu.ethereum.worldstate.WorldStateStorage; +import org.hyperledger.besu.plugin.services.MetricsSystem; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.List; +import java.util.Optional; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.atomic.AtomicBoolean; + +import static java.util.concurrent.CompletableFuture.completedFuture; +import static org.hyperledger.besu.ethereum.eth.sync.fastsync.PivotBlockRetriever.MAX_QUERY_RETRIES_PER_PEER; +import static org.hyperledger.besu.ethereum.util.LogUtil.throttledLog; + +public class PoSFastSyncTargetManager extends FastSyncTargetManager { + private static final Logger LOG = LoggerFactory.getLogger(PoSFastSyncTargetManager.class); + + private final ProtocolSchedule protocolSchedule; + private final EthContext ethContext; + private final MetricsSystem metricsSystem; + private final FastSyncState fastSyncState; + public PoSFastSyncTargetManager( + final SynchronizerConfiguration config, + final WorldStateStorage worldStateStorage, + final ProtocolSchedule protocolSchedule, + final ProtocolContext protocolContext, + final EthContext ethContext, + final MetricsSystem metricsSystem, + final FastSyncState fastSyncState) { + super(config, worldStateStorage, protocolSchedule, protocolContext, ethContext, metricsSystem, fastSyncState); + this.protocolSchedule = protocolSchedule; + this.ethContext = ethContext; + this.metricsSystem = metricsSystem; + this.fastSyncState = fastSyncState; + } + + @Override + protected CompletableFuture> selectBestAvailableSyncTarget() { + final BlockHeader pivotBlockHeader = fastSyncState.getPivotBlockHeader().get(); + final Optional maybeBestPeer = findPeerWithPivot(pivotBlockHeader); + if (maybeBestPeer.isEmpty()) { +// throttledLog( +// LOG::info, +// String.format( +// "Unable to find sync target. Currently checking %d peers for usefulness. Pivot block: %s", +// ethContext.getEthPeers().peerCount(), pivotBlockHeader.toLogString()), +// logDebug, +// logDebugRepeatDelay); + LOG.atDebug().setMessage("Unable to find sync target. Currently checking {} peers for usefulness. Pivot block: {}").addArgument(ethContext.getEthPeers().peerCount()).addArgument(pivotBlockHeader::toLogString).log(); + return completedFuture(Optional.empty()); + } else { + LOG.info("Using peer {} as sync target", maybeBestPeer.get()); + return completedFuture(maybeBestPeer); + } + } + + private Optional findPeerWithPivot(final BlockHeader pivotBlockHeader) { + final var task = RetryingGetHeaderFromPeerByHashTask.byHash(protocolSchedule, ethContext, pivotBlockHeader.getHash(), 0, metricsSystem); + task.run(); + return task.getAssignedPeer(); + } +} From 589f89faabbb01af1ec8e5b700b0d4a18abe168a Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Thu, 25 May 2023 20:49:29 +0200 Subject: [PATCH 03/23] Require that best peers are fully validated Signed-off-by: Fabio Di Fabio --- .../besu/ethereum/eth/manager/EthPeers.java | 4 +- .../sync/fastsync/PivotBlockConfirmer.java | 1 - .../fastsync/PoSFastSyncTargetManager.java | 51 +++++++++++-------- 3 files changed, 33 insertions(+), 23 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java index da5dc4cff8a..aa5ef3dfd0a 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java @@ -320,7 +320,9 @@ public Stream streamAvailablePeers() { } public Stream streamBestPeers() { - return streamAvailablePeers().sorted(getBestChainComparator().reversed()); + return streamAvailablePeers() + .filter(EthPeer::isFullyValidated) + .sorted(getBestChainComparator().reversed()); } public Optional bestPeer() { diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockConfirmer.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockConfirmer.java index 1253dbb17c1..dff6c703314 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockConfirmer.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockConfirmer.java @@ -183,7 +183,6 @@ private Optional createPivotQuery(final l .getEthPeers() .streamBestPeers() .filter(p -> p.chainState().getEstimatedHeight() >= blockNumber) - .filter(EthPeer::isFullyValidated) .filter(p -> !pivotBlockQueriesByPeerId.keySet().contains(p.nodeId())) .findFirst() .flatMap((peer) -> createGetHeaderTask(peer, blockNumber)); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PoSFastSyncTargetManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PoSFastSyncTargetManager.java index 583f5890e87..21a775229cc 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PoSFastSyncTargetManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PoSFastSyncTargetManager.java @@ -14,30 +14,23 @@ */ package org.hyperledger.besu.ethereum.eth.sync.fastsync; +import static java.util.concurrent.CompletableFuture.completedFuture; + import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; -import org.hyperledger.besu.ethereum.eth.manager.EthPeers; -import org.hyperledger.besu.ethereum.eth.sync.SyncTargetManager; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.eth.sync.tasks.RetryingGetHeaderFromPeerByHashTask; -import org.hyperledger.besu.ethereum.eth.sync.tasks.RetryingGetHeaderFromPeerByNumberTask; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; -import org.hyperledger.besu.ethereum.p2p.rlpx.wire.messages.DisconnectMessage.DisconnectReason; import org.hyperledger.besu.ethereum.worldstate.WorldStateStorage; import org.hyperledger.besu.plugin.services.MetricsSystem; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import java.util.List; import java.util.Optional; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.atomic.AtomicBoolean; -import static java.util.concurrent.CompletableFuture.completedFuture; -import static org.hyperledger.besu.ethereum.eth.sync.fastsync.PivotBlockRetriever.MAX_QUERY_RETRIES_PER_PEER; -import static org.hyperledger.besu.ethereum.util.LogUtil.throttledLog; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class PoSFastSyncTargetManager extends FastSyncTargetManager { private static final Logger LOG = LoggerFactory.getLogger(PoSFastSyncTargetManager.class); @@ -46,6 +39,7 @@ public class PoSFastSyncTargetManager extends FastSyncTargetManager { private final EthContext ethContext; private final MetricsSystem metricsSystem; private final FastSyncState fastSyncState; + public PoSFastSyncTargetManager( final SynchronizerConfiguration config, final WorldStateStorage worldStateStorage, @@ -54,7 +48,14 @@ public PoSFastSyncTargetManager( final EthContext ethContext, final MetricsSystem metricsSystem, final FastSyncState fastSyncState) { - super(config, worldStateStorage, protocolSchedule, protocolContext, ethContext, metricsSystem, fastSyncState); + super( + config, + worldStateStorage, + protocolSchedule, + protocolContext, + ethContext, + metricsSystem, + fastSyncState); this.protocolSchedule = protocolSchedule; this.ethContext = ethContext; this.metricsSystem = metricsSystem; @@ -66,14 +67,20 @@ protected CompletableFuture> selectBestAvailableSyncTarget() { final BlockHeader pivotBlockHeader = fastSyncState.getPivotBlockHeader().get(); final Optional maybeBestPeer = findPeerWithPivot(pivotBlockHeader); if (maybeBestPeer.isEmpty()) { -// throttledLog( -// LOG::info, -// String.format( -// "Unable to find sync target. Currently checking %d peers for usefulness. Pivot block: %s", -// ethContext.getEthPeers().peerCount(), pivotBlockHeader.toLogString()), -// logDebug, -// logDebugRepeatDelay); - LOG.atDebug().setMessage("Unable to find sync target. Currently checking {} peers for usefulness. Pivot block: {}").addArgument(ethContext.getEthPeers().peerCount()).addArgument(pivotBlockHeader::toLogString).log(); + // throttledLog( + // LOG::info, + // String.format( + // "Unable to find sync target. Currently checking %d peers for usefulness. Pivot + // block: %s", + // ethContext.getEthPeers().peerCount(), pivotBlockHeader.toLogString()), + // logDebug, + // logDebugRepeatDelay); + LOG.atDebug() + .setMessage( + "Unable to find sync target. Currently checking {} peers for usefulness. Pivot block: {}") + .addArgument(ethContext.getEthPeers().peerCount()) + .addArgument(pivotBlockHeader::toLogString) + .log(); return completedFuture(Optional.empty()); } else { LOG.info("Using peer {} as sync target", maybeBestPeer.get()); @@ -82,7 +89,9 @@ protected CompletableFuture> selectBestAvailableSyncTarget() { } private Optional findPeerWithPivot(final BlockHeader pivotBlockHeader) { - final var task = RetryingGetHeaderFromPeerByHashTask.byHash(protocolSchedule, ethContext, pivotBlockHeader.getHash(), 0, metricsSystem); + final var task = + RetryingGetHeaderFromPeerByHashTask.byHash( + protocolSchedule, ethContext, pivotBlockHeader.getHash(), 0, metricsSystem); task.run(); return task.getAssignedPeer(); } From a924542cfb42e6d1990d9928b7cff64ac70951fe Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Mon, 29 May 2023 11:17:26 +0200 Subject: [PATCH 04/23] Move PoS sync targe in another branch Signed-off-by: Fabio Di Fabio --- .../CheckpointSyncChainDownloader.java | 3 +- .../fastsync/FastSyncChainDownloader.java | 2 +- .../fastsync/PoSFastSyncTargetManager.java | 98 ------------------- 3 files changed, 2 insertions(+), 101 deletions(-) delete mode 100644 ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PoSFastSyncTargetManager.java diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointSyncChainDownloader.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointSyncChainDownloader.java index 54ddf4734d0..854972c39fd 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointSyncChainDownloader.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/checkpointsync/CheckpointSyncChainDownloader.java @@ -21,7 +21,6 @@ import org.hyperledger.besu.ethereum.eth.sync.fastsync.FastSyncChainDownloader; import org.hyperledger.besu.ethereum.eth.sync.fastsync.FastSyncState; import org.hyperledger.besu.ethereum.eth.sync.fastsync.FastSyncTargetManager; -import org.hyperledger.besu.ethereum.eth.sync.fastsync.PoSFastSyncTargetManager; import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.ethereum.worldstate.WorldStateStorage; @@ -40,7 +39,7 @@ public static ChainDownloader create( final FastSyncState fastSyncState) { final FastSyncTargetManager syncTargetManager = - new PoSFastSyncTargetManager( + new FastSyncTargetManager( config, worldStateStorage, protocolSchedule, diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncChainDownloader.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncChainDownloader.java index db29e4429c7..c4034cb87a0 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncChainDownloader.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncChainDownloader.java @@ -39,7 +39,7 @@ public static ChainDownloader create( final FastSyncState fastSyncState) { final FastSyncTargetManager syncTargetManager = - new PoSFastSyncTargetManager( + new FastSyncTargetManager( config, worldStateStorage, protocolSchedule, diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PoSFastSyncTargetManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PoSFastSyncTargetManager.java deleted file mode 100644 index 21a775229cc..00000000000 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PoSFastSyncTargetManager.java +++ /dev/null @@ -1,98 +0,0 @@ -/* - * Copyright Hyperledger Besu Contributors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - * - * SPDX-License-Identifier: Apache-2.0 - */ -package org.hyperledger.besu.ethereum.eth.sync.fastsync; - -import static java.util.concurrent.CompletableFuture.completedFuture; - -import org.hyperledger.besu.ethereum.ProtocolContext; -import org.hyperledger.besu.ethereum.core.BlockHeader; -import org.hyperledger.besu.ethereum.eth.manager.EthContext; -import org.hyperledger.besu.ethereum.eth.manager.EthPeer; -import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; -import org.hyperledger.besu.ethereum.eth.sync.tasks.RetryingGetHeaderFromPeerByHashTask; -import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; -import org.hyperledger.besu.ethereum.worldstate.WorldStateStorage; -import org.hyperledger.besu.plugin.services.MetricsSystem; - -import java.util.Optional; -import java.util.concurrent.CompletableFuture; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -public class PoSFastSyncTargetManager extends FastSyncTargetManager { - private static final Logger LOG = LoggerFactory.getLogger(PoSFastSyncTargetManager.class); - - private final ProtocolSchedule protocolSchedule; - private final EthContext ethContext; - private final MetricsSystem metricsSystem; - private final FastSyncState fastSyncState; - - public PoSFastSyncTargetManager( - final SynchronizerConfiguration config, - final WorldStateStorage worldStateStorage, - final ProtocolSchedule protocolSchedule, - final ProtocolContext protocolContext, - final EthContext ethContext, - final MetricsSystem metricsSystem, - final FastSyncState fastSyncState) { - super( - config, - worldStateStorage, - protocolSchedule, - protocolContext, - ethContext, - metricsSystem, - fastSyncState); - this.protocolSchedule = protocolSchedule; - this.ethContext = ethContext; - this.metricsSystem = metricsSystem; - this.fastSyncState = fastSyncState; - } - - @Override - protected CompletableFuture> selectBestAvailableSyncTarget() { - final BlockHeader pivotBlockHeader = fastSyncState.getPivotBlockHeader().get(); - final Optional maybeBestPeer = findPeerWithPivot(pivotBlockHeader); - if (maybeBestPeer.isEmpty()) { - // throttledLog( - // LOG::info, - // String.format( - // "Unable to find sync target. Currently checking %d peers for usefulness. Pivot - // block: %s", - // ethContext.getEthPeers().peerCount(), pivotBlockHeader.toLogString()), - // logDebug, - // logDebugRepeatDelay); - LOG.atDebug() - .setMessage( - "Unable to find sync target. Currently checking {} peers for usefulness. Pivot block: {}") - .addArgument(ethContext.getEthPeers().peerCount()) - .addArgument(pivotBlockHeader::toLogString) - .log(); - return completedFuture(Optional.empty()); - } else { - LOG.info("Using peer {} as sync target", maybeBestPeer.get()); - return completedFuture(maybeBestPeer); - } - } - - private Optional findPeerWithPivot(final BlockHeader pivotBlockHeader) { - final var task = - RetryingGetHeaderFromPeerByHashTask.byHash( - protocolSchedule, ethContext, pivotBlockHeader.getHash(), 0, metricsSystem); - task.run(); - return task.getAssignedPeer(); - } -} From 1378afc8c6e7d50cbc15209c153f0b04af0a9f9f Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Mon, 29 May 2023 12:35:11 +0200 Subject: [PATCH 05/23] Record empty responses when retrying a peer task Signed-off-by: Fabio Di Fabio --- .../task/AbstractRetryingPeerTask.java | 19 ++++++++++++++++--- .../AbstractRetryingSwitchingPeerTask.java | 8 +------- .../task/RetryingGetBlocksFromPeersTask.java | 18 +++++------------- ...gGetHeadersEndingAtFromPeerByHashTask.java | 14 -------------- 4 files changed, 22 insertions(+), 37 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java index cf48d69847d..7f8512a6a34 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java @@ -16,6 +16,7 @@ import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.eth.manager.exceptions.IncompleteResultsException; import org.hyperledger.besu.ethereum.eth.manager.exceptions.MaxRetriesReachedException; import org.hyperledger.besu.ethereum.eth.manager.exceptions.NoAvailablePeersException; import org.hyperledger.besu.ethereum.eth.manager.exceptions.PeerBreachedProtocolException; @@ -93,9 +94,19 @@ protected void executeTask() { if (error != null) { handleTaskError(error); } else { - // If we get a partial success, reset the retry counter. - if (!isEmptyResponse.test(peerResult)) { + // If we get a partial success, reset the retry counter, otherwise demote the peer + if (isEmptyResponse.test(peerResult)) { + // record this empty response, so that the peer will be disconnected if there were + // too many + assignedPeer.ifPresent( + peer -> { + peer.recordUselessResponse(getClass().getSimpleName()); + throw new IncompleteResultsException( + "Empty response from peer " + peer.getShortNodeId()); + }); + } else { retryCount = 0; + assignedPeer.ifPresent(EthPeer::recordUsefulResponse); } executeTaskTimed(); } @@ -140,7 +151,9 @@ protected void handleTaskError(final Throwable error) { } protected boolean isRetryableError(final Throwable error) { - return error instanceof TimeoutException || (!assignedPeer.isPresent() && isPeerFailure(error)); + return error instanceof IncompleteResultsException + || error instanceof TimeoutException + || (!assignedPeer.isPresent() && isPeerFailure(error)); } protected boolean isPeerFailure(final Throwable error) { diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingSwitchingPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingSwitchingPeerTask.java index ccb3bdc7ebc..254c0dfd4a8 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingSwitchingPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingSwitchingPeerTask.java @@ -25,7 +25,6 @@ import java.util.Optional; import java.util.Set; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.TimeoutException; import java.util.function.Predicate; import java.util.stream.Stream; @@ -106,11 +105,6 @@ protected void handleTaskError(final Throwable error) { super.handleTaskError(error); } - @Override - protected boolean isRetryableError(final Throwable error) { - return error instanceof TimeoutException || isPeerFailure(error); - } - private Optional selectNextPeer() { final Optional maybeNextPeer = remainingPeersToTry().findFirst(); @@ -136,7 +130,7 @@ private void refreshPeers() { // If we are at max connections, then refresh peers disconnecting one of the failed peers, // or the least useful - if (peers.peerCount() >= peers.getMaxPeers()) { + if (peers.peerCount() >= peers.getPeerLowerBound()) { failedPeers.stream() .filter(peer -> !peer.isDisconnected()) .findAny() diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetBlocksFromPeersTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetBlocksFromPeersTask.java index 0e663ffe24d..e082becde2a 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetBlocksFromPeersTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetBlocksFromPeersTask.java @@ -18,7 +18,6 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; -import org.hyperledger.besu.ethereum.eth.manager.exceptions.IncompleteResultsException; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask.PeerTaskResult; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.plugin.services.MetricsSystem; @@ -44,7 +43,11 @@ protected RetryingGetBlocksFromPeersTask( final MetricsSystem metricsSystem, final int maxRetries, final List headers) { - super(ethContext, metricsSystem, Objects::isNull, maxRetries); + super( + ethContext, + metricsSystem, + res -> Objects.isNull(res) || res.getResult().isEmpty(), + maxRetries); this.protocolSchedule = protocolSchedule; this.headers = headers; } @@ -77,22 +80,11 @@ protected CompletableFuture>> executeTaskOnCurrentPee .addArgument(this::getRetryCount) .log(); - if (peerResult.getResult().isEmpty()) { - currentPeer.recordUselessResponse("GetBodiesFromPeerTask"); - throw new IncompleteResultsException( - "No blocks returned by peer " + currentPeer.getShortNodeId()); - } - result.complete(peerResult); return peerResult; }); } - @Override - protected boolean isRetryableError(final Throwable error) { - return super.isRetryableError(error) || error instanceof IncompleteResultsException; - } - @Override protected void handleTaskError(final Throwable error) { if (getRetryCount() < getMaxRetries()) { diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetHeadersEndingAtFromPeerByHashTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetHeadersEndingAtFromPeerByHashTask.java index e467bfba119..4be2b571f8f 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetHeadersEndingAtFromPeerByHashTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetHeadersEndingAtFromPeerByHashTask.java @@ -21,7 +21,6 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; -import org.hyperledger.besu.ethereum.eth.manager.exceptions.IncompleteResultsException; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.plugin.services.MetricsSystem; @@ -98,21 +97,8 @@ protected CompletableFuture> executeTaskOnCurrentPeer( referenceHash, currentPeer, peerResult.getResult()); - if (peerResult.getResult().isEmpty()) { - currentPeer.recordUselessResponse("GetHeadersFromPeerByHashTask"); - throw new IncompleteResultsException( - "No block headers for hash " - + referenceHash - + " returned by peer " - + currentPeer.getShortNodeId()); - } result.complete(peerResult.getResult()); return peerResult.getResult(); }); } - - @Override - protected boolean isRetryableError(final Throwable error) { - return super.isRetryableError(error) || error instanceof IncompleteResultsException; - } } From a20c5edaca44ad6bda35621c44ec0bde96d8b8d5 Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Mon, 29 May 2023 16:01:41 +0200 Subject: [PATCH 06/23] Review and consolidate the way result is set in retrying tasks Signed-off-by: Fabio Di Fabio --- .../RetryingGetAccountRangeFromPeerTask.java | 21 ++++--- .../snap/RetryingGetBytecodeFromPeerTask.java | 20 +++--- .../RetryingGetStorageRangeFromPeerTask.java | 20 +++--- .../snap/RetryingGetTrieNodeFromPeerTask.java | 20 +++--- .../task/AbstractRetryingPeerTask.java | 63 ++++++++++++------- .../AbstractRetryingSwitchingPeerTask.java | 11 +--- .../task/RetryingGetBlockFromPeersTask.java | 20 +++--- .../task/RetryingGetBlocksFromPeersTask.java | 28 ++++----- ...gGetHeadersEndingAtFromPeerByHashTask.java | 18 ++++-- .../task/RetryingGetNodeDataFromPeerTask.java | 20 +++--- .../sync/backwardsync/ForwardSyncStep.java | 17 +++-- .../eth/sync/tasks/CompleteBlocksTask.java | 15 ++++- .../tasks/DownloadHeaderSequenceTask.java | 24 ++++--- .../sync/tasks/GetReceiptsForHeadersTask.java | 16 ++++- .../RetryingGetHeaderFromPeerByHashTask.java | 24 +++---- ...RetryingGetHeaderFromPeerByNumberTask.java | 23 ++++--- .../task/AbstractRetryingPeerTaskTest.java | 14 ++++- 17 files changed, 231 insertions(+), 143 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java index 7512bdd03a9..bf594a58d81 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java @@ -17,6 +17,7 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask.PeerTaskResult; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractRetryingPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.EthTask; import org.hyperledger.besu.ethereum.eth.messages.snap.AccountRangeMessage; @@ -42,8 +43,7 @@ private RetryingGetAccountRangeFromPeerTask( final Bytes32 endKeyHash, final BlockHeader blockHeader, final MetricsSystem metricsSystem) { - super( - ethContext, 4, data -> data.accounts().isEmpty() && data.proofs().isEmpty(), metricsSystem); + super(ethContext, 4, metricsSystem); this.ethContext = ethContext; this.startKeyHash = startKeyHash; this.endKeyHash = endKeyHash; @@ -68,11 +68,16 @@ protected CompletableFuture executePeerTas GetAccountRangeFromPeerTask.forAccountRange( ethContext, startKeyHash, endKeyHash, blockHeader, metricsSystem); assignedPeer.ifPresent(task::assignPeer); - return executeSubTask(task::run) - .thenApply( - peerResult -> { - result.complete(peerResult.getResult()); - return peerResult.getResult(); - }); + return executeSubTask(task::run).thenApply(PeerTaskResult::getResult); + } + + @Override + protected boolean emptyResult(final AccountRangeMessage.AccountRangeData data) { + return data.accounts().isEmpty() && data.proofs().isEmpty(); + } + + @Override + protected boolean successfulResult(final AccountRangeMessage.AccountRangeData peerResult) { + return !emptyResult(peerResult); } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetBytecodeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetBytecodeFromPeerTask.java index 7d23ec944d3..8c18fcb5eb4 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetBytecodeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetBytecodeFromPeerTask.java @@ -17,6 +17,7 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask.PeerTaskResult; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractRetryingPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.EthTask; import org.hyperledger.besu.plugin.services.MetricsSystem; @@ -41,7 +42,7 @@ private RetryingGetBytecodeFromPeerTask( final List codeHashes, final BlockHeader blockHeader, final MetricsSystem metricsSystem) { - super(ethContext, 4, Map::isEmpty, metricsSystem); + super(ethContext, 4, metricsSystem); this.ethContext = ethContext; this.codeHashes = codeHashes; this.blockHeader = blockHeader; @@ -62,11 +63,16 @@ protected CompletableFuture> executePeerTask( final GetBytecodeFromPeerTask task = GetBytecodeFromPeerTask.forBytecode(ethContext, codeHashes, blockHeader, metricsSystem); assignedPeer.ifPresent(task::assignPeer); - return executeSubTask(task::run) - .thenApply( - peerResult -> { - result.complete(peerResult.getResult()); - return peerResult.getResult(); - }); + return executeSubTask(task::run).thenApply(PeerTaskResult::getResult); + } + + @Override + protected boolean emptyResult(final Map peerResult) { + return peerResult.isEmpty(); + } + + @Override + protected boolean successfulResult(final Map peerResult) { + return !emptyResult(peerResult); } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetStorageRangeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetStorageRangeFromPeerTask.java index 7a095de9292..e526580d899 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetStorageRangeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetStorageRangeFromPeerTask.java @@ -17,6 +17,7 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask.PeerTaskResult; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractRetryingPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.EthTask; import org.hyperledger.besu.ethereum.eth.messages.snap.StorageRangeMessage; @@ -45,7 +46,7 @@ private RetryingGetStorageRangeFromPeerTask( final Bytes32 endKeyHash, final BlockHeader blockHeader, final MetricsSystem metricsSystem) { - super(ethContext, 4, data -> data.proofs().isEmpty() && data.slots().isEmpty(), metricsSystem); + super(ethContext, 4, metricsSystem); this.ethContext = ethContext; this.accountHashes = accountHashes; this.startKeyHash = startKeyHash; @@ -72,11 +73,16 @@ protected CompletableFuture executePeerTask( GetStorageRangeFromPeerTask.forStorageRange( ethContext, accountHashes, startKeyHash, endKeyHash, blockHeader, metricsSystem); assignedPeer.ifPresent(task::assignPeer); - return executeSubTask(task::run) - .thenApply( - peerResult -> { - result.complete(peerResult.getResult()); - return peerResult.getResult(); - }); + return executeSubTask(task::run).thenApply(PeerTaskResult::getResult); + } + + @Override + protected boolean emptyResult(final StorageRangeMessage.SlotRangeData peerResult) { + return peerResult.proofs().isEmpty() && peerResult.slots().isEmpty(); + } + + @Override + protected boolean successfulResult(final StorageRangeMessage.SlotRangeData peerResult) { + return !emptyResult(peerResult); } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java index ebfd8856898..5071187754a 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java @@ -17,6 +17,7 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask.PeerTaskResult; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractRetryingPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.EthTask; import org.hyperledger.besu.plugin.services.MetricsSystem; @@ -40,7 +41,7 @@ private RetryingGetTrieNodeFromPeerTask( final Map> paths, final BlockHeader blockHeader, final MetricsSystem metricsSystem) { - super(ethContext, 4, Map::isEmpty, metricsSystem); + super(ethContext, 4, metricsSystem); this.ethContext = ethContext; this.paths = paths; this.blockHeader = blockHeader; @@ -61,11 +62,16 @@ protected CompletableFuture> executePeerTask( final GetTrieNodeFromPeerTask task = GetTrieNodeFromPeerTask.forTrieNodes(ethContext, paths, blockHeader, metricsSystem); assignedPeer.ifPresent(task::assignPeer); - return executeSubTask(task::run) - .thenApply( - peerResult -> { - result.complete(peerResult.getResult()); - return peerResult.getResult(); - }); + return executeSubTask(task::run).thenApply(PeerTaskResult::getResult); + } + + @Override + protected boolean emptyResult(final Map peerResult) { + return peerResult.isEmpty(); + } + + @Override + protected boolean successfulResult(final Map peerResult) { + return !emptyResult(peerResult); } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java index 7f8512a6a34..d10ee358a59 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java @@ -28,7 +28,6 @@ import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeoutException; -import java.util.function.Predicate; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -36,7 +35,14 @@ /** * A task that will retry a fixed number of times before completing the associated CompletableFuture * exceptionally with a new {@link MaxRetriesReachedException}. If the future returned from {@link - * #executePeerTask(Optional)} is complete with a non-empty list the retry counter is reset. + * #executePeerTask(Optional)} is considered an empty result by {@link #emptyResult(Object)} the + * peer is demoted, if the result is complete according to {@link #successfulResult(Object)} then + * the final task result is set, otherwise the result is considered partial and the retry counter is + * reset. + * + *

Note: extending classes should never set the final task result, using {@code + * result.complete} by themselves, but should return true from {@link #successfulResult(Object)} + * when done. * * @param The type as a typed list that the peer task can get partial or full results in. */ @@ -45,7 +51,6 @@ public abstract class AbstractRetryingPeerTask extends AbstractEthTask { private static final Logger LOG = LoggerFactory.getLogger(AbstractRetryingPeerTask.class); private final EthContext ethContext; private final int maxRetries; - private final Predicate isEmptyResponse; private final MetricsSystem metricsSystem; private int retryCount = 0; private Optional assignedPeer = Optional.empty(); @@ -53,18 +58,13 @@ public abstract class AbstractRetryingPeerTask extends AbstractEthTask { /** * @param ethContext The context of the current Eth network we are attached to. * @param maxRetries Maximum number of retries to accept before completing exceptionally. - * @param isEmptyResponse Test if the response received was empty. * @param metricsSystem The metrics system used to measure task. */ protected AbstractRetryingPeerTask( - final EthContext ethContext, - final int maxRetries, - final Predicate isEmptyResponse, - final MetricsSystem metricsSystem) { + final EthContext ethContext, final int maxRetries, final MetricsSystem metricsSystem) { super(metricsSystem); this.ethContext = ethContext; this.maxRetries = maxRetries; - this.isEmptyResponse = isEmptyResponse; this.metricsSystem = metricsSystem; } @@ -94,21 +94,21 @@ protected void executeTask() { if (error != null) { handleTaskError(error); } else { - // If we get a partial success, reset the retry counter, otherwise demote the peer - if (isEmptyResponse.test(peerResult)) { - // record this empty response, so that the peer will be disconnected if there were - // too many - assignedPeer.ifPresent( - peer -> { - peer.recordUselessResponse(getClass().getSimpleName()); - throw new IncompleteResultsException( - "Empty response from peer " + peer.getShortNodeId()); - }); + if (successfulResult(peerResult)) { + result.complete(peerResult); } else { - retryCount = 0; - assignedPeer.ifPresent(EthPeer::recordUsefulResponse); + if (emptyResult(peerResult)) { + // record this empty response, so that the peer will be disconnected if there + // were too many + assignedPeer.ifPresent( + peer -> peer.recordUselessResponse(getClass().getSimpleName())); + } else { + // If we get a partial success, reset the retry counter + retryCount = 0; + } + // retry + executeTaskTimed(); } - executeTaskTimed(); } }); } @@ -177,4 +177,23 @@ public int getRetryCount() { public int getMaxRetries() { return maxRetries; } + + /** + * Identify if the result is empty. + * + * @param peerResult the result to check + * @return true if the result is empty and the peer should be demoted and the request retried + */ + protected abstract boolean emptyResult(final T peerResult); + + /** + * Identify a successful and complete result. Partial results that are not considered successful + * should return false, so that the request is retried. This check has precedence over the {@link + * #emptyResult(Object)}, so if an empty result is also successful the task completes successfully + * with an empty result. + * + * @param peerResult the result to check + * @return true if the result is successful and can be set as the task result + */ + protected abstract boolean successfulResult(final T peerResult); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingSwitchingPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingSwitchingPeerTask.java index 254c0dfd4a8..eb52312f5a3 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingSwitchingPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingSwitchingPeerTask.java @@ -25,7 +25,6 @@ import java.util.Optional; import java.util.Set; import java.util.concurrent.CompletableFuture; -import java.util.function.Predicate; import java.util.stream.Stream; import org.slf4j.Logger; @@ -40,11 +39,8 @@ public abstract class AbstractRetryingSwitchingPeerTask extends AbstractRetry private final Set failedPeers = new HashSet<>(); protected AbstractRetryingSwitchingPeerTask( - final EthContext ethContext, - final MetricsSystem metricsSystem, - final Predicate isEmptyResponse, - final int maxRetries) { - super(ethContext, maxRetries, isEmptyResponse, metricsSystem); + final EthContext ethContext, final MetricsSystem metricsSystem, final int maxRetries) { + super(ethContext, maxRetries, metricsSystem); } @Override @@ -92,7 +88,6 @@ protected CompletableFuture executePeerTask(final Optional assignedP .addArgument(peerToUse) .addArgument(this::getRetryCount) .log(); - result.complete(peerResult); return peerResult; }); } @@ -100,7 +95,7 @@ protected CompletableFuture executePeerTask(final Optional assignedP @Override protected void handleTaskError(final Throwable error) { if (isPeerFailure(error)) { - getAssignedPeer().ifPresent(peer -> failedPeers.add(peer)); + getAssignedPeer().ifPresent(failedPeers::add); } super.handleTaskError(error); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetBlockFromPeersTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetBlockFromPeersTask.java index f7266f9c88e..e3ac7309eaf 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetBlockFromPeersTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetBlockFromPeersTask.java @@ -18,12 +18,10 @@ import org.hyperledger.besu.ethereum.core.Block; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; -import org.hyperledger.besu.ethereum.eth.manager.exceptions.IncompleteResultsException; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask.PeerTaskResult; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.plugin.services.MetricsSystem; -import java.util.Objects; import java.util.Optional; import java.util.concurrent.CompletableFuture; @@ -46,7 +44,7 @@ protected RetryingGetBlockFromPeersTask( final int maxRetries, final Optional maybeBlockHash, final long blockNumber) { - super(ethContext, metricsSystem, Objects::isNull, maxRetries); + super(ethContext, metricsSystem, maxRetries); this.protocolSchedule = protocolSchedule; this.maybeBlockHash = maybeBlockHash; this.blockNumber = blockNumber; @@ -80,16 +78,10 @@ protected CompletableFuture> executeTaskOnCurrentPeer( .addArgument(peerResult.getPeer()) .addArgument(this::getRetryCount) .log(); - result.complete(peerResult); return peerResult; }); } - @Override - protected boolean isRetryableError(final Throwable error) { - return super.isRetryableError(error) || error instanceof IncompleteResultsException; - } - @Override protected void handleTaskError(final Throwable error) { if (getRetryCount() < getMaxRetries()) { @@ -109,6 +101,16 @@ protected void handleTaskError(final Throwable error) { super.handleTaskError(error); } + @Override + protected boolean emptyResult(final PeerTaskResult peerResult) { + return peerResult.getResult() == null; + } + + @Override + protected boolean successfulResult(final PeerTaskResult peerResult) { + return !emptyResult(peerResult); + } + private String logBlockNumberMaybeHash() { return blockNumber + maybeBlockHash.map(h -> " (" + h.toHexString() + ")").orElse(""); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetBlocksFromPeersTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetBlocksFromPeersTask.java index e082becde2a..8902b8e6798 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetBlocksFromPeersTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetBlocksFromPeersTask.java @@ -18,19 +18,16 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; -import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask.PeerTaskResult; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.plugin.services.MetricsSystem; import java.util.List; -import java.util.Objects; import java.util.concurrent.CompletableFuture; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class RetryingGetBlocksFromPeersTask - extends AbstractRetryingSwitchingPeerTask>> { +public class RetryingGetBlocksFromPeersTask extends AbstractRetryingSwitchingPeerTask> { private static final Logger LOG = LoggerFactory.getLogger(RetryingGetBlocksFromPeersTask.class); @@ -43,11 +40,7 @@ protected RetryingGetBlocksFromPeersTask( final MetricsSystem metricsSystem, final int maxRetries, final List headers) { - super( - ethContext, - metricsSystem, - res -> Objects.isNull(res) || res.getResult().isEmpty(), - maxRetries); + super(ethContext, metricsSystem, maxRetries); this.protocolSchedule = protocolSchedule; this.headers = headers; } @@ -63,8 +56,7 @@ public static RetryingGetBlocksFromPeersTask forHeaders( } @Override - protected CompletableFuture>> executeTaskOnCurrentPeer( - final EthPeer currentPeer) { + protected CompletableFuture> executeTaskOnCurrentPeer(final EthPeer currentPeer) { final GetBodiesFromPeerTask getBodiesTask = GetBodiesFromPeerTask.forHeaders( protocolSchedule, getEthContext(), headers, getMetricsSystem()); @@ -79,9 +71,7 @@ protected CompletableFuture>> executeTaskOnCurrentPee .addArgument(peerResult.getPeer()) .addArgument(this::getRetryCount) .log(); - - result.complete(peerResult); - return peerResult; + return peerResult.getResult(); }); } @@ -99,4 +89,14 @@ protected void handleTaskError(final Throwable error) { } super.handleTaskError(error); } + + @Override + protected boolean emptyResult(final List peerResult) { + return peerResult.isEmpty(); + } + + @Override + protected boolean successfulResult(final List peerResult) { + return !emptyResult(peerResult); + } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetHeadersEndingAtFromPeerByHashTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetHeadersEndingAtFromPeerByHashTask.java index 4be2b571f8f..f2a97354d8e 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetHeadersEndingAtFromPeerByHashTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetHeadersEndingAtFromPeerByHashTask.java @@ -50,7 +50,7 @@ public class RetryingGetHeadersEndingAtFromPeerByHashTask final int count, final MetricsSystem metricsSystem, final int maxRetries) { - super(ethContext, metricsSystem, List::isEmpty, maxRetries); + super(ethContext, metricsSystem, maxRetries); this.protocolSchedule = protocolSchedule; this.minimumRequiredBlockNumber = minimumRequiredBlockNumber; this.count = count; @@ -91,14 +91,24 @@ protected CompletableFuture> executeTaskOnCurrentPeer( return executeSubTask(task::run) .thenApply( peerResult -> { + final var res = peerResult.getResult(); LOG.debug( "Get {} block headers by hash {} from peer {} has result {}", count, referenceHash, currentPeer, - peerResult.getResult()); - result.complete(peerResult.getResult()); - return peerResult.getResult(); + res); + return res; }); } + + @Override + protected boolean emptyResult(final List peerResult) { + return peerResult.isEmpty(); + } + + @Override + protected boolean successfulResult(final List peerResult) { + return !emptyResult(peerResult); + } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetNodeDataFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetNodeDataFromPeerTask.java index 16040e963ae..5cbe811b662 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetNodeDataFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetNodeDataFromPeerTask.java @@ -17,6 +17,7 @@ import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; +import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask.PeerTaskResult; import org.hyperledger.besu.plugin.services.MetricsSystem; import java.util.Collection; @@ -40,7 +41,7 @@ private RetryingGetNodeDataFromPeerTask( final Collection hashes, final long pivotBlockNumber, final MetricsSystem metricsSystem) { - super(ethContext, 4, data -> false, metricsSystem); + super(ethContext, 4, metricsSystem); this.ethContext = ethContext; this.hashes = new HashSet<>(hashes); this.pivotBlockNumber = pivotBlockNumber; @@ -61,11 +62,16 @@ protected CompletableFuture> executePeerTask( final GetNodeDataFromPeerTask task = GetNodeDataFromPeerTask.forHashes(ethContext, hashes, pivotBlockNumber, metricsSystem); assignedPeer.ifPresent(task::assignPeer); - return executeSubTask(task::run) - .thenApply( - peerResult -> { - result.complete(peerResult.getResult()); - return peerResult.getResult(); - }); + return executeSubTask(task::run).thenApply(PeerTaskResult::getResult); + } + + @Override + protected boolean emptyResult(final Map peerResult) { + return false; + } + + @Override + protected boolean successfulResult(final Map peerResult) { + return !emptyResult(peerResult); } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/ForwardSyncStep.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/ForwardSyncStep.java index c71a97a5f49..6d9550d06ca 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/ForwardSyncStep.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/ForwardSyncStep.java @@ -16,7 +16,6 @@ import org.hyperledger.besu.ethereum.core.Block; import org.hyperledger.besu.ethereum.core.BlockHeader; -import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.RetryingGetBlocksFromPeersTask; import java.util.Comparator; @@ -84,15 +83,13 @@ protected CompletableFuture> requestBodies(final List b context.getEthContext().getEthPeers().peerCount(), blockHeaders); - final CompletableFuture>> run = - getBodiesFromPeerTask.run(); - return run.thenApply(AbstractPeerTask.PeerTaskResult::getResult) - .thenApply( - blocks -> { - LOG.debug("Got {} blocks from peers", blocks.size()); - blocks.sort(Comparator.comparing(block -> block.getHeader().getNumber())); - return blocks; - }); + final CompletableFuture> run = getBodiesFromPeerTask.run(); + return run.thenApply( + blocks -> { + LOG.debug("Got {} blocks from peers", blocks.size()); + blocks.sort(Comparator.comparing(block -> block.getHeader().getNumber())); + return blocks; + }); } @VisibleForTesting diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/CompleteBlocksTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/CompleteBlocksTask.java index 6e04833b644..ff0689e726c 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/CompleteBlocksTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/CompleteBlocksTask.java @@ -31,7 +31,6 @@ import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.plugin.services.MetricsSystem; -import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Map; @@ -66,7 +65,7 @@ private CompleteBlocksTask( final List headers, final int maxRetries, final MetricsSystem metricsSystem) { - super(ethContext, maxRetries, Collection::isEmpty, metricsSystem); + super(ethContext, maxRetries, metricsSystem); checkArgument(headers.size() > 0, "Must supply a non-empty headers list"); this.protocolSchedule = protocolSchedule; this.ethContext = ethContext; @@ -134,6 +133,16 @@ protected CompletableFuture> executePeerTask(final Optional return requestBodies(assignedPeer).thenCompose(this::processBodiesResult); } + @Override + protected boolean emptyResult(final List peerResult) { + return peerResult.isEmpty(); + } + + @Override + protected boolean successfulResult(final List peerResult) { + return incompleteHeaders().isEmpty(); + } + private CompletableFuture> requestBodies(final Optional assignedPeer) { final List incompleteHeaders = incompleteHeaders(); if (incompleteHeaders.isEmpty()) { @@ -157,7 +166,7 @@ private CompletableFuture> processBodiesResult(final List blo blocksResult.forEach((block) -> blocks.put(block.getHeader().getNumber(), block)); if (incompleteHeaders().isEmpty()) { - result.complete( + return completedFuture( headers.stream().map(h -> blocks.get(h.getNumber())).collect(Collectors.toList())); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DownloadHeaderSequenceTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DownloadHeaderSequenceTask.java index 23e26ff8922..3bc13c1325f 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DownloadHeaderSequenceTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DownloadHeaderSequenceTask.java @@ -39,7 +39,6 @@ import org.hyperledger.besu.plugin.services.MetricsSystem; import java.util.Arrays; -import java.util.Collection; import java.util.List; import java.util.Optional; import java.util.concurrent.CompletableFuture; @@ -78,7 +77,7 @@ private DownloadHeaderSequenceTask( final int maxRetries, final ValidationPolicy validationPolicy, final MetricsSystem metricsSystem) { - super(ethContext, maxRetries, Collection::isEmpty, metricsSystem); + super(ethContext, maxRetries, metricsSystem); this.protocolSchedule = protocolSchedule; this.protocolContext = protocolContext; this.ethContext = ethContext; @@ -139,19 +138,30 @@ protected CompletableFuture> executePeerTask( "Downloading headers from {} to {}.", startingBlockNumber, referenceHeader.getNumber()); final CompletableFuture> task = downloadHeaders(assignedPeer).thenCompose(this::processHeaders); - return task.whenComplete( - (r, t) -> { - // We're done if we've filled all requested headers - if (lastFilledHeaderIndex == 0) { + return task.thenApply( + r -> { + if (successfulResult(r)) { LOG.debug( "Finished downloading headers from {} to {}.", headers[0].getNumber(), headers[segmentLength - 1].getNumber()); - result.complete(Arrays.asList(headers)); + return Arrays.asList(headers); } + return r; }); } + @Override + protected boolean emptyResult(final List peerResult) { + return peerResult.isEmpty(); + } + + @Override + protected boolean successfulResult(final List peerResult) { + // We're done if we've filled all requested headers + return lastFilledHeaderIndex == 0; + } + private CompletableFuture>> downloadHeaders( final Optional assignedPeer) { // Figure out parameters for our headers request diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/GetReceiptsForHeadersTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/GetReceiptsForHeadersTask.java index 699bf9c4b17..2ef3a7bfb18 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/GetReceiptsForHeadersTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/GetReceiptsForHeadersTask.java @@ -55,7 +55,7 @@ private GetReceiptsForHeadersTask( final List headers, final int maxRetries, final MetricsSystem metricsSystem) { - super(ethContext, maxRetries, Map::isEmpty, metricsSystem); + super(ethContext, maxRetries, metricsSystem); checkArgument(headers.size() > 0, "Must supply a non-empty headers list"); this.ethContext = ethContext; this.metricsSystem = metricsSystem; @@ -92,6 +92,16 @@ protected CompletableFuture>> executeP return requestReceipts(assignedPeer).thenCompose(this::processResponse); } + @Override + protected boolean emptyResult(final Map> peerResult) { + return peerResult.isEmpty(); + } + + @Override + protected boolean successfulResult(final Map> peerResult) { + return isComplete(); + } + private CompletableFuture>> requestReceipts( final Optional assignedPeer) { final List incompleteHeaders = incompleteHeaders(); @@ -115,8 +125,8 @@ private CompletableFuture>> processRes final Map> responseData) { receipts.putAll(responseData); - if (isComplete()) { - result.complete(receipts); + if (successfulResult(responseData)) { + return CompletableFuture.completedFuture(receipts); } return CompletableFuture.completedFuture(responseData); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/RetryingGetHeaderFromPeerByHashTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/RetryingGetHeaderFromPeerByHashTask.java index c36599e6354..bd55e3951e1 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/RetryingGetHeaderFromPeerByHashTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/RetryingGetHeaderFromPeerByHashTask.java @@ -20,7 +20,6 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; -import org.hyperledger.besu.ethereum.eth.manager.exceptions.IncompleteResultsException; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractGetHeadersFromPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractRetryingSwitchingPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.GetHeadersFromPeerByHashTask; @@ -50,7 +49,7 @@ public class RetryingGetHeaderFromPeerByHashTask final long minimumRequiredBlockNumber, final MetricsSystem metricsSystem, final int maxRetries) { - super(ethContext, metricsSystem, List::isEmpty, maxRetries); + super(ethContext, metricsSystem, maxRetries); this.protocolSchedule = protocolSchedule; this.minimumRequiredBlockNumber = minimumRequiredBlockNumber; checkNotNull(referenceHash); @@ -90,24 +89,21 @@ protected CompletableFuture> executeTaskOnCurrentPeer(final Et referenceHash, peer, peerResult.getResult()); - if (peerResult.getResult().isEmpty()) { - throw new IncompleteResultsException( - "No block header for hash " - + referenceHash - + " returned by peer " - + peer.getShortNodeId()); - } - result.complete(peerResult.getResult()); return peerResult.getResult(); }); } + public CompletableFuture getHeader() { + return run().thenApply(singletonList -> singletonList.get(0)); + } + @Override - protected boolean isRetryableError(final Throwable error) { - return super.isRetryableError(error) || error instanceof IncompleteResultsException; + protected boolean emptyResult(final List peerResult) { + return peerResult.isEmpty(); } - public CompletableFuture getHeader() { - return run().thenApply(singletonList -> singletonList.get(0)); + @Override + protected boolean successfulResult(final List peerResult) { + return !emptyResult(peerResult); } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/RetryingGetHeaderFromPeerByNumberTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/RetryingGetHeaderFromPeerByNumberTask.java index 676382c069e..e334cbddb73 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/RetryingGetHeaderFromPeerByNumberTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/RetryingGetHeaderFromPeerByNumberTask.java @@ -18,12 +18,12 @@ import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractGetHeadersFromPeerTask; +import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask.PeerTaskResult; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractRetryingPeerTask; import org.hyperledger.besu.ethereum.eth.manager.task.GetHeadersFromPeerByNumberTask; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.plugin.services.MetricsSystem; -import java.util.Collection; import java.util.List; import java.util.Optional; import java.util.concurrent.CompletableFuture; @@ -41,7 +41,7 @@ private RetryingGetHeaderFromPeerByNumberTask( final MetricsSystem metricsSystem, final long pivotBlockNumber, final int maxRetries) { - super(ethContext, maxRetries, Collection::isEmpty, metricsSystem); + super(ethContext, maxRetries, metricsSystem); this.protocolSchedule = protocolSchedule; this.ethContext = ethContext; this.pivotBlockNumber = pivotBlockNumber; @@ -65,14 +65,17 @@ protected CompletableFuture> executePeerTask( GetHeadersFromPeerByNumberTask.forSingleNumber( protocolSchedule, ethContext, pivotBlockNumber, metricsSystem); assignedPeer.ifPresent(getHeadersTask::assignPeer); - return executeSubTask(getHeadersTask::run) - .thenApply( - peerResult -> { - if (!peerResult.getResult().isEmpty()) { - result.complete(peerResult.getResult()); - } - return peerResult.getResult(); - }); + return executeSubTask(getHeadersTask::run).thenApply(PeerTaskResult::getResult); + } + + @Override + protected boolean emptyResult(final List peerResult) { + return peerResult.isEmpty(); + } + + @Override + protected boolean successfulResult(final List peerResult) { + return !emptyResult(peerResult); } public CompletableFuture getHeader() { diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTaskTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTaskTest.java index 90b407f2fce..ff616782d71 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTaskTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTaskTest.java @@ -23,7 +23,6 @@ import org.hyperledger.besu.metrics.noop.NoOpMetricsSystem; import org.hyperledger.besu.plugin.services.MetricsSystem; -import java.util.Objects; import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; @@ -80,7 +79,7 @@ private class TaskThatFailsSometimes extends AbstractRetryingPeerTask { int failures = 0; protected TaskThatFailsSometimes(final int initialFailures, final int maxRetries) { - super(ethContext, maxRetries, Objects::isNull, metricsSystem); + super(ethContext, maxRetries, metricsSystem); this.initialFailures = initialFailures; } @@ -91,9 +90,18 @@ protected CompletableFuture executePeerTask(final Optional ass failures++; return CompletableFuture.completedFuture(null); } else { - result.complete(Boolean.TRUE); return CompletableFuture.completedFuture(Boolean.TRUE); } } + + @Override + protected boolean emptyResult(final Boolean peerResult) { + return peerResult == null; + } + + @Override + protected boolean successfulResult(final Boolean peerResult) { + return !emptyResult(peerResult); + } } } From c695c32294d5bf2b49eb53fa4709debe994e8ad9 Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Tue, 30 May 2023 13:03:48 +0200 Subject: [PATCH 07/23] Fix: always use the root cause to check the errors Signed-off-by: Fabio Di Fabio --- .../task/AbstractRetryingPeerTask.java | 26 +++++++++---------- .../AbstractRetryingSwitchingPeerTask.java | 4 ++- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java index d10ee358a59..b4cc0bdb164 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java @@ -116,14 +116,14 @@ protected void executeTask() { protected abstract CompletableFuture executePeerTask(Optional assignedPeer); protected void handleTaskError(final Throwable error) { - final Throwable cause = ExceptionUtils.rootCause(error); - if (!isRetryableError(cause)) { + final Throwable rootCause = ExceptionUtils.rootCause(error); + if (!isRetryableError(rootCause)) { // Complete exceptionally - result.completeExceptionally(cause); + result.completeExceptionally(rootCause); return; } - if (cause instanceof NoAvailablePeersException) { + if (rootCause instanceof NoAvailablePeersException) { LOG.debug( "No useful peer found, wait max 5 seconds for new peer to connect: current peers {}", ethContext.getEthPeers().peerCount()); @@ -141,7 +141,7 @@ protected void handleTaskError(final Throwable error) { LOG.debug( "Retrying after recoverable failure from peer task {}: {}", this.getClass().getSimpleName(), - cause.getMessage()); + rootCause.getMessage()); // Wait before retrying on failure executeSubTask( () -> @@ -150,16 +150,16 @@ protected void handleTaskError(final Throwable error) { .scheduleFutureTask(this::executeTaskTimed, Duration.ofSeconds(1))); } - protected boolean isRetryableError(final Throwable error) { - return error instanceof IncompleteResultsException - || error instanceof TimeoutException - || (!assignedPeer.isPresent() && isPeerFailure(error)); + protected boolean isRetryableError(final Throwable rootCause) { + return rootCause instanceof IncompleteResultsException + || rootCause instanceof TimeoutException + || (!assignedPeer.isPresent() && isPeerFailure(rootCause)); } - protected boolean isPeerFailure(final Throwable error) { - return error instanceof PeerBreachedProtocolException - || error instanceof PeerDisconnectedException - || error instanceof NoAvailablePeersException; + protected boolean isPeerFailure(final Throwable rootCause) { + return rootCause instanceof PeerBreachedProtocolException + || rootCause instanceof PeerDisconnectedException + || rootCause instanceof NoAvailablePeersException; } protected EthContext getEthContext() { diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingSwitchingPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingSwitchingPeerTask.java index eb52312f5a3..76ad3e05b45 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingSwitchingPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingSwitchingPeerTask.java @@ -20,6 +20,7 @@ import org.hyperledger.besu.ethereum.eth.manager.exceptions.NoAvailablePeersException; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.messages.DisconnectMessage.DisconnectReason; import org.hyperledger.besu.plugin.services.MetricsSystem; +import org.hyperledger.besu.util.ExceptionUtils; import java.util.HashSet; import java.util.Optional; @@ -94,7 +95,8 @@ protected CompletableFuture executePeerTask(final Optional assignedP @Override protected void handleTaskError(final Throwable error) { - if (isPeerFailure(error)) { + final Throwable rootCause = ExceptionUtils.rootCause(error); + if (isPeerFailure(rootCause)) { getAssignedPeer().ifPresent(failedPeers::add); } super.handleTaskError(error); From ed2def648711519f6b9a14ed9905f13453ae342d Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Tue, 30 May 2023 16:20:22 +0200 Subject: [PATCH 08/23] Adapt unit tests Signed-off-by: Fabio Di Fabio --- .../ethtaskutils/RetryingMessageTaskTest.java | 24 +++- .../RetryingSwitchingPeerMessageTaskTest.java | 112 ++++++++++++------ .../RetryingGetBlockFromPeersTaskTest.java | 3 +- .../RetryingGetNodeDataFromPeerTaskTest.java | 15 ++- .../sync/tasks/CompleteBlocksTaskTest.java | 8 +- .../tasks/DownloadHeaderSequenceTaskTest.java | 7 +- .../tasks/GetReceiptsForHeadersTaskTest.java | 2 +- ...yingGetHeaderFromPeerByNumberTaskTest.java | 3 +- 8 files changed, 116 insertions(+), 58 deletions(-) diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/RetryingMessageTaskTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/RetryingMessageTaskTest.java index b69e316398e..7274c06c0ce 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/RetryingMessageTaskTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/RetryingMessageTaskTest.java @@ -36,7 +36,7 @@ */ public abstract class RetryingMessageTaskTest extends AbstractMessageTaskTest { protected static final int DEFAULT_MAX_RETRIES = 4; - protected int maxRetries; + private int maxRetries; @Before public void resetMaxRetries() { @@ -49,6 +49,17 @@ protected void assertResultMatchesExpectation( assertThat(response).isEqualTo(requestedData); } + @Override + protected EthTask createTask(final T requestedData) { + return createTask(requestedData, getMaxRetries()); + } + + protected abstract EthTask createTask(T requestedData, int maxRetries); + + protected int getMaxRetries() { + return maxRetries; + } + @Test public void failsWhenPeerReturnsPartialResultThenStops() { // Setup data to be requested and expected response @@ -79,7 +90,7 @@ public void failsWhenPeerReturnsPartialResultThenStops() { assertThat(future.isDone()).isFalse(); // Respond max times - 1 with no data - respondingPeer.respondTimes(emptyResponder, maxRetries - 1); + respondingPeer.respondTimes(emptyResponder, getMaxRetries() - 1); assertThat(future).isNotDone(); // Next retry should fail @@ -142,7 +153,7 @@ public void doesNotCompleteWhenPeersAreUnavailable() { // Setup data to be requested final T requestedData = generateDataToBeRequested(); - final EthTask task = createTask(requestedData); + final EthTask task = createTask(requestedData, getMaxRetries() + 1); final CompletableFuture future = task.run(); assertThat(future.isDone()).isFalse(); @@ -155,7 +166,8 @@ public void completesWhenPeersAreTemporarilyUnavailable() final T requestedData = generateDataToBeRequested(); // Execute task and wait for response - final EthTask task = createTask(requestedData); + // +2 is required for switching peers tasks where the max retries = number of peers + final EthTask task = createTask(requestedData, getMaxRetries() + 2); final CompletableFuture future = task.run(); assertThat(future.isDone()).isFalse(); @@ -182,7 +194,7 @@ public void completeWhenPeersTimeoutTemporarily() EthProtocolManagerTestUtil.createPeer(ethProtocolManager); final T requestedData = generateDataToBeRequested(); - final EthTask task = createTask(requestedData); + final EthTask task = createTask(requestedData, ethPeers.peerCount() + 1); final CompletableFuture future = task.run(); assertThat(future.isDone()).isFalse(); @@ -208,7 +220,7 @@ public void failsWhenPeersSendEmptyResponses() { assertThat(future.isDone()).isFalse(); // Respond max times - 1 - respondingPeer.respondTimes(responder, maxRetries - 1); + respondingPeer.respondTimes(responder, getMaxRetries() - 1); assertThat(future).isNotDone(); // Next retry should fail diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/RetryingSwitchingPeerMessageTaskTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/RetryingSwitchingPeerMessageTaskTest.java index e7c953f0935..9068ce0affe 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/RetryingSwitchingPeerMessageTaskTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/RetryingSwitchingPeerMessageTaskTest.java @@ -17,7 +17,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import org.hyperledger.besu.ethereum.eth.manager.EthPeer; import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManagerTestUtil; import org.hyperledger.besu.ethereum.eth.manager.RespondingEthPeer; import org.hyperledger.besu.ethereum.eth.manager.exceptions.MaxRetriesReachedException; @@ -25,7 +24,6 @@ import java.util.ArrayList; import java.util.List; -import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; @@ -37,13 +35,15 @@ * @param The type of data being requested from the network */ public abstract class RetryingSwitchingPeerMessageTaskTest extends RetryingMessageTaskTest { - protected Optional responsivePeer = Optional.empty(); @Override - protected void assertResultMatchesExpectation( - final T requestedData, final T response, final EthPeer respondingPeer) { - assertThat(response).isEqualTo(requestedData); - responsivePeer.ifPresent(rp -> assertThat(rp).isEqualByComparingTo(respondingPeer)); + protected EthTask createTask(final T requestedData) { + return createTask(requestedData, getMaxRetries()); + } + + @Override + protected int getMaxRetries() { + return ethPeers.peerCount(); } @Test @@ -70,8 +70,6 @@ public void completesWhenBestPeerEmptyAndSecondPeerIsResponsive() blockchain, protocolContext.getWorldStateArchive(), transactionPool), 2); - responsivePeer = Optional.of(secondPeer.getEthPeer()); - assertThat(future.isDone()).isTrue(); assertResultMatchesExpectation(requestedData, future.get(), secondPeer.getEthPeer()); } @@ -80,32 +78,26 @@ public void completesWhenBestPeerEmptyAndSecondPeerIsResponsive() public void completesWhenBestPeerTimeoutsAndSecondPeerIsResponsive() throws ExecutionException, InterruptedException { // Setup first unresponsive peer - final RespondingEthPeer firstPeer = - EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 10); + EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 10); // Setup second responsive peer final RespondingEthPeer secondPeer = EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 9); + // First peer timeouts + peerCountToTimeout.set(1); + // Execute task and wait for response final T requestedData = generateDataToBeRequested(); final EthTask task = createTask(requestedData); final CompletableFuture future = task.run(); - // First peer timeouts - peerCountToTimeout.set(1); - firstPeer.respondTimes( - RespondingEthPeer.blockchainResponder( - blockchain, protocolContext.getWorldStateArchive(), transactionPool), - 2); // Second peer is responsive secondPeer.respondTimes( RespondingEthPeer.blockchainResponder( blockchain, protocolContext.getWorldStateArchive(), transactionPool), 2); - responsivePeer = Optional.of(secondPeer.getEthPeer()); - assertThat(future.isDone()).isTrue(); assertResultMatchesExpectation(requestedData, future.get(), secondPeer.getEthPeer()); } @@ -113,26 +105,19 @@ public void completesWhenBestPeerTimeoutsAndSecondPeerIsResponsive() @Test public void failsWhenAllPeersFail() { // Setup first unresponsive peer - final RespondingEthPeer firstPeer = - EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 10); + EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 10); // Setup second unresponsive peer - final RespondingEthPeer secondPeer = - EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 9); + EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 9); + + // all peers timeout + peerCountToTimeout.set(2); // Execute task and wait for response final T requestedData = generateDataToBeRequested(); final EthTask task = createTask(requestedData); final CompletableFuture future = task.run(); - for (int i = 0; i < maxRetries && !future.isDone(); i++) { - // First peer is unresponsive - firstPeer.respondWhile(RespondingEthPeer.emptyResponder(), firstPeer::hasOutstandingRequests); - // Second peer is unresponsive - secondPeer.respondWhile( - RespondingEthPeer.emptyResponder(), secondPeer::hasOutstandingRequests); - } - assertThat(future.isDone()).isTrue(); assertThat(future.isCompletedExceptionally()).isTrue(); assertThatThrownBy(future::get).hasCauseInstanceOf(MaxRetriesReachedException.class); @@ -140,23 +125,74 @@ public void failsWhenAllPeersFail() { @Test public void disconnectAPeerWhenAllPeersTried() { - maxRetries = MAX_PEERS + 1; final int numPeers = MAX_PEERS; final List respondingPeers = new ArrayList<>(numPeers); for (int i = 0; i < numPeers; i++) { respondingPeers.add(EthProtocolManagerTestUtil.createPeer(ethProtocolManager, numPeers - i)); } + // all peers timeout + peerCountToTimeout.set(numPeers); + // Execute task and wait for response final T requestedData = generateDataToBeRequested(); - final EthTask task = createTask(requestedData); + final EthTask task = createTask(requestedData, MAX_PEERS + 1); task.run(); - respondingPeers.forEach( - respondingPeer -> - respondingPeer.respondWhile( - RespondingEthPeer.emptyResponder(), respondingPeer::hasOutstandingRequests)); - assertThat(respondingPeers.get(numPeers - 1).getEthPeer().isDisconnected()).isTrue(); } + + @Test + @Override + public void failsWhenPeersSendEmptyResponses() { + // Setup unresponsive peers + final RespondingEthPeer.Responder responder = RespondingEthPeer.emptyResponder(); + final RespondingEthPeer respondingPeer1 = + EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 2); + final RespondingEthPeer respondingPeer2 = + EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 1); + + // Setup data to be requested + final T requestedData = generateDataToBeRequested(); + + // Setup and run task + final EthTask task = createTask(requestedData); + final CompletableFuture future = task.run(); + + assertThat(future.isDone()).isFalse(); + + // Respond max times - 1 + respondingPeer1.respondTimes(responder, getMaxRetries() - 1); + assertThat(future).isNotDone(); + + // Next retry should fail + respondingPeer2.respond(responder); + assertThat(future).isDone(); + assertThat(future).isCompletedExceptionally(); + assertThatThrownBy(future::get).hasCauseInstanceOf(MaxRetriesReachedException.class); + } + + @Test + @Override + public void completesWhenPeersAreTemporarilyUnavailable() + throws ExecutionException, InterruptedException { + // Setup data to be requested + final T requestedData = generateDataToBeRequested(); + + // Execute task and wait for response + final EthTask task = createTask(requestedData, 2); + final CompletableFuture future = task.run(); + + assertThat(future.isDone()).isFalse(); + + // Setup a peer + final RespondingEthPeer.Responder responder = + RespondingEthPeer.blockchainResponder( + blockchain, protocolContext.getWorldStateArchive(), transactionPool); + final RespondingEthPeer respondingPeer = + EthProtocolManagerTestUtil.createPeer(ethProtocolManager); + respondingPeer.respondWhile(responder, () -> !future.isDone()); + + assertResultMatchesExpectation(requestedData, future.get(), respondingPeer.getEthPeer()); + } } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetBlockFromPeersTaskTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetBlockFromPeersTaskTest.java index 9e900287b1f..aedcf6f1290 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetBlockFromPeersTaskTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetBlockFromPeersTaskTest.java @@ -47,7 +47,8 @@ protected PeerTaskResult generateDataToBeRequested() { } @Override - protected RetryingGetBlockFromPeersTask createTask(final PeerTaskResult requestedData) { + protected RetryingGetBlockFromPeersTask createTask( + final PeerTaskResult requestedData, final int maxRetries) { return RetryingGetBlockFromPeersTask.create( protocolSchedule, ethContext, diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetNodeDataFromPeerTaskTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetNodeDataFromPeerTaskTest.java index 21a2473d852..ec634aed852 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetNodeDataFromPeerTaskTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetNodeDataFromPeerTaskTest.java @@ -21,7 +21,7 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManagerTestUtil; import org.hyperledger.besu.ethereum.eth.manager.RespondingEthPeer; -import org.hyperledger.besu.ethereum.eth.manager.ethtaskutils.RetryingMessageTaskTest; +import org.hyperledger.besu.ethereum.eth.manager.ethtaskutils.RetryingSwitchingPeerMessageTaskTest; import java.util.List; import java.util.Map; @@ -34,7 +34,8 @@ import org.junit.Ignore; import org.junit.Test; -public class RetryingGetNodeDataFromPeerTaskTest extends RetryingMessageTaskTest> { +public class RetryingGetNodeDataFromPeerTaskTest + extends RetryingSwitchingPeerMessageTaskTest> { @Override protected Map generateDataToBeRequested() { @@ -50,10 +51,11 @@ protected Map generateDataToBeRequested() { } @Override - protected EthTask> createTask(final Map requestedData) { + protected EthTask> createTask( + final Map requestedData, final int maxRetries) { final List hashes = Lists.newArrayList(requestedData.keySet()); return RetryingGetNodeDataFromPeerTask.forHashes( - ethContext, hashes, GENESIS_BLOCK_NUMBER, metricsSystem, ethPeers.peerCount()); + ethContext, hashes, GENESIS_BLOCK_NUMBER, metricsSystem, maxRetries); } @Test @@ -95,4 +97,9 @@ public void failsWhenPeerReturnsPartialResultThenStops() {} @Override @Ignore("Empty responses count as valid when requesting node data") public void failsWhenPeersSendEmptyResponses() {} + + @Test + @Override + @Ignore("Empty responses count as valid when requesting node data") + public void completesWhenBestPeerEmptyAndSecondPeerIsResponsive() {} } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/CompleteBlocksTaskTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/CompleteBlocksTaskTest.java index 2723740bd08..e2033f2ebb7 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/CompleteBlocksTaskTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/CompleteBlocksTaskTest.java @@ -73,7 +73,7 @@ protected List generateDataToBeRequested(final int nbBlock) { } @Override - protected CompleteBlocksTask createTask(final List requestedData) { + protected CompleteBlocksTask createTask(final List requestedData, final int maxRetries) { final List headersToComplete = requestedData.stream().map(Block::getHeader).collect(Collectors.toList()); return CompleteBlocksTask.forHeaders( @@ -136,7 +136,7 @@ public void shouldCreateWithdrawalsAwareEmptyBlock_whenWithdrawalsAreEnabled() { mockProtocolSchedule, ethContext, List.of(header1, header2), - maxRetries, + getMaxRetries(), new NoOpMetricsSystem()); assertThat(task.run()).isCompletedWithValue(expectedBlocks); } @@ -193,7 +193,7 @@ public void shouldCompleteBlockThatOnlyContainsWithdrawals_whenWithdrawalsAreEna mockProtocolSchedule, ethContext, List.of(header1, header2, header3), - maxRetries, + getMaxRetries(), new NoOpMetricsSystem()); final CompletableFuture> runningTask = task.run(); @@ -220,7 +220,7 @@ public void shouldReduceTheBlockSegmentSizeAfterEachRetry() { final List requestedData = generateDataToBeRequested(10); - final CompleteBlocksTask task = createTask(requestedData); + final EthTask> task = createTask(requestedData); final CompletableFuture> future = task.run(); final List messageCollector = new ArrayList<>(4); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DownloadHeaderSequenceTaskTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DownloadHeaderSequenceTaskTest.java index 9fcb5c147ee..d9f7a29e1ab 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DownloadHeaderSequenceTaskTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DownloadHeaderSequenceTaskTest.java @@ -54,7 +54,8 @@ protected List generateDataToBeRequested() { } @Override - protected EthTask> createTask(final List requestedData) { + protected EthTask> createTask( + final List requestedData, final int maxRetries) { final BlockHeader lastHeader = requestedData.get(requestedData.size() - 1); final BlockHeader referenceHeader = blockchain.getBlockHeader(lastHeader.getNumber() + 1).get(); return DownloadHeaderSequenceTask.endingAtHeader( @@ -82,7 +83,7 @@ public void failsWhenPeerReturnsOnlyReferenceHeader() { ethContext, referenceHeader, 10, - maxRetries, + getMaxRetries(), validationPolicy, metricsSystem); final CompletableFuture> future = task.run(); @@ -112,7 +113,7 @@ public void failsWhenPeerReturnsOnlySubsetOfHeaders() { ethContext, referenceHeader, 10, - maxRetries, + getMaxRetries(), validationPolicy, metricsSystem); final CompletableFuture> future = task.run(); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/GetReceiptsForHeadersTaskTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/GetReceiptsForHeadersTaskTest.java index 57d8270e04b..7beb1336309 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/GetReceiptsForHeadersTaskTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/GetReceiptsForHeadersTaskTest.java @@ -48,7 +48,7 @@ protected Map> generateDataToBeRequested() @Override protected EthTask>> createTask( - final Map> requestedData) { + final Map> requestedData, final int maxRetries) { final List headersToComplete = new ArrayList<>(requestedData.keySet()); return GetReceiptsForHeadersTask.forHeaders( ethContext, headersToComplete, maxRetries, metricsSystem); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/RetryingGetHeaderFromPeerByNumberTaskTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/RetryingGetHeaderFromPeerByNumberTaskTest.java index a63decb61a1..0ef91fbade0 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/RetryingGetHeaderFromPeerByNumberTaskTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/RetryingGetHeaderFromPeerByNumberTaskTest.java @@ -36,7 +36,8 @@ protected List generateDataToBeRequested() { } @Override - protected EthTask> createTask(final List requestedData) { + protected EthTask> createTask( + final List requestedData, final int maxRetries) { return RetryingGetHeaderFromPeerByNumberTask.forSingleNumber( protocolSchedule, ethContext, metricsSystem, PIVOT_BLOCK_NUMBER, maxRetries); } From 419e9becffd8250956d8b5e048210f9ebc428f18 Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Mon, 11 Dec 2023 17:33:10 +1000 Subject: [PATCH 09/23] fix annotation Signed-off-by: stefan.pingel@consensys.net --- .../eth/manager/task/RetryingGetNodeDataFromPeerTaskTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetNodeDataFromPeerTaskTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetNodeDataFromPeerTaskTest.java index bda0fd538dd..5ffef51a35c 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetNodeDataFromPeerTaskTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetNodeDataFromPeerTaskTest.java @@ -100,6 +100,6 @@ public void failsWhenPeersSendEmptyResponses() {} @Test @Override - @Ignore("Empty responses count as valid when requesting node data") + @Disabled("Empty responses count as valid when requesting node data") public void completesWhenBestPeerEmptyAndSecondPeerIsResponsive() {} } From 5ee3d486ec7b71fc76f464987fca6ecc659d6f76 Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Thu, 14 Dec 2023 10:50:42 +1000 Subject: [PATCH 10/23] fix peer lookup to include unfinished connections Signed-off-by: stefan.pingel@consensys.net --- .../org/hyperledger/besu/ethereum/eth/manager/EthPeers.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java index a45e7b5787f..1f0cab7e362 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java @@ -182,9 +182,8 @@ private List getIncompleteConnections(final Bytes id) { } public boolean registerDisconnect(final PeerConnection connection) { - final Bytes id = connection.getPeer().getId(); - final EthPeer peer = completeConnections.get(id); - return registerDisconnect(id, peer, connection); + final EthPeer peer = peer(connection); + return registerDisconnect(peer.getId(), peer, connection); } private boolean registerDisconnect( From b62c7a2af4001bfb9f0ec89d1489edc309177398 Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Thu, 14 Dec 2023 11:49:44 +1000 Subject: [PATCH 11/23] check can exceed in all cases Signed-off-by: stefan.pingel@consensys.net --- .../besu/ethereum/eth/manager/EthPeers.java | 24 ++++++++----------- .../p2p/peers/DefaultPeerPrivileges.java | 6 +++-- .../ethereum/p2p/peers/PeerPrivileges.java | 6 +++-- .../besu/ethereum/p2p/rlpx/RlpxAgent.java | 4 ++-- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java index 1f0cab7e362..0424def408a 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java @@ -37,7 +37,6 @@ import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.ExecutionException; import java.util.function.Predicate; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -219,12 +218,9 @@ private void abortPendingRequestsAssignedToDisconnectedPeers() { } public EthPeer peer(final PeerConnection connection) { - try { - return incompleteConnections.get( - connection, () -> completeConnections.get(connection.getPeer().getId())); - } catch (final ExecutionException e) { - throw new RuntimeException(e); - } + final EthPeer ethPeer = incompleteConnections.getIfPresent( + connection); + return ethPeer != null ? ethPeer : completeConnections.get(connection.getPeer().getId()); } public EthPeer peer(final Bytes peerId) { @@ -366,10 +362,10 @@ public Stream getAllConnections() { } public boolean shouldConnect(final Peer peer, final boolean inbound) { - if (peerCount() >= peerUpperBound) { + final Bytes id = peer.getId(); + if (peerCount() >= peerUpperBound && !canExceedPeerLimits(id) ) { return false; } - final Bytes id = peer.getId(); final EthPeer ethPeer = completeConnections.get(id); if (ethPeer != null && !ethPeer.isDisconnected()) { return false; @@ -429,8 +425,8 @@ private void ethPeerStatusExchanged(final EthPeer peer) { private int comparePeerPriorities(final EthPeer p1, final EthPeer p2) { final PeerConnection a = p1.getConnection(); final PeerConnection b = p2.getConnection(); - final boolean aCanExceedPeerLimits = canExceedPeerLimits(a); - final boolean bCanExceedPeerLimits = canExceedPeerLimits(b); + final boolean aCanExceedPeerLimits = canExceedPeerLimits(a.getPeer().getId()); + final boolean bCanExceedPeerLimits = canExceedPeerLimits(b.getPeer().getId()); if (aCanExceedPeerLimits && !bCanExceedPeerLimits) { return -1; } else if (bCanExceedPeerLimits && !aCanExceedPeerLimits) { @@ -442,11 +438,11 @@ private int comparePeerPriorities(final EthPeer p1, final EthPeer p2) { } } - private boolean canExceedPeerLimits(final PeerConnection a) { + private boolean canExceedPeerLimits(final Bytes peerId) { if (rlpxAgent == null) { - return true; + return false; } - return rlpxAgent.canExceedConnectionLimits(a.getPeer()); + return rlpxAgent.canExceedConnectionLimits(peerId); } private int compareConnectionInitiationTimes(final PeerConnection a, final PeerConnection b) { diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/peers/DefaultPeerPrivileges.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/peers/DefaultPeerPrivileges.java index 535a49f2ac6..c52b24f61cb 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/peers/DefaultPeerPrivileges.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/peers/DefaultPeerPrivileges.java @@ -14,6 +14,8 @@ */ package org.hyperledger.besu.ethereum.p2p.peers; +import org.apache.tuweni.bytes.Bytes; + public class DefaultPeerPrivileges implements PeerPrivileges { private final MaintainedPeers maintainedPeers; @@ -22,7 +24,7 @@ public DefaultPeerPrivileges(final MaintainedPeers maintainedPeers) { } @Override - public boolean canExceedConnectionLimits(final Peer peer) { - return maintainedPeers.contains(peer); + public boolean canExceedConnectionLimits(final Bytes peerId) { + return maintainedPeers.streamPeers().anyMatch(p -> p.getId().equals(peerId)); } } diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/peers/PeerPrivileges.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/peers/PeerPrivileges.java index e99b1a39729..f03d9e22bf8 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/peers/PeerPrivileges.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/peers/PeerPrivileges.java @@ -14,14 +14,16 @@ */ package org.hyperledger.besu.ethereum.p2p.peers; +import org.apache.tuweni.bytes.Bytes; + public interface PeerPrivileges { /** * If true, the given peer can connect or remain connected even if the max connection limit or the * maximum remote connection limit has been reached or exceeded. * - * @param peer The peer to be checked. + * @param peerId The id of the peer to be checked. * @return {@code true} if the peer should be allowed to connect regardless of connection limits. */ - boolean canExceedConnectionLimits(final Peer peer); + boolean canExceedConnectionLimits(final Bytes peerId); } diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/RlpxAgent.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/RlpxAgent.java index 6cf250d715b..98a1f60df37 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/RlpxAgent.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/RlpxAgent.java @@ -310,8 +310,8 @@ private CompletableFuture initiateOutboundConnection(final Peer }); } - public boolean canExceedConnectionLimits(final Peer peer) { - return peerPrivileges.canExceedConnectionLimits(peer); + public boolean canExceedConnectionLimits(final Bytes peerId) { + return peerPrivileges.canExceedConnectionLimits(peerId); } private void handleIncomingConnection(final PeerConnection peerConnection) { From a337253172d9b3ef4b3ae2d47d2a87937b51193d Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Thu, 14 Dec 2023 12:47:32 +1000 Subject: [PATCH 12/23] add debug log for disconnecting an established peer Signed-off-by: stefan.pingel@consensys.net --- .../besu/ethereum/eth/manager/EthPeers.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java index 0424def408a..9f2aa80a4f6 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java @@ -195,7 +195,11 @@ private boolean registerDisconnect( disconnectCallbacks.forEach(callback -> callback.onDisconnect(peer)); peer.handleDisconnect(); abortPendingRequestsAssignedToDisconnectedPeers(); - LOG.debug("Disconnected EthPeer {}", peer.getShortNodeId()); + if (System.currentTimeMillis() - peer.getConnection().getInitiatedAt() > 600000) { + LOG.debug("Disonnected ESTABLISHED peer {}", peer); + } else { + LOG.debug("Disconnected EthPeer {}", peer.getShortNodeId()); + } LOG.trace("Disconnected EthPeer {}", peer); } } @@ -218,9 +222,8 @@ private void abortPendingRequestsAssignedToDisconnectedPeers() { } public EthPeer peer(final PeerConnection connection) { - final EthPeer ethPeer = incompleteConnections.getIfPresent( - connection); - return ethPeer != null ? ethPeer : completeConnections.get(connection.getPeer().getId()); + final EthPeer ethPeer = incompleteConnections.getIfPresent(connection); + return ethPeer != null ? ethPeer : completeConnections.get(connection.getPeer().getId()); } public EthPeer peer(final Bytes peerId) { @@ -363,7 +366,7 @@ public Stream getAllConnections() { public boolean shouldConnect(final Peer peer, final boolean inbound) { final Bytes id = peer.getId(); - if (peerCount() >= peerUpperBound && !canExceedPeerLimits(id) ) { + if (peerCount() >= peerUpperBound && !canExceedPeerLimits(id)) { return false; } final EthPeer ethPeer = completeConnections.get(id); From bb7cbd1e66f19a653e5f166faa7a0ce8200ebfdb Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Thu, 14 Dec 2023 13:28:04 +1000 Subject: [PATCH 13/23] fix compile Signed-off-by: stefan.pingel@consensys.net --- .../besu/ethereum/eth/manager/EthPeers.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java index 9f2aa80a4f6..205410cff6f 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java @@ -464,7 +464,7 @@ private void enforceRemoteConnectionLimits() { getActivePrioritizedPeers() .filter(p -> p.getConnection().inboundInitiated()) - .filter(p -> !canExceedPeerLimits(p.getConnection())) + .filter(p -> !canExceedPeerLimits(p.getId())) .skip(maxRemotelyInitiatedConnections) .forEach( conn -> { @@ -491,7 +491,7 @@ private void enforceConnectionLimits() { .filter(p -> !p.isDisconnected()) .skip(peerUpperBound) .map(EthPeer::getConnection) - .filter(c -> !canExceedPeerLimits(c)) + .filter(c -> !canExceedPeerLimits(c.getPeer().getId())) .forEach( conn -> { LOG.trace( @@ -516,7 +516,7 @@ private long countUntrustedRemotelyInitiatedConnections() { .map(ep -> ep.getConnection()) .filter(c -> c.inboundInitiated()) .filter(c -> !c.isDisconnected()) - .filter(conn -> !canExceedPeerLimits(conn)) + .filter(conn -> !canExceedPeerLimits(conn.getPeer().getId())) .count(); } @@ -542,7 +542,7 @@ private boolean addPeerToEthPeers(final EthPeer peer) { final Bytes id = peer.getId(); if (!randomPeerPriority) { // Disconnect if too many peers - if (!canExceedPeerLimits(connection) && peerCount() >= peerUpperBound) { + if (!canExceedPeerLimits(id) && peerCount() >= peerUpperBound) { LOG.trace( "Too many peers. Disconnect connection: {}, max connections {}", connection, @@ -552,7 +552,7 @@ private boolean addPeerToEthPeers(final EthPeer peer) { } // Disconnect if too many remotely-initiated connections if (connection.inboundInitiated() - && !canExceedPeerLimits(connection) + && !canExceedPeerLimits(id) && remoteConnectionLimitReached()) { LOG.trace( "Too many remotely-initiated connections. Disconnect incoming connection: {}, maxRemote={}", From cbca4451a696a2eec035e0faa71ecfa1131a5faf Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Thu, 14 Dec 2023 17:04:43 +1000 Subject: [PATCH 14/23] fix unit test Signed-off-by: stefan.pingel@consensys.net --- .../eth/peervalidation/AbstractPeerBlockValidatorTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/peervalidation/AbstractPeerBlockValidatorTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/peervalidation/AbstractPeerBlockValidatorTest.java index 1781428531a..4ebdcb69d4b 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/peervalidation/AbstractPeerBlockValidatorTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/peervalidation/AbstractPeerBlockValidatorTest.java @@ -71,7 +71,7 @@ public void validatePeer_requestBlockFromPeerBeingTested() { final PeerValidator validator = createValidator(blockNumber, 0); - final int peerCount = 1000; + final int peerCount = 24; final List otherPeers = Stream.generate( () -> EthProtocolManagerTestUtil.createPeer(ethProtocolManager, blockNumber)) From 24a96a5b2a6c909ff5644ebc9f21f42d06492ba1 Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Fri, 15 Dec 2023 13:18:19 +1000 Subject: [PATCH 15/23] print message when not expected Signed-off-by: stefan.pingel@consensys.net --- .../org/hyperledger/besu/ethereum/eth/manager/EthPeer.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java index 4e734447035..1056e77bc9e 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java @@ -425,10 +425,11 @@ Optional dispatch(final EthMessage ethMessage, final String prot localRequestManager -> localRequestManager.dispatchResponse(ethMessage), () -> { LOG.trace( - "Message {} not expected has just been received for protocol {}, peer {} ", + "Message {} not expected has just been received for protocol {}, peer {}, data: {} ", messageCode, protocolName, - this); + this, + ethMessage.getData()); }); return requestManager; } From addcfcd9923e15b25e1e19508c68a82289296396 Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Fri, 15 Dec 2023 14:07:16 +1000 Subject: [PATCH 16/23] really print the data Signed-off-by: stefan.pingel@consensys.net --- .../java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java index 1056e77bc9e..fddaf1e007d 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java @@ -429,7 +429,7 @@ Optional dispatch(final EthMessage ethMessage, final String prot messageCode, protocolName, this, - ethMessage.getData()); + ethMessage.getData().getData()); }); return requestManager; } From 93b772a4b2cb4a514773a8f6b492f635c05f4ce7 Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Mon, 18 Dec 2023 17:59:29 +1000 Subject: [PATCH 17/23] do not report useless for got storage range Signed-off-by: stefan.pingel@consensys.net --- .../snap/RetryingGetStorageRangeFromPeerTask.java | 5 +++++ .../eth/manager/task/AbstractRetryingPeerTask.java | 10 ++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetStorageRangeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetStorageRangeFromPeerTask.java index d8fd3d8ccf6..b34658973ca 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetStorageRangeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetStorageRangeFromPeerTask.java @@ -88,6 +88,11 @@ protected boolean emptyResult(final StorageRangeMessage.SlotRangeData peerResult return peerResult.proofs().isEmpty() && peerResult.slots().isEmpty(); } + @Override + protected boolean reportUselessIfEmptyResponse() { + return false; + } + @Override protected boolean successfulResult(final StorageRangeMessage.SlotRangeData peerResult) { return !emptyResult(peerResult); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java index b4cc0bdb164..88ca6ae47eb 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java @@ -97,12 +97,14 @@ protected void executeTask() { if (successfulResult(peerResult)) { result.complete(peerResult); } else { - if (emptyResult(peerResult)) { + final boolean emptyResult = emptyResult(peerResult); + if (emptyResult && reportUselessIfEmptyResponse()) { // record this empty response, so that the peer will be disconnected if there // were too many assignedPeer.ifPresent( peer -> peer.recordUselessResponse(getClass().getSimpleName())); - } else { + } + if (!emptyResult) { // If we get a partial success, reset the retry counter retryCount = 0; } @@ -113,6 +115,10 @@ protected void executeTask() { }); } + protected boolean reportUselessIfEmptyResponse() { + return true; + } + protected abstract CompletableFuture executePeerTask(Optional assignedPeer); protected void handleTaskError(final Throwable error) { From b9265e2811c92071e112de968061eced7384b078 Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Tue, 19 Dec 2023 12:48:07 +1000 Subject: [PATCH 18/23] two more tasks should not report useless responses Signed-off-by: stefan.pingel@consensys.net --- .../manager/snap/RetryingGetAccountRangeFromPeerTask.java | 5 +++++ .../eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java index f9a1bf5cc67..d9253a69867 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java @@ -77,6 +77,11 @@ protected boolean emptyResult(final AccountRangeMessage.AccountRangeData data) { return data.accounts().isEmpty() && data.proofs().isEmpty(); } + @Override + protected boolean reportUselessIfEmptyResponse() { + return false; + } + @Override protected boolean successfulResult(final AccountRangeMessage.AccountRangeData peerResult) { return !emptyResult(peerResult); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java index 6f9b6046db7..16d6a2c51f6 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java @@ -72,6 +72,11 @@ protected boolean emptyResult(final Map peerResult) { return peerResult.isEmpty(); } + @Override + protected boolean reportUselessIfEmptyResponse() { + return false; + } + @Override protected boolean successfulResult(final Map peerResult) { return !emptyResult(peerResult); From 2e2e48f292be0db628ab8e53a8b1cca58fac7f6f Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Tue, 19 Dec 2023 15:11:20 +1000 Subject: [PATCH 19/23] one more task Signed-off-by: stefan.pingel@consensys.net --- .../eth/manager/snap/RetryingGetBytecodeFromPeerTask.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetBytecodeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetBytecodeFromPeerTask.java index 09d6875071a..8c0a303238a 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetBytecodeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetBytecodeFromPeerTask.java @@ -73,6 +73,11 @@ protected boolean emptyResult(final Map peerResult) { return peerResult.isEmpty(); } + @Override + protected boolean reportUselessIfEmptyResponse() { + return false; + } + @Override protected boolean successfulResult(final Map peerResult) { return !emptyResult(peerResult); From e98213ccc3ad328b8b91eb4767953b84eda573b7 Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Tue, 19 Dec 2023 15:19:24 +1000 Subject: [PATCH 20/23] add java doc to new method Signed-off-by: stefan.pingel@consensys.net --- .../manager/task/AbstractRetryingPeerTask.java | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java index 88ca6ae47eb..94cc432aea3 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java @@ -115,10 +115,6 @@ protected void executeTask() { }); } - protected boolean reportUselessIfEmptyResponse() { - return true; - } - protected abstract CompletableFuture executePeerTask(Optional assignedPeer); protected void handleTaskError(final Throwable error) { @@ -188,10 +184,19 @@ public int getMaxRetries() { * Identify if the result is empty. * * @param peerResult the result to check - * @return true if the result is empty and the peer should be demoted and the request retried + * @return true if the result is empty and the request retried */ protected abstract boolean emptyResult(final T peerResult); + /** + * Identify if and empty response can be reported as useless + * + * @return true if an empty response can be reported useless + */ + protected boolean reportUselessIfEmptyResponse() { + return true; + } + /** * Identify a successful and complete result. Partial results that are not considered successful * should return false, so that the request is retried. This check has precedence over the {@link From 2b05f8bb39e228a565c1e2dc6fdd20a3ce9df8e9 Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Tue, 16 Jan 2024 17:12:02 +1000 Subject: [PATCH 21/23] check whether an empty response needs to be reported Signed-off-by: stefan.pingel@consensys.net --- .../org/hyperledger/besu/ethereum/eth/manager/EthPeer.java | 5 ++--- .../besu/ethereum/eth/manager/EthProtocolManager.java | 2 ++ .../manager/snap/RetryingGetAccountRangeFromPeerTask.java | 2 +- .../eth/manager/snap/RetryingGetBytecodeFromPeerTask.java | 2 +- .../manager/snap/RetryingGetStorageRangeFromPeerTask.java | 2 +- .../eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java | 2 +- .../ethereum/eth/manager/task/AbstractRetryingPeerTask.java | 2 +- 7 files changed, 9 insertions(+), 8 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java index fddaf1e007d..4e734447035 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeer.java @@ -425,11 +425,10 @@ Optional dispatch(final EthMessage ethMessage, final String prot localRequestManager -> localRequestManager.dispatchResponse(ethMessage), () -> { LOG.trace( - "Message {} not expected has just been received for protocol {}, peer {}, data: {} ", + "Message {} not expected has just been received for protocol {}, peer {} ", messageCode, protocolName, - this, - ethMessage.getData().getData()); + this); }); return requestManager; } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java index 6925df9d490..b1b449b4c8a 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java @@ -316,6 +316,8 @@ public void processMessage(final Capability cap, final Message message) { return; } + // TODO: stefan: find out whether ethMessage is a response or a request + // There are some annoying TRACE messages in the following call if this is not a response // This will handle responses ethPeers.dispatchMessage(ethPeer, ethMessage, getSupportedProtocol()); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java index d9253a69867..2f8e213cfd2 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java @@ -78,7 +78,7 @@ protected boolean emptyResult(final AccountRangeMessage.AccountRangeData data) { } @Override - protected boolean reportUselessIfEmptyResponse() { + protected boolean reportUselessIfEmptyResponse(final long currentBlockHight) { return false; } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetBytecodeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetBytecodeFromPeerTask.java index 8c0a303238a..f58d93bdd40 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetBytecodeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetBytecodeFromPeerTask.java @@ -74,7 +74,7 @@ protected boolean emptyResult(final Map peerResult) { } @Override - protected boolean reportUselessIfEmptyResponse() { + protected boolean reportUselessIfEmptyResponse(final long currentBlockHight) { return false; } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetStorageRangeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetStorageRangeFromPeerTask.java index b34658973ca..d4a3b2124a9 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetStorageRangeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetStorageRangeFromPeerTask.java @@ -89,7 +89,7 @@ protected boolean emptyResult(final StorageRangeMessage.SlotRangeData peerResult } @Override - protected boolean reportUselessIfEmptyResponse() { + protected boolean reportUselessIfEmptyResponse(final long currentBlockHight) { return false; } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java index 16d6a2c51f6..dbb253f47ac 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java @@ -73,7 +73,7 @@ protected boolean emptyResult(final Map peerResult) { } @Override - protected boolean reportUselessIfEmptyResponse() { + protected boolean reportUselessIfEmptyResponse(final long currentBlockHight) { return false; } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java index 94cc432aea3..884166b7c5f 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java @@ -193,7 +193,7 @@ public int getMaxRetries() { * * @return true if an empty response can be reported useless */ - protected boolean reportUselessIfEmptyResponse() { + protected boolean reportUselessIfEmptyResponse(final long currentBlockHight) { return true; } From b2ab6d7e28cf99fa097d3145d12a85d13bc38623 Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Wed, 17 Jan 2024 13:47:28 +1000 Subject: [PATCH 22/23] make sure peers support snap Signed-off-by: stefan.pingel@consensys.net --- .../manager/snap/RetryingGetAccountRangeFromPeerTask.java | 7 +++++++ .../eth/manager/snap/RetryingGetBytecodeFromPeerTask.java | 7 +++++++ .../manager/snap/RetryingGetStorageRangeFromPeerTask.java | 7 +++++++ .../eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java | 7 +++++++ .../manager/task/AbstractRetryingSwitchingPeerTask.java | 6 ++++++ 5 files changed, 34 insertions(+) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java index d9253a69867..6f06679dc25 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java @@ -15,6 +15,7 @@ package org.hyperledger.besu.ethereum.eth.manager.snap; import org.hyperledger.besu.ethereum.core.BlockHeader; +import org.hyperledger.besu.ethereum.eth.SnapProtocol; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask.PeerTaskResult; @@ -24,6 +25,7 @@ import org.hyperledger.besu.plugin.services.MetricsSystem; import java.util.concurrent.CompletableFuture; +import java.util.function.Predicate; import org.apache.tuweni.bytes.Bytes32; @@ -86,4 +88,9 @@ protected boolean reportUselessIfEmptyResponse() { protected boolean successfulResult(final AccountRangeMessage.AccountRangeData peerResult) { return !emptyResult(peerResult); } + + @Override + protected Predicate getPeerFilter() { + return (peer) -> peer.getConnection().getAgreedCapabilities().stream().anyMatch( (c) -> c.getName().equals(SnapProtocol.NAME)); + } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetBytecodeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetBytecodeFromPeerTask.java index 8c0a303238a..74fe28fda34 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetBytecodeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetBytecodeFromPeerTask.java @@ -15,6 +15,7 @@ package org.hyperledger.besu.ethereum.eth.manager.snap; import org.hyperledger.besu.ethereum.core.BlockHeader; +import org.hyperledger.besu.ethereum.eth.SnapProtocol; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask.PeerTaskResult; @@ -25,6 +26,7 @@ import java.util.List; import java.util.Map; import java.util.concurrent.CompletableFuture; +import java.util.function.Predicate; import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.bytes.Bytes32; @@ -82,4 +84,9 @@ protected boolean reportUselessIfEmptyResponse() { protected boolean successfulResult(final Map peerResult) { return !emptyResult(peerResult); } + + @Override + protected Predicate getPeerFilter() { + return (peer) -> peer.getConnection().getAgreedCapabilities().stream().anyMatch( (c) -> c.getName().equals(SnapProtocol.NAME)); + } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetStorageRangeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetStorageRangeFromPeerTask.java index b34658973ca..db031335cbd 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetStorageRangeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetStorageRangeFromPeerTask.java @@ -15,6 +15,7 @@ package org.hyperledger.besu.ethereum.eth.manager.snap; import org.hyperledger.besu.ethereum.core.BlockHeader; +import org.hyperledger.besu.ethereum.eth.SnapProtocol; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask.PeerTaskResult; @@ -25,6 +26,7 @@ import java.util.List; import java.util.concurrent.CompletableFuture; +import java.util.function.Predicate; import org.apache.tuweni.bytes.Bytes32; @@ -97,4 +99,9 @@ protected boolean reportUselessIfEmptyResponse() { protected boolean successfulResult(final StorageRangeMessage.SlotRangeData peerResult) { return !emptyResult(peerResult); } + + @Override + protected Predicate getPeerFilter() { + return (peer) -> peer.getConnection().getAgreedCapabilities().stream().anyMatch( (c) -> c.getName().equals(SnapProtocol.NAME)); + } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java index 16d6a2c51f6..8ec013e1ac1 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java @@ -15,6 +15,7 @@ package org.hyperledger.besu.ethereum.eth.manager.snap; import org.hyperledger.besu.ethereum.core.BlockHeader; +import org.hyperledger.besu.ethereum.eth.SnapProtocol; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask.PeerTaskResult; @@ -25,6 +26,7 @@ import java.util.List; import java.util.Map; import java.util.concurrent.CompletableFuture; +import java.util.function.Predicate; import org.apache.tuweni.bytes.Bytes; @@ -81,4 +83,9 @@ protected boolean reportUselessIfEmptyResponse() { protected boolean successfulResult(final Map peerResult) { return !emptyResult(peerResult); } + + @Override + protected Predicate getPeerFilter() { + return (peer) -> peer.getConnection().getAgreedCapabilities().stream().anyMatch( (c) -> c.getName().equals(SnapProtocol.NAME)); + } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingSwitchingPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingSwitchingPeerTask.java index 6e1b8a64bae..84fb251b63c 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingSwitchingPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingSwitchingPeerTask.java @@ -26,6 +26,7 @@ import java.util.Optional; import java.util.Set; import java.util.concurrent.CompletableFuture; +import java.util.function.Predicate; import java.util.stream.Stream; import org.slf4j.Logger; @@ -119,9 +120,14 @@ private Stream remainingPeersToTry() { return getEthContext() .getEthPeers() .streamBestPeers() + .filter(getPeerFilter()) .filter(peer -> !triedPeers.contains(peer)); } + protected Predicate getPeerFilter() { + return (p) -> true; + } + private void refreshPeers() { final EthPeers peers = getEthContext().getEthPeers(); // If we are at max connections, then refresh peers disconnecting one of the failed peers, From 3209a37b855c9e5c2919b1deadd21e46c5ad3c1c Mon Sep 17 00:00:00 2001 From: "stefan.pingel@consensys.net" Date: Wed, 17 Jan 2024 14:07:53 +1000 Subject: [PATCH 23/23] make sure peers support snap Signed-off-by: stefan.pingel@consensys.net --- .../eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java | 4 +++- .../eth/manager/snap/RetryingGetBytecodeFromPeerTask.java | 4 +++- .../eth/manager/snap/RetryingGetStorageRangeFromPeerTask.java | 4 +++- .../eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java | 4 +++- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java index 6f06679dc25..d4dd5f0beab 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java @@ -91,6 +91,8 @@ protected boolean successfulResult(final AccountRangeMessage.AccountRangeData pe @Override protected Predicate getPeerFilter() { - return (peer) -> peer.getConnection().getAgreedCapabilities().stream().anyMatch( (c) -> c.getName().equals(SnapProtocol.NAME)); + return (peer) -> + peer.getConnection().getAgreedCapabilities().stream() + .anyMatch((c) -> c.getName().equals(SnapProtocol.NAME)); } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetBytecodeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetBytecodeFromPeerTask.java index 74fe28fda34..38068528dd4 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetBytecodeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetBytecodeFromPeerTask.java @@ -87,6 +87,8 @@ protected boolean successfulResult(final Map peerResult) { @Override protected Predicate getPeerFilter() { - return (peer) -> peer.getConnection().getAgreedCapabilities().stream().anyMatch( (c) -> c.getName().equals(SnapProtocol.NAME)); + return (peer) -> + peer.getConnection().getAgreedCapabilities().stream() + .anyMatch((c) -> c.getName().equals(SnapProtocol.NAME)); } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetStorageRangeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetStorageRangeFromPeerTask.java index db031335cbd..4e1510f1f13 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetStorageRangeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetStorageRangeFromPeerTask.java @@ -102,6 +102,8 @@ protected boolean successfulResult(final StorageRangeMessage.SlotRangeData peerR @Override protected Predicate getPeerFilter() { - return (peer) -> peer.getConnection().getAgreedCapabilities().stream().anyMatch( (c) -> c.getName().equals(SnapProtocol.NAME)); + return (peer) -> + peer.getConnection().getAgreedCapabilities().stream() + .anyMatch((c) -> c.getName().equals(SnapProtocol.NAME)); } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java index 8ec013e1ac1..fd10b9f4d44 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetTrieNodeFromPeerTask.java @@ -86,6 +86,8 @@ protected boolean successfulResult(final Map peerResult) { @Override protected Predicate getPeerFilter() { - return (peer) -> peer.getConnection().getAgreedCapabilities().stream().anyMatch( (c) -> c.getName().equals(SnapProtocol.NAME)); + return (peer) -> + peer.getConnection().getAgreedCapabilities().stream() + .anyMatch((c) -> c.getName().equals(SnapProtocol.NAME)); } }