-
Notifications
You must be signed in to change notification settings - Fork 519
network: wsnet with p2p backup meshing strategy #6391
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
network: wsnet with p2p backup meshing strategy #6391
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6391 +/- ##
==========================================
- Coverage 47.05% 47.00% -0.05%
==========================================
Files 667 660 -7
Lines 88831 88852 +21
==========================================
- Hits 41798 41766 -32
- Misses 44290 44325 +35
- Partials 2743 2761 +18 ☔ View full report in Codecov by Sentry. |
127b66b to
4f7a7ca
Compare
libp2p pubsub can connect to PX streams so that we could end up with more than GossipFanout outgoing peers. This PR marks explicitly dialed peers using peerstore metadata so that each connection can be checked and vetted
4f7a7ca to
dd1293d
Compare
c47fd05 to
7187dfe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a WS-network-with-P2P-backup meshing strategy for hybrid topology networks, along with various network improvements and test fixes.
- Updated hybrid relay mesh creator to prioritize WebSocket connections and use P2P as backup to reach target connection counts
- Added validation for non-dialed outgoing P2P connections to prevent interference from sub-components like pubsub
- Updated libp2p-pubsub parameters to align with GossipFanout configuration
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
network/mesh.go |
Implements hybrid WS-priority meshing strategy with P2P backup and updates mesh function signatures |
network/wsNetwork.go |
Updates mesh functions to accept target connection count parameter and pass it through |
network/p2pNetwork.go |
Updates P2P mesh functions and adds dialed peer tracking functionality |
network/p2p/streams.go |
Adds validation to ignore non-dialed outgoing connections in stream manager |
network/p2p/pubsub.go |
Derives GossipSub parameters based on GossipFanout configuration |
network/p2p/peerstore/peerstore.go |
Renames addressData key constant to use standardized psmdk prefix |
*_test.go files |
Adds test coverage and fixes flaky tests by setting DNSBootstrapID = "" |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
for posterity, reading this PR #668 helps you understand the motivation behind the code that is being made more generic here |
cce
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Curious how well the mesher can manage connections between ws / p2p when in hybrid mode over these coarse-grained check intervals
Summary
This PR contains the following changes:
hybridRelayMeshCreatorfor relays: use WS as a primary source of peers and fallback to p2p nodesdialNodeoutgoing connects (such as made by pubsub to peers learned from PX).GossipFanoutDNSBootstrapID = "") most likely caused to non-existing DNS records lookup timeout - if it takes longer thanEventuallytimeout for connectivity assertions tests fail.addressData->psmdkAddressDatavariable rename to standardize with other keys by usingpsmdkprefix.Related: #6045
Test Plan
Added unit tests
Performance cluster tests summary: 8k TPS (as expected for hybrid), 7+1 or 6+2 (ws+p2p) outgoing connections (as expected as well).