From 497faef83342b635dc2404146c7a3cc03412780d Mon Sep 17 00:00:00 2001 From: Stefan Pingel <16143240+pinges@users.noreply.github.com> Date: Wed, 31 Jan 2024 15:40:04 +1000 Subject: [PATCH] Disconnect worst peer (#6443) * When refreshing, only disconnect a peer if we have max peers * If we are disconnecting a peer, disconnect the least useful peer Signed-off-by: stefan.pingel@consensys.net Signed-off-by: Stefan Pingel <16143240+pinges@users.noreply.github.com> Co-authored-by: Sally MacFarlane --- .../org/hyperledger/besu/RunnerBuilder.java | 2 +- .../besu/ethereum/eth/manager/EthPeers.java | 8 ++++++-- .../AbstractRetryingSwitchingPeerTask.java | 9 ++++----- .../p2p/network/DefaultP2PNetwork.java | 15 +++++++-------- .../besu/ethereum/p2p/rlpx/RlpxAgent.java | 18 +++++++++--------- 5 files changed, 27 insertions(+), 25 deletions(-) diff --git a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java index bb3172b7dee..8d0a693e27f 100644 --- a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java @@ -690,7 +690,7 @@ public Runner build() { .timestampForks(besuController.getGenesisConfigOptions().getForkBlockTimestamps()) .allConnectionsSupplier(ethPeers::getAllConnections) .allActiveConnectionsSupplier(ethPeers::getAllActiveConnections) - .peersLowerBound(ethPeers.getPeerLowerBound()) + .maxPeers(ethPeers.getMaxPeers()) .build(); }; 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 ade4cc92c2f..a24e892e3f5 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 @@ -54,10 +54,14 @@ public class EthPeers { private static final Logger LOG = LoggerFactory.getLogger(EthPeers.class); public static final Comparator TOTAL_DIFFICULTY = - Comparator.comparing(((final EthPeer p) -> p.chainState().getEstimatedTotalDifficulty())); + Comparator.comparing((final EthPeer p) -> p.chainState().getEstimatedTotalDifficulty()); public static final Comparator CHAIN_HEIGHT = - Comparator.comparing(((final EthPeer p) -> p.chainState().getEstimatedHeight())); + Comparator.comparing((final EthPeer p) -> p.chainState().getEstimatedHeight()); + + public static final Comparator MOST_USEFUL_PEER = + Comparator.comparing((final EthPeer p) -> p.getReputation().getScore()) + .thenComparing(CHAIN_HEIGHT); public static final Comparator HEAVIEST_CHAIN = TOTAL_DIFFICULTY.thenComparing(CHAIN_HEIGHT); 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 8116042901b..7db355646f0 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 @@ -137,15 +137,14 @@ private void refreshPeers() { // or the least useful if (peers.peerCount() >= peers.getMaxPeers()) { - failedPeers.stream() - .filter(peer -> !peer.isDisconnected()) - .findAny() - .or(() -> peers.streamAvailablePeers().min(peers.getBestChainComparator())) + failedPeers.stream().filter(peer -> !peer.isDisconnected()).findAny().stream() + .min(EthPeers.MOST_USEFUL_PEER) + .or(() -> peers.streamAvailablePeers().min(EthPeers.MOST_USEFUL_PEER)) .ifPresent( peer -> { LOG.atDebug() .setMessage( - "Refresh peers disconnecting peer {}... Waiting for better peers. Current {} of max {}") + "Refresh peers disconnecting peer {} Waiting for better peers. Current {} of max {}") .addArgument(peer::getLoggableId) .addArgument(peers::peerCount) .addArgument(peers::getMaxPeers) diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/DefaultP2PNetwork.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/DefaultP2PNetwork.java index 11352b38cc7..eee530268db 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/DefaultP2PNetwork.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/DefaultP2PNetwork.java @@ -189,10 +189,9 @@ public class DefaultP2PNetwork implements P2PNetwork { this.peerPermissions = peerPermissions; this.vertx = vertx; - // set the requirement here that the number of peers be greater than the lower bound - final int peerLowerBound = rlpxAgent.getPeerLowerBound(); - LOG.debug("setting peerLowerBound {}", peerLowerBound); - peerDiscoveryAgent.addPeerRequirement(() -> rlpxAgent.getConnectionCount() >= peerLowerBound); + final int maxPeers = rlpxAgent.getMaxPeers(); + LOG.debug("setting maxPeers {}", maxPeers); + peerDiscoveryAgent.addPeerRequirement(() -> rlpxAgent.getConnectionCount() >= maxPeers); subscribeDisconnect(reputationManager); } @@ -512,7 +511,7 @@ public static class Builder { private boolean legacyForkIdEnabled = false; private Supplier> allConnectionsSupplier; private Supplier> allActiveConnectionsSupplier; - private int peersLowerBound; + private int maxPeers; private PeerTable peerTable; public P2PNetwork build() { @@ -593,7 +592,7 @@ private RlpxAgent createRlpxAgent( .p2pTLSConfiguration(p2pTLSConfiguration) .allConnectionsSupplier(allConnectionsSupplier) .allActiveConnectionsSupplier(allActiveConnectionsSupplier) - .peersLowerBound(peersLowerBound) + .maxPeers(maxPeers) .peerTable(peerTable) .build(); } @@ -710,8 +709,8 @@ public Builder allActiveConnectionsSupplier( return this; } - public Builder peersLowerBound(final int peersLowerBound) { - this.peersLowerBound = peersLowerBound; + public Builder maxPeers(final int maxPeers) { + this.maxPeers = maxPeers; return this; } } 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 4a8e227d3d5..97ccb6ef4c5 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 @@ -71,7 +71,7 @@ public class RlpxAgent { private final PeerPrivileges peerPrivileges; private final AtomicBoolean started = new AtomicBoolean(false); private final AtomicBoolean stopped = new AtomicBoolean(false); - private final int lowerBound; + private final int maxPeers; private final Supplier> allConnectionsSupplier; private final Supplier> allActiveConnectionsSupplier; private final Cache> peersConnectingCache = @@ -87,7 +87,7 @@ private RlpxAgent( final ConnectionInitializer connectionInitializer, final PeerRlpxPermissions peerPermissions, final PeerPrivileges peerPrivileges, - final int peersLowerBound, + final int maxPeers, final Supplier> allConnectionsSupplier, final Supplier> allActiveConnectionsSupplier) { this.localNode = localNode; @@ -95,7 +95,7 @@ private RlpxAgent( this.connectionInitializer = connectionInitializer; this.peerPermissions = peerPermissions; this.peerPrivileges = peerPrivileges; - this.lowerBound = peersLowerBound; + this.maxPeers = maxPeers; this.allConnectionsSupplier = allConnectionsSupplier; this.allActiveConnectionsSupplier = allActiveConnectionsSupplier; } @@ -358,8 +358,8 @@ public ConcurrentMap> getMapOfCompletab return peersConnectingCache.asMap(); } - public int getPeerLowerBound() { - return lowerBound; + public int getMaxPeers() { + return maxPeers; } public static class Builder { @@ -374,7 +374,7 @@ public static class Builder { private Optional p2pTLSConfiguration; private Supplier> allConnectionsSupplier; private Supplier> allActiveConnectionsSupplier; - private int peersLowerBound; + private int maxPeers; private PeerTable peerTable; private Builder() {} @@ -413,7 +413,7 @@ public RlpxAgent build() { connectionInitializer, rlpxPermissions, peerPrivileges, - peersLowerBound, + maxPeers, allConnectionsSupplier, allActiveConnectionsSupplier); } @@ -492,8 +492,8 @@ public Builder allActiveConnectionsSupplier( return this; } - public Builder peersLowerBound(final int peersLowerBound) { - this.peersLowerBound = peersLowerBound; + public Builder maxPeers(final int maxPeers) { + this.maxPeers = maxPeers; return this; }