feat: support serviceslots in peermanager #631
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Jenkins BuildsClick to see older builds (55)
|
| @@ -225,6 +225,7 @@ func (c *PeerConnectionStrategy) workPublisher(ctx context.Context) { | |||
| case <-ctx.Done(): | |||
| return | |||
| case p := <-c.peerCh: | |||
| //TODO: Modify to use peermanager AddPeer | |||
There was a problem hiding this comment.
Should we create an issue for this TODO?
There was a problem hiding this comment.
This will be taken up during another peer-manager task itself, hence not required.
Just did not want to club it with service slots.
waku/v2/peermanager/peer_manager.go
Outdated
|
|
||
| // RemovePeer deletes peer from the peerStore after disconnecting it. | ||
| func (pm *PeerManager) RemovePeer(peerID peer.ID) { | ||
| // TODO: Need to handle removePeer also via peermanager |
There was a problem hiding this comment.
Should an issue be created to not forget about this?
waku/v2/peermanager/peer_manager.go
Outdated
| if proto == WakuRelayIDv200 { | ||
| if filteredPeers.Len() > 0 { | ||
| pm.logger.Info("Got peer from peerstore", zap.String("peerId", filteredPeers[0].Pretty())) | ||
| return filteredPeers[0], nil |
There was a problem hiding this comment.
Should a random peer be returned instead to match the function description?
Co-authored-by: richΛrd <[email protected]>
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
Adding discovered peers also moved to peer manager
richard-ramos
approved these changes
Aug 10, 2023
| peerIDs, ok := pm.serviceSlots[proto] | ||
| if ok || len(peerIDs) > 0 { | ||
| pm.logger.Info("Got peer from service slots", logging.HostID("peer", peerIDs[0])) | ||
| return peerIDs[0], nil |
There was a problem hiding this comment.
Maybe this should be random here as well? 🤔
There was a problem hiding this comment.
I don't see multiple static peers per service being added as of now, hence left it as is.
Anyways instead of random, at a later point we will have scoring and other params based selection.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Added concept of service slots in peer manager similar to waku-org/nwaku#1473.
Also updated Store protocol to use the new selectPeer function from peerManager.
Changes
Keeping the existing selectPeer functionality in utils package. The reason behind this is so that a user can use any of the Waku protocols without initializing a wakunode, but by just initializing a libp2p host. Documentation shall be created to document the usage of g-waku in both ways.
Tests