Skip to content
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: Build new peer task system with proof of concept example task implementation #7602

Draft
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

Matilda-Clerke
Copy link
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.

peerManager.getPeer(
(candidatePeer) ->
isPeerUnused(candidatePeer, usedEthPeers)
&& isPeerHeightHighEnough(candidatePeer, peerTask.getRequiredBlockNumber())
Copy link
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.

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