fix!: remove autodialer #2639
fix!: remove autodialer #2639
Conversation
There was a problem hiding this comment.
First pass, but shouldn't we be basing this off a release-v2.0 branch? Similar to what we did with release-v1.0 ?
achingbrain
force-pushed
the
feat/remove-autodialler
branch
2 times, most recently
from
cf95a57 to
ee07e25
Compare
The autodialer is a feature from an older time when the DHT was less reliable and we didn't have things like the random walk component. There's not a lot of benefit in opening connections to any old peer, instead protocols now have better ways of targetting the kind of peers they require. Actively dialing peers harms platforms where connections are extremely expensive such as react-native so this feature has been removed. Closes #2621 BREAKING CHANGE: the autodialer has been removed as well as the corresponding config keys
achingbrain
force-pushed
the
feat/remove-autodialler
branch
from
ee07e25 to
319ddba
Compare
There was a problem hiding this comment.
Nice work 🚀 there are just a few little things to be addressed.
- The Peer Discovery docs still have a few references to
minConnections - The
autoDialIntervalis now irrelevant - We should include a migration guide essentially stating which configurations i.e. the autoDial params, have been removed and why.
|
Need to double check the reconnect on disconnect for |
| * Proactively tries to connect to known peers stored in the PeerStore. | ||
| * It will keep the number of connections below the upper limit and sort | ||
| * the peers to connect based on whether we know their keys and protocols. |
There was a problem hiding this comment.
do we have this peer sorting functionality elsewhere? anything we need to keep in here?
Also kind of related to @maschad 's original ask about potentially keeping this for some users: There may be a need to have an auto-dialer for swarm-like functionality, where you want to stay connected to a specific set of peers? Can users accomplish that without auto-dial today?
There was a problem hiding this comment.
There may be a need ... to stay connected to a specific set of peers? Can users accomplish that without auto-dial today?
Yes, tag them with KEEP_ALIVE.
|
|
||
| this.running = true | ||
|
|
||
| this.log('not enough connections %d/%d - will dial peers to increase the number of connections', numConnections, this.minConnections) |
There was a problem hiding this comment.
IIRC, we already have logic somewhere that prevents the need to dial bootstrappers if we have highly rated "close" peers in the peer store.. if it's elsewhere other than the auto-dialer, then this can all probably go away.
Question: does the auto-dialer tag/modify peers in the peer store that could change behavior for other services/components? It doesn't look like it does, but I could be missing something.
There was a problem hiding this comment.
i think the peer-discovery-bootstrap component does this..?
There was a problem hiding this comment.
does the auto-dialer tag/modify peers in the peer store
No, it only reads peers from the peer store.
IIRC, we already have logic somewhere that prevents the need to dial bootstrappers if we have highly rated "close" peers in the peer store
The notion of "close" is down to the protocol that defines it.
For example KAD-DHT already populates it's routing tables with peer store peers on startup, but it doesn't need to actively dial those peers, only during an actual query.
In the case where those peers have gone away between restarts, the KAD paper defines how to ping old peers to make sure they're still contactable during routing table maintenance.
The autodialer is a feature from an older time when the DHT was less reliable and we didn't have things like the random walk component. There's not a lot of benefit in opening connections to any old peer, instead protocols now have better ways of targetting the kind of peers they require. Actively dialing peers harms platforms where connections are extremely expensive such as react-native so this feature has been removed. Closes #2621 Fixes #2418 BREAKING CHANGE: the autodialer has been removed as well as the corresponding config keys --------- Co-authored-by: Russell Dempsey <[email protected]>
The autodialer is a feature from an older time when the DHT was less reliable and we didn't have things like the random walk component. There's not a lot of benefit in opening connections to any old peer, instead protocols now have better ways of targetting the kind of peers they require. Actively dialing peers harms platforms where connections are extremely expensive such as react-native so this feature has been removed. Closes #2621 Fixes #2418 BREAKING CHANGE: the autodialer has been removed as well as the corresponding config keys --------- Co-authored-by: Russell Dempsey <[email protected]>
The autodialer is a feature from an older time when the DHT was less reliable and we didn't have things like the random walk component. There's not a lot of benefit in opening connections to any old peer, instead protocols now have better ways of targetting the kind of peers they require. Actively dialing peers harms platforms where connections are extremely expensive such as react-native so this feature has been removed. Closes #2621 Fixes #2418 BREAKING CHANGE: the autodialer has been removed as well as the corresponding config keys --------- Co-authored-by: Russell Dempsey <[email protected]>
|
After extending autoDial, the module stopped working for me js-libp2p-pubsub-peer-discovery https://github.com/libp2p/js-libp2p-pubsub-peer-discovery How can I make the search start working? |
The autodialer is a feature from an older time when the DHT was less reliable and we didn't have things like the random walk component.
There's not a lot of benefit in opening connections to any old peer, instead protocols now have better ways of targetting the kind of peers they require.
Actively dialing peers harms platforms where connections are extremely expensive such as react-native so this feature has been removed.
Closes #2621
Fixes #2418
BREAKING CHANGE: the autodialer has been removed as well as the corresponding config keys
Change checklist