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

Connection(peer) management for the underlying overlay #19

Open
mhchia opened this issue Jul 27, 2018 · 5 comments
Open

Connection(peer) management for the underlying overlay #19

mhchia opened this issue Jul 27, 2018 · 5 comments
Labels
code Specific to the code enhancement New feature or request later

Comments

@mhchia
Copy link
Collaborator

mhchia commented Jul 27, 2018

What is wrong?

Despite the fact that gossipsub chooses mesh peers for the overlay of each topic for us, we still need a way to control the connections of the node.

How can it be fixed

Possibly can be solved through ConnManager. Should further decide

  • Determine the max size of the connections
  • Should we control the size of Peerstore?

Related issue:

@mhchia mhchia added enhancement New feature or request code Specific to the code design Need design later and removed design Need design labels Jul 27, 2018
@mhchia
Copy link
Collaborator Author

mhchia commented Sep 9, 2018

Note: my thoughts to manage connections for pubsub is, if our node is currently subscribed to a specific topic, we might want it keeps its connections with its "mesh peers" in that topic, prevent the connections from being closed.

@raulk
Copy link
Contributor

raulk commented Sep 18, 2018

Hey @mhchia, evolving and maturing the connection manager in libp2p is on my personal roadmap for the next weeks/months, so I would love to chat to you about this. Currently gossipsub/floodsub attaches to the host as a Notifee and gets informed of connections and disconnections:

https://github.com/libp2p/go-floodsub/blob/e7b1fe6e75c377fda9f928f11faddcb7e5d842bf/notify.go#L18

We could consider tagging the connections of the peers on meshsub (topics we're subscribed to) to prevent them from being killed.

That said, the connection manager is a bit naïve at the moment; it simply deals with linear scores and closes connections in batches. We've started discussing some topics like protocol weights, traffic shaping, temporary allowances, connection draining, etc. here:

libp2p/go-libp2p-connmgr#19
libp2p/go-libp2p-connmgr#20

They are very early discussions, but I think you guys have a lot to contribute.

@NIC619
Copy link
Collaborator

NIC619 commented Sep 29, 2018

Hi @raulk thanks for the great introduction on connection manager. I'm currently looking into it, play around and have a few questions regarding trimming connection:

  1. cm.peers(or cm.connCount) is only updated when being notified(Connected/Disconnected) but in getConnsToClose the list of connections to be trimmed is derived from cm.peers:
    https://github.com/libp2p/go-libp2p-connmgr/blob/master/connmgr.go#L99-L101

    My observation was that this could lead to trimming connections down to below low watermark or even 0 if no Disconnected notification was made and manager keep trimming(triggered by TrimOpenConns or new connections).
    Is this scenario possible or expected?

  2. Currently gossipsub/floodsub attaches to the host as a Notifee and gets informed of connections and disconnections

    Just to make clear. Do you mean gossipsub/floodsub would notify the host about connections and disconnections or the other way around?

Thanks!

@raulk
Copy link
Contributor

raulk commented Oct 1, 2018

@NIC619 Answers:

  1. When trimming, connections are first sorted based on score. Then, only N connections are trimmed, where N is the number of connections to make the final count == lowWatermark. There's also a check above to prohibit the trimming from running altogether if the number of connections is lower than the low watermark. So I don't think the scenario you described is possible, but I may be wrong, and happy to continue looking if you still think it can happen.

  2. It's the other way around. The current implementation of gossipsub/floodsub doesn't establish connections to peers out of its own accord. It attaches a notifee to the host network, and receives callbacks as connections are opened. On each connection, it attempts to open a new stream for pubsub, and if it succeeds, the peer becomes a pubsub peer.

@NIC619
Copy link
Collaborator

NIC619 commented Oct 1, 2018

@raulk

  1. I don't know if the scenario below makes sense:
// Basically copied from `TestConnTrimming`
cm := NewConnManager(10, 20, 0)
not := cm.Notifee()

var conns []inet.Conn
for i := 0; i < 30; i++ {
	rc := randConn(t)
	conns = append(conns, rc)
	not.Connected(nil, rc)
}

// wait for a few seconds
time.Sleep(time.Second * 5)

but in this setup eventually all connections are trimmed(closed).
And if some of the peers were tagged with score > 0, their connection will remain opened.

It seems to me that cm.peers should be updated(i.e., remove the peer from cm.peers if it's connections are closed) in TrimOpenConns? But I might have missed something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code Specific to the code enhancement New feature or request later
Projects
None yet
Development

No branches or pull requests

3 participants