-
Notifications
You must be signed in to change notification settings - Fork 820
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
7311: add peertask foundation code #7628
Changes from 17 commits
4b80016
a8d5a9f
e4be5c0
5859444
c335cbe
08c66fd
049cae2
5afba63
f2ac53e
e901fdf
6e349e1
ad86ae6
38f04ab
6de3fb3
da9cd43
ce7d245
c9eb22e
e2fda73
608fece
2d07800
ad26297
96c8030
b0f2ed0
598b519
bc25b16
e31bb70
41923d3
720f94e
7d845b3
50c26f1
b7c0c95
a81855d
8718102
e63f473
d1847f2
6d2cb95
d84520a
77ed748
0896e31
5f924c4
2865625
82cedb0
bdd96ba
c047f42
5aa6b0b
8becdb3
8186a77
37b0ec2
545fd5c
4f544f4
1c268b7
b06f38b
3c12d3d
b1c47ae
d66dd3a
fa22e93
a3f5d4a
3a68980
c422bc5
56c1f9d
e733452
b3a252b
4f9cf52
3c96eba
7daf30f
44fd3a8
ac1c4ed
7503535
ed25941
5a79636
b075a91
74995bb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
/* | ||
* Copyright contributors to Hyperledger Besu. | ||
* | ||
* 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.manager.peertask; | ||
|
||
import org.hyperledger.besu.ethereum.eth.manager.EthPeer; | ||
import org.hyperledger.besu.ethereum.p2p.peers.PeerId; | ||
|
||
import java.util.Comparator; | ||
import java.util.Map; | ||
import java.util.Optional; | ||
import java.util.concurrent.ConcurrentHashMap; | ||
import java.util.function.Predicate; | ||
|
||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
/** | ||
* This is a simple PeerSelector implementation that can be used the default implementation in most | ||
* situations | ||
*/ | ||
public class DefaultPeerSelector implements PeerSelector { | ||
private static final Logger LOG = LoggerFactory.getLogger(DefaultPeerSelector.class); | ||
|
||
private final Map<PeerId, EthPeer> ethPeersByPeerId = new ConcurrentHashMap<>(); | ||
|
||
/** | ||
* Gets the highest reputation peer matching the supplied filter | ||
* | ||
* @param filter a filter to match prospective peers with | ||
* @return the highest reputation peer matching the supplies filter | ||
* @throws NoAvailablePeerException If there are no suitable peers | ||
*/ | ||
@Override | ||
public EthPeer getPeer(final Predicate<EthPeer> filter) throws NoAvailablePeerException { | ||
LOG.trace("Finding peer from pool of {} peers", ethPeersByPeerId.size()); | ||
return ethPeersByPeerId.values().stream() | ||
.filter(filter) | ||
.max(Comparator.naturalOrder()) | ||
.orElseThrow(NoAvailablePeerException::new); | ||
} | ||
|
||
@Override | ||
public Optional<EthPeer> getPeerByPeerId(final PeerId peerId) { | ||
return Optional.ofNullable(ethPeersByPeerId.get(peerId)); | ||
} | ||
|
||
@Override | ||
public void addPeer(final EthPeer ethPeer) { | ||
ethPeersByPeerId.put(ethPeer.getConnection().getPeer(), ethPeer); | ||
} | ||
|
||
@Override | ||
public void removePeer(final PeerId peerId) { | ||
ethPeersByPeerId.remove(peerId); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
/* | ||
* Copyright contributors to Hyperledger Besu. | ||
* | ||
* 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.manager.peertask; | ||
|
||
public class InvalidPeerTaskResponseException extends Exception { | ||
|
||
public InvalidPeerTaskResponseException() { | ||
super(); | ||
} | ||
|
||
public InvalidPeerTaskResponseException(final Throwable cause) { | ||
super(cause); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
/* | ||
* Copyright contributors to Hyperledger Besu. | ||
* | ||
* 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.manager.peertask; | ||
|
||
public class NoAvailablePeerException extends Exception {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
/* | ||
* Copyright contributors to Hyperledger Besu. | ||
* | ||
* 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.manager.peertask; | ||
|
||
import org.hyperledger.besu.ethereum.eth.manager.EthPeer; | ||
import org.hyperledger.besu.ethereum.p2p.peers.PeerId; | ||
|
||
import java.util.Optional; | ||
import java.util.function.Predicate; | ||
|
||
/** Selects the EthPeers for the PeerTaskExecutor */ | ||
public interface PeerSelector { | ||
|
||
/** | ||
* Gets a peer matching the supplied filter | ||
* | ||
* @param filter a filter to match prospective peers with | ||
* @return a peer matching the supplied filter | ||
* @throws NoAvailablePeerException If there are no suitable peers | ||
*/ | ||
EthPeer getPeer(final Predicate<EthPeer> filter) throws NoAvailablePeerException; | ||
|
||
/** | ||
* Attempts to get the EthPeer identified by peerId | ||
* | ||
* @param peerId the peerId of the desired EthPeer | ||
* @return An Optional\<EthPeer\> containing the EthPeer identified by peerId if present in the | ||
* PeerSelector, or empty otherwise | ||
*/ | ||
Optional<EthPeer> getPeerByPeerId(final PeerId peerId); | ||
|
||
/** | ||
* Add the supplied EthPeer to the PeerSelector | ||
* | ||
* @param ethPeer the EthPeer to be added to the PeerSelector | ||
*/ | ||
void addPeer(final EthPeer ethPeer); | ||
|
||
/** | ||
* Remove the EthPeer identified by peerId from the PeerSelector | ||
* | ||
* @param peerId the PeerId of the EthPeer to be removed from the PeerSelector | ||
*/ | ||
void removePeer(final PeerId peerId); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
/* | ||
* Copyright contributors to Hyperledger Besu. | ||
* | ||
* 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.manager.peertask; | ||
|
||
import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData; | ||
|
||
import java.util.Collection; | ||
|
||
/** | ||
* Represents a task to be executed on an EthPeer by the PeerTaskExecutor | ||
* | ||
* @param <T> The type of the result of this PeerTask | ||
*/ | ||
public interface PeerTask<T> { | ||
/** | ||
* Returns the SubProtocol used for this PeerTask | ||
* | ||
* @return the SubProtocol used for this PeerTask | ||
*/ | ||
String getSubProtocol(); | ||
|
||
/** | ||
* Gets the minimum required block number for a peer to have to successfully execute this task | ||
* | ||
* @return the minimum required block number for a peer to have to successfully execute this task | ||
*/ | ||
long getRequiredBlockNumber(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: might need nice to combine these into a "prerequisites" object. Feels like details leaking into the interface There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a musing...what if we pass a peer into the task's prerequisite filter? e.g. "here's my set of peers sorted by rep, choose the first one that matches my task's criteria." Currently feels like peer selection logic is spread out across the Manager, Executor and Task |
||
|
||
/** | ||
* Gets the request data to send to the EthPeer | ||
* | ||
* @return the request data to send to the EthPeer | ||
*/ | ||
MessageData getRequestMessage(); | ||
|
||
/** | ||
* Parses the MessageData response from the EthPeer | ||
* | ||
* @param messageData the response MessageData to be parsed | ||
* @return a T built from the response MessageData | ||
* @throws InvalidPeerTaskResponseException if the response messageData is invalid | ||
*/ | ||
T parseResponse(MessageData messageData) throws InvalidPeerTaskResponseException; | ||
|
||
/** | ||
* Gets the Collection of behaviors this task is expected to exhibit in the PeetTaskExecutor | ||
* | ||
* @return the Collection of behaviors this task is expected to exhibit in the PeetTaskExecutor | ||
*/ | ||
Collection<PeerTaskBehavior> getPeerTaskBehaviors(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Think a set would make more sense, don't think it makes sense to have duplicate behaviours There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep true |
||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,20 @@ | ||||||
/* | ||||||
* Copyright contributors to Hyperledger Besu. | ||||||
* | ||||||
* 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.manager.peertask; | ||||||
|
||||||
public enum PeerTaskBehavior { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
RETRY_WITH_SAME_PEER, | ||||||
RETRY_WITH_OTHER_PEERS | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this also include the non-retry single try behaviour if the intent is to include all the current peer behaviours? Otherwise, like @siladu mentioned, this should be the PeerTaskRetryBehavior There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-retry behaviour is just the absence of retry behaviours. I wanted to leave this as PeerTaskBehavior to allow for other behaviors to be added later. They aren't strictly just for describing desired retry behavior, but there aren't currently any other behaviors. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think if the goal is for the code to be more readable, I would go with more specific/descriptive naming and let whoever may (or may not) add more behaviours in later worry about the appropriate name then. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alright, that makes sense |
||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the stronger
SubProtocol
type be used here? Not sure how much of hassle it is practice thoughThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Iirc I looked at using the SubProtocol type and decided against it. I can't remember why, so I'll take another look.