libp2p / js-libp2p

The JavaScript Implementation of libp2p networking stack.
https://libp2p.io
Other
2.28k stars 438 forks source link

Remove the autodialer? #2621

Open achingbrain opened 1 month ago

achingbrain commented 1 month ago

The perceived wisdom is that we need active connections to ensure connectivity etc, so we have an autodialer that ensures we always have some connections.

This is quite bad for mobile apps, since the internet connection can be intermittent, the connection churn is higher so your phone starts to get warm and your app is more likely to be killed as a resource hog.

Some protocols like gossipsub will require active connections but others such as kad-dht generally don't since they are message based (in that we connect, send a message and read the response).

If the peer has an up to date peer book with some entry points to the network it may not be necessary to maintain more connections than just to a relay to ensure dialability, for example.

This needs some experimentation but it might be enough for the bootstrap module to dial the bootstrap peers instead of "discovering" them.

Once identify has run the bootstrappers will be in the DHT routing tables so there will be a starting point for subsequent queries and there should be no need to ensure we have active connections.

maschad commented 1 month ago

I second this, although I don't think it should be removed completely, but decoupled.

Consumers like Lodestar already have their own PeerManager for this very use case, as dialling and connection maintenance logic tends to be quite application specific, and so we should allow consumers to manage them on their own.

That being said, I do think that the autodialler still has use for new consumers who have not yet thought about connection management in detail. Those consumers would use a copy/paste basic setup similar to the existing one in the config. The potential issue here though is that if we are going to decouple the autodialler so that it can be a pluggable module then the ConnectionManager would also have to become a config that's passed in.

Either way this would be a pretty breaking change so it's something that would be ideal for a v2.

achingbrain commented 1 month ago

I agree that this is a v2 thing, since we'll be removing a bunch of config.

then the ConnectionManager would also have to become a config that's passed in.

I don't think this is true. The connection manager isn't just the auto dialler, it's the dial queue, the current list of connections, and the connection pruner as well.

We've talked about making the connection pruner more configurable but not the dial queue so I don't think the whole thing will be a configurable module.

Agreed the autodialer could be converted into a service if it's required by someone.

I do think that the autodialler still has use for new consumers who have not yet thought about connection management in detail

I don't know that re-connecting to old peers is a good starting point for these users since our existing protocols all use other strategies to ensure they have connections to high-quality peers where necessary.

None of the above require a node to always have 5 or 10 or whatever connections to a peer, any peer.

Constantly dialling peers is also bad for the node's performance since chatty protocols like bitswap execute on connection and consume resources, most of the time needlessly. This hurts browsers and mobile disproportionately as their networking can be quite unstable.

Can you point to any scenarios that need arbitrary peers to continuously be reconnected to?

maschad commented 1 month ago

Can you point to any scenarios that need arbitrary peers to continuously be reconnected to?

You make a good case, and after our discussion on 23-07-2024 at the maintainers I agree that if anything, including the autodialer could be a potential foot gun, so let's remove it all together. We can include the rationale for this in changelog and migration guide.