Skip to content

7311: Build new peer task system with proof of concept example task implementation#7602

Closed
Matilda-Clerke wants to merge 20 commits intobesu-eth:mainfrom
Matilda-Clerke:spike-replace-peer-task-system
Closed

7311: Build new peer task system with proof of concept example task implementation#7602
Matilda-Clerke wants to merge 20 commits intobesu-eth:mainfrom
Matilda-Clerke:spike-replace-peer-task-system

Conversation

@Matilda-Clerke
Copy link
Copy Markdown
Contributor

@Matilda-Clerke Matilda-Clerke commented Sep 10, 2024

PR description

Work in progress! This is a spike for developing a new peer task system as outlined in below:

PeerTask

Represents a single specific task intended to be executed by the PeerTaskExecutor. It handles building the MessageData required for a request, as well as parsing the response MessageData into the result type T. In addition, provides a Collection of PeerTaskBehavior to instruct the PeerTaskExecutor about the desired execution behaviours of the task (e.g. retrying on a different peer).

PeerTaskExecutor

This coordinates the execution of provided tasks. From request building, to peer selection, to sending/receiving, and finally response parsing. In addition, this provides methods for asynchronous execution if desired. A PeerTasks’s PeerTaskBehaviours will instruct the PeerTaskExecutor which behaviours are allowed for the given PeerTask (e.g.

PeerManager

Manages our peers and their quality, providing specific or appropriate peers by request.

RequestManager

Manages the sending of requests and receiving of responses to and from the supplied EthPeer.

Simple Usecase Walkthrough

When we need to send a request to a peer, we start by building the appropriate subclass of PeerTask, supplying it with any additional details it may need. We then call the desired execute method on PeerTaskExecutor, for this example we’ll assume synchronous execution is used.

The PeerTaskExecutor then retrieves an appropriate peer from the PeerManager.

The PeerTaskExecutor then calls the getRequestMessage method on the PeerTask to retrieve the request MessageData.

The PeerTaskExecutor then calls the sendRequest method on the RequestManager, passing in both the EthPeer and request MessageData.

The RequestManager sends the MessageData to the supplied peer, and produces the response MessageData from the response.

The PeerTaskExecutor then calls the parseResponse method on the supplied PeerTask to translate the MessageData to the PeerTask’s response type T.

Finally, the PeerTaskExecutor wraps the T response in a PeerTaskExecutorResult, and returns it to the calling code.

Implementation Proposal

I propose implementing this on a spike branch to prove the concept. Once we’re confident in the concept, we can provide a feature toggle to allow us and users to test the new system with relative safety. Finally, once any problems are fixed and we are confident in the new system, we can remove the old system entirely and allow the new system to fully replace it.

…mplementation

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>
Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>
Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>
…ead of lowest

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>
peerManager.getPeer(
(candidatePeer) ->
isPeerUnused(candidatePeer, usedEthPeers)
&& isPeerHeightHighEnough(candidatePeer, peerTask.getRequiredBlockNumber())
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should be careful here with the required block number, as after the merge the estimated hight does not get updated very often. I think in other places we do set the required block number to 0 if we are doing PoS.

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>
Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>
Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>
Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>
…lete code

Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>
Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>
Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>
Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>
Signed-off-by: Matilda Clerke <matilda.clerke@consensys.net>
@Matilda-Clerke
Copy link
Copy Markdown
Contributor Author

Closing as this was just a spike/proof of concept. See #7628 and #7638 for proper implementation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants