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

feat: PeerManager scoring peers and handling connections. #668

Closed
harsh-98 opened this issue Aug 23, 2023 · 6 comments
Closed

feat: PeerManager scoring peers and handling connections. #668

harsh-98 opened this issue Aug 23, 2023 · 6 comments

Comments

@harsh-98
Copy link
Contributor

harsh-98 commented Aug 23, 2023

Background

PeerManager's responsibility is to handle peers for "/vac/waku/relay/2.0.0" topic and otherTopics in serviceSlots. It uses peerConnector for connecting to new Peers via dialPeer on basic.basichost in libp2p.
Whenever there are more inward connections than the limit, pm closes those connections via pruneInRelayConns(There is also removePeer fn in peerManager that's not used in go-waku repo, i think we missed calling it from pruneInRelayConns to remove these peers from serviceSlots and peerStore?).
Currently there is no intelligent way of selecting InPeers to close. There is a TODO in peer_manager.go to add score.

Details

host.Network() returns libp2p-swarm, which has ConnsToPeer to get all active connections to peer. Each connection has flags like Transient(closing soon), Direct(transport is proxy).
For each peer we can calculate a score such as : betterConnectionFlags

Direct: 1 InDirect:0
Transient:0  Lasting:1

                               Score: sigma(Conn.Flags)

This calculation doesn't actively fetch the transient or direct status for each connection. But uses the previously stored values for that connection, reducing computation time.

Other scoring criteria:
  • There is also Opened flag for timestamp when the connection was opened( if needed we can also add this to score calc).
  • I think transient/lasting flags' score can be more than direct/indirect flags.

Improvements for serviceSlots:

Acceptance criteria

  • add calculateScoreFn to calculate score for all inRelayPeers.
  • ServiceSlot can be data struct with map[proto]struct{sync.mutex, ids: map[peerId]bool}.
  • Remove utils.SelectRandomPeer? it is only used in peerManager and won't be needed after above changes.
@harsh-98 harsh-98 changed the title chore: PeerManager scoring peers and handling connections. feat: PeerManager scoring peers and handling connections. Aug 23, 2023
@harsh-98
Copy link
Contributor Author

harsh-98 commented Aug 24, 2023

@chaitanyaprem
Copy link
Collaborator

Whenever there are more inward connections than the limit, pm closes those connections via pruneInRelayConns(There is also removePeer fn in peerManager that's not used in go-waku repo, i think we missed calling it from pruneInRelayConns to remove these peers from serviceSlots and peerStore?).

This is intentional, as we disconnecting from peers and keeping them in peerStore are different tasks.
e.g: There are 10 connections, whereas inRelayTarget is 8. So we disconnect 2 peers and remove them from peerStore.
After few mins if we loose connections to some more peers, we won't have peers to establish additional connections to.
In order to avoid this, we are keeping the disconnected peers in peerStore so that if we have less connections we can establish connections to some of these from connectToPeers.

Regarding a more intelligent algorithm for scoring and selecting peers for disconnection, refer to earlier discussion here #594. Left it as a TODO since it is not priority right now.
Having said that, we can keep this issue open for further study and analysis.

I do like the idea of using transient and Direct flag to identify and score peers. To start with we can use betterConn as a way to decide which ones to keep.
But, we should be considering some more params based on gossipSub scoring and some performance metrics of peers like RTT, latency etc.

Good catch wrt serviceSlots issue. Go ahead raise a PR for fixing this.

Note: I would recommend not to merge multiple items in one issue as it would be hard to keep track and pick and choose to implement. Better to segregate into separate issues.

@chaitanyaprem
Copy link
Collaborator

We should also consider the origin of the peer during servicePeer selection. We should always give highest priority to the peer specified statically by a user while bringing up the node, unless there is a peer discovered dynamically which provides better service. This is what the idea of having serviceSlots also about. In nwaku they only have 1 serviceSlot, but I relaized it is better to have more than 1 peer per service and select amongst them based on some params.
Initial idea was to just use origin and select statically configured peer as first preference. As this indicates the user's preference as well.

@harsh-98
Copy link
Contributor Author

Is scoring for in-connections, both type of connections? In peerManager, only inconnections are pruned. So, scoring is only neccessary for inConnections?

@chaitanyaprem
Copy link
Collaborator

Scoring can be used in multiple places, while pruning connections(either in or out).
Selecting a service peer in order to request a service like filter/store etc.

@chaitanyaprem
Copy link
Collaborator

Descoping this from go-waku.

If there is a requirement at a later stage, it can be taken up.

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

No branches or pull requests

2 participants