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

Decouple Chain from networking #381

Closed
pipermerriam opened this issue Feb 20, 2018 · 6 comments
Closed

Decouple Chain from networking #381

pipermerriam opened this issue Feb 20, 2018 · 6 comments

Comments

@pipermerriam
Copy link
Member

What is wrong?

The evm.p2p.lightchain.LightChain extends the Chain class. This is looking to be problematic as I implement the various processes in trinity. The JSON-RPC server needs access to the Chain object. In order to do that as things are written, we'd need to setup a multiprocessing.Manager in the networking process to expose the various chain methods for the JSON-RPC process.

This is problematic because it allows for requests to the JSON-RPC server to compete with the execution of the networking processes.

How can it be fixed

I would like to create a new class type to contain the networking specific code that currently lives in the LightChain class. Maybe something like ChainNetwork or NetworkManager. This class can then interact with the Chain using a proxy object, much the same way that it interacts with the ChainDB object.

@pipermerriam
Copy link
Member Author

Alternatively, the Chain object could live within the database process itself, giving it first class access to the ChainDB. This cuts down one layer of interprocess communication which should be a good thing.

@pipermerriam
Copy link
Member Author

ping @gsalgado thoughts?

@gsalgado
Copy link
Collaborator

So, IIUC the Chain would have direct access to ChainDB, and what is currently the database process would become a chain process? Or would we still need a process to access the ChainDB directly? I think it'd be good if we could have just the chain process accessing ChainDB

@pipermerriam
Copy link
Member Author

After some experimentation yesterday, I think I've changed my stance on this, but it's still unclear what the path forward is.

Complication 1

Consider the following split.

  • Chain object in process A
  • Networking in process B
  • JSON-RPC in process C

Separating Chain and networking keeps the networking processes from getting blocked by the Chain object. Similarly, JSON-RPC needs it's own process to ensure similar guarantees to keep it from blocking the other processes.

The JSON-RPC server needs access to the Chain object to serve requests. Naive approach would be to use the multiprocessing Manager pattern to serve it over a socket like we're currently doing with the databases. This however doesn't work, because it doesn't allow us to serve JSON-RPC requests when we run an LES node because many of the JSON-RPC requests will require us to go out over the networking process to retrieve responses.

Conclusion: Whatever Chain object the JSON-RPC server uses must be able to delegate certain requests to the networking process. This indicates that having the Chain and networking code combined as they are now might be the right choice.

@pipermerriam
Copy link
Member Author

Complication 2

Assuming that we keep the Chain and networking processes together as they are now, we need a solution for the following two things.

  1. Avoid calls to Chain methods which would block and subsequently time out the networking processes. Without separating them, I don't know how we can accomplish this beyond self enforced discipline which is error prone.
  2. Expose access to the Chain to the JSON-RPC process. Current known approach using multiprocessing Manager pattern doesn't appear viable because it will allow heavy request load from the JSON-RPC server to overload the networking process, causing timeouts.

@pipermerriam
Copy link
Member Author

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

No branches or pull requests

2 participants