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

fix!: do not auto-dial peers #1397

Merged
merged 3 commits into from Oct 6, 2022
Merged

Conversation

achingbrain
Copy link
Member

@achingbrain achingbrain commented Sep 28, 2022

Currently whenever a new peer is discovered we automatically dial them if we are below the connection manager connection limit.

This is incredibly resource intensive and does not necessarily result in high-quality connections (e.g. KAD close, relays or supporting protocols we are interested in).

Now that we can search the DHT for peers, this is no longer necessary so remove the functionality.

BREAKING CHANGE: the old behaviour was to dial any peer we discover, now we just add them to the peer store instead

Currently whenever a new peer is discovered we automatically dial
them if we are below the connection manager connection limit.

This is incredibly resource-intensive and does not result in high-quality
connections (e.g. KAD close, relays or supporting protocols we are
interested in).

Now that we can search the DHT for peers, this is no longer necessary
so remove the functionality.
@achingbrain achingbrain merged commit ca30192 into master Oct 6, 2022
Maintenance Priorities - JS automation moved this from In Review to Done Oct 6, 2022
@achingbrain achingbrain deleted the fix/do-not-auto-dial-peers branch October 6, 2022 13:14
@danisharora099
Copy link

danisharora099 commented Jan 16, 2023

Was this PR/commit overwritten to reverse the functionality?

Not sure what I'm missing.

cc @achingbrain

@achingbrain
Copy link
Member Author

The autodialer component ensures the node has active connections to a minimum number of peers (configured by connectionManager.minConnections) - if it drops below this the node will dial random peers from the peer store until the number of connections is above minConnections. This is to ensure reachability for things like pubsub messages, DHT queries etc.

The "auto-dial" functionality removed in this PR was that libp2p used to dial every peer it discovered - this is separate to the minConnections behaviour.

Hopefully that makes sense.

@achingbrain
Copy link
Member Author

Looks like the docs are a little out of date 😩 sorry about that!

@danisharora099
Copy link

@achingbrain I see. To confirm, libp2p still "autodials" to minConnections by default?

@achingbrain
Copy link
Member Author

Yes, as per the above, the autodialer component ensures a node keeps minConnections open.

@danisharora099
Copy link

Thanks for clarifying!

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

Successfully merging this pull request may close these issues.

None yet

3 participants