ConnManager should be able to create connections

[lthibault]

Abstract

ConnManager is positioned as an interface that provides "connection tracking and management interfaces for libp2p", yet it only allows implementations to prune connections. This is useful for placing an upper bound on a host's resource usage, but many applications also need to take action when there are too few connections to a host.

I propose a simple improvement to the ConnManager subpackage, which allows implementors create connections according to user-defined logic, for example if the connection count falls below a low-water mark. The proposed changes maintain backwards compatibility and are of great utility to application developers concerned with maintaining homeostasis in their clusters.

I am happy to provide a pull-request with these changes if there's any interest. Thank you in advance for your kind consideration, and for the outstanding work on this library!

Motivation

I am currently working on an application to manage computational resources in a cloud environment. This application uses go-libp2p-pubsub to periodically send cluster-wide heartbeat messages. pubsub.PubSub requires that the host already be connected to at least one other in order for the overlay network to self-assemble; even with DHT routing in place, PubSub won't create the first connection automatically.

Because connections may spontaneously drop, this opens the possibility that a Host becomes orphaned and unable to re-join the cluster. To mitigate against this, I would like to automatically create connections with randomly-selected peers (from the host's Peerstore) if the connection count drops below an arbitrary low-water mark. Ideally, this should be handled by a ConnManager, as this seems like a logical place to encapsulate connection management in general.

In order to implement the aforementioned logic, the ConnManager needs a reference to both network.Network and peerstore.Peerstore. The simplest way to achieve this is for ConnManager to hold a reference to host.Host, but this is made difficult by the fact that hosts expect their ConnManagers to be passed in as arguments to lip2p.New.

Proposal

The following solution satisfies the above constraints, without breaking backwards-compatibility, and without requiring any modifications to existing ConnManager implementations.

The first step is to add an optional interface to go-libp2p-core/connmgr:

type HostSetter interface {
    SetHost(context.Context, host.Host)
}

From there, a couple of lines need to be added to Config.NewNode exactly here:

// ...

if s, ok := cfg.ConnManager.(connmgr.HostSetter); ok {
    s.SetHost(ctx, h)
}

// ...

This allows user-defined connection-management code to run until the context expires.

Conclusion

Again, I'm more than happy to issue a PR for these changes. Please let me know if I should begin work!

I hope everyone is doing well in spite of the pandemic 😃

[Stebalien]

Unfortunately, the connection manager doesn't have enough information to do this effectively. Very rarely rarely do application simply need connections. Instead, they need connections to specific peers or peers supporting specific protocols.

For example, we have an open issue in the DHT (libp2p/go-libp2p-kad-dht#387) to bootstrap when the routing table empties. This can happen even when we have connections.

[lthibault]

the connection manager doesn't have enough information to do this effectively.

@Stebalien I'm not sure I understand. AFAICT the only missing information is a reference to the host (see the Proposal section above).

Very rarely rarely do application simply need connections. Instead, they need connections to specific peers or peers supporting specific protocols.

I'm not sure how this is at odds with the above proposal. Custom ConnManager implementations are free to connect to whomever they like.

More generally, is this a "no" ?

[Stebalien]

the connection manager doesn't have enough information to do this effectively.

@Stebalien I'm not sure I understand. AFAICT the only missing information is a reference to the host (see the Proposal section above).

My point is that a general-purpose connection manager won't know what kinds of connections services might need.

I'm not sure how this is at odds with the above proposal. Custom ConnManager implementations are free to connect to whomever they like.

I see. But in that case, you'll need to change/extend your custom connection manager every time you add/remove/swap/upgrade a service.

The only reason the connection manager is a global service is that it enforces a global limit: the maximum number of peers we can support. The minimum number of peers isn't really a global thing.

Side-note: IPFS does assume that we want to be connected to some minimum number of peers. But this is a bug that can cause an IPFS node to disconnect from the network if it happens to be connected to a bunch of nodes that don't support the DHT protocol.

---

This isn't a "no", I just want to make sure you've thought this through. I agree that giving the connection manager access to the host makes sense in general.

[lthibault]

@Stebalien Thanks very much for the clarifications!

in that case, you'll need to change/extend your custom connection manager every time you add/remove/swap/upgrade a service.

I see how this might be true, but I don't see why it must be true. Perhaps we're not quite on the same wavelength. 🤔

My problem is basically: "I want to maintain a robust connection topology between peers" with the added caveat of "I need an upper bound on the number of connections per peer". This seems like a pretty common requirement.

In order to satisfy this requirement, the application must be able to:

1. Create connections (using some arbitrary logic to select when to connect, and to whom).
2. Prune connections (again, using some arbitrary logic).

Making this the single responsibility of a connection manager (i.e.: that which manages connections) feels right.

The only reason the connection manager is a global service is that it enforces a global limit: the maximum number of peers we can support. The minimum number of peers isn't really a global thing.

In the above problem-class, it is absolutely a global thing.

We're effectively tasked with maintaining a sparse graph with strong connectivity properties. This is done to ensure message broadcast is efficient, and to ensure the cluster is resilient against failures of individual nodes. The minimum number of neighbors per node is a property of the graph (i.e.: of the cluster), so it's definitely global.

I understand there are many p2p applications that do not have this requirement, but I'll go out on a limb and say it's not rare, either. 🙂

This isn't a "no", I just want to make sure you've thought this through. I agree that giving the connection manager access to the host makes sense in general.

Cool! I'm glad the discussion is open, and your thoughtful feedback is most earnestly appreciated!

I've considered other ways of doing this, but frankly they seem hacky. At the risk of insisting, I think this is a very safe/simple change that would be extremely helpful for a sizable minority of users. That minority is basically anyone who, like me, is trying to build a distributed system on top of p2p infrastructure.

[Stebalien]

I don't want to build a footgun. I'm telling you not to do it this way because IPFS made the same mistake and will assume that it's "well connected" as long as it has 4 connections.

You may have a homogeneous network now, but you'll eventually add additional services to some nodes and disable services on others. When you do that, you'll no longer care about being "well connected" and you'll start caring about each subsystem being well connected.

The maximum number of connections is a global property because libp2p itself will stop functioning with too many connections because we'll run out of resources (file descriptors + memory).

A minimum number of connections isn't a global libp2p issue because libp2p itself doesn't need any connections to function. Specific subsystems need connections peers with specific properties:

• Pubsub needs peers subscribing to the same pubsub topic. Pubsub has an option, WithDiscovery, for automatically discovering peers for a given topic (using the provided service) when it runs low. It sounds like that's what you're looking for.
• Kademlia needs to remain connected to DHT servers. Not all nodes in the network will operate in "DHT server" mode.
• AutoRelay needs to remain connected to at least one public relay.

[lthibault]

Ok this clarifies things quite a bit - thanks for the added perspective!

You may have a homogeneous network now, but you'll eventually add additional services to some nodes and disable services on others.

Ok, I now see where you're coming from. I have a hard requirement such that no node can ever be special, but that is indeed pretty rare. In light of this, the IPFS example makes more sense, and I agree that this proposal constitutes a footgun. Please disregard. 🙂

A minimum number of connections isn't a global libp2p issue because libp2p itself doesn't need any connections to function.

I now see your point. 👍 I initially understood global vs local to mean "cluster vs node", as opposed to global program state, hence my confusion.

Pubsub needs peers subscribing to the same pubsub topic. Pubsub has an option, WithDiscovery, for automatically discovering peers for a given topic (using the provided service) when it runs low. It sounds like that's what you're looking for.

Yes, this was what I initially reached for, but I was surprised by its behavior. It seems like one initial connection is required, even when using WithDiscovery. Note that this is despite having entered peers into the host's peerstore.

Is this a bug?

[Stebalien]

I have a hard requirement such that no node can ever be special, but that is indeed pretty rare.

Ah, got it. Yeah, in that case, it makes sense. However, I'd encourage you to consider dropping that requirement if at all possible (but I understand that that's not always the case). One of the beautiful things about libp2p is that nodes can can participate in multiple networks at the same time, sharing common connections and services between networks. For example, go-ipfs would ideally be able to connect to the eth2 network to resolve ENS names as a light client (some day).

It seems like one initial connection is required, even when using WithDiscovery. Note that this is despite having entered peers into the host's peerstore.

That's libp2p/go-libp2p-kad-dht#387, unfortunately. The open PR to fix that is libp2p/go-libp2p-kad-dht#460 but we've suspended work on that issue while we fix other issues with the DHT. We plan on resuming work in a few weeks once we release libp2p/go-libp2p-kad-dht#473.

[lthibault]

I'd encourage you to consider dropping that requirement if at all possible (but I understand that that's not always the case).

Duly noted! I'll have a think, and will definitely make adjustments in that direction if at all possible. Thanks once again for sharing your experience - this kind of feedback is greatly appreciated.

That's libp2p/go-libp2p-kad-dht#387, unfortunately.

No worries! I'll just wait a few short weeks until it fixes itself. 😉

Thanks again for the assist. Closing now.