Skip to content
This repository has been archived by the owner on Apr 13, 2022. It is now read-only.

Refactor NetworkController and PeerManager actors #228

Open
mike-aksarin opened this issue Apr 13, 2018 · 3 comments
Open

Refactor NetworkController and PeerManager actors #228

mike-aksarin opened this issue Apr 13, 2018 · 3 comments

Comments

@mike-aksarin
Copy link
Contributor

NetworkController and PeerManager actors looks to be very tightly coupled and have complex mutual communications. Looks like they share same responsibility and this makes the code rather unclear.

I suggest to refactor these two actors. I suppose to make PeerManager just an local storage component for NetworkController. We still will store peerDatabase and connection state there, but will access it not as an actor, but just using local reference. All external API messages for PeerManager will go to NetworkController. We can provide an abstract superclass for them if we will need to clean NetworkController from API logics.

@ceilican
Copy link
Contributor

I generally agree with reducing the number of actors in Scorex. Currently, it seem that everything is an actor in Scorex, but this doesn't need to be so.

I tend to agree in the particular case of PeerManager as well. One concern I have is that we wouldn't want NetworkController to be blocked for too long waiting for a non-actor Peer Manager to complete an operation. But I guess that, if this concern really becomes a problem, we could solve it by using Future when calling the methods of PeerManager from NetworkController.

@mike-aksarin
Copy link
Contributor Author

Do we have heavy operations in Peer Manager that could block NetworkController if we made them single actor?

@ceilican
Copy link
Contributor

As far as I know, we don't. But I don't know everything. That is why I wanted to raise this potential issue.

@kushti, what do you think?

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

No branches or pull requests

2 participants