Open Stebalien opened 4 years ago
The backup pool idea makes sense. The Session may also want to keep track of peers it deliberately removed from the Session (for example because they sent too many DONT_HAVEs) so it doesn't bother trying to reconnect to them.
Would it make sense to use ConnectionManager tagging to decide whether to reconnect? We would not try to reconnect to peers we did not tag.
Currently it looks like we tag a peer when we add it to the session and untag a peer when it is removed from the session. However peers are removed from the session whenever they disconnect. Should we only untag the peer if we explicitly remove it from the session?
The Session may also want to keep track of peers it deliberately removed from the Session (for example because they sent too many DONT_HAVEs) so it doesn't bother trying to reconnect to them.
There are two parts to this:
For the first class, we can just not add them to the backup set. If we find out they're actually useful later (respond to a broadcast), we can add them back.
For the second case, yeah, we'll need some way to track this. But I think that's a separate issue.
Would it make sense to use ConnectionManager tagging to decide whether to reconnect? We would not try to reconnect to peers we did not tag.
The connection manager forgets peers when they disconnect (may not be the best behavior but that's what it does). If the peer was in the session but disconnected, I'd still consider them useful and put them into the backup set.
Really, I'm just thinking about a very simple "here's a set of peers that have worked in the past, try them maybe?" pool.
Ideally, when we disconnect from a peer in a session, we'd reconnect instead of forgetting about them.
A simple approach would be to put them into some form of "backup" pool. If we run out of peers in the session, we'd try connecting to peers in the backup pool. We'd also do this whenever we randomly decide to search the DHT for more peers.
I noticed this issue when trying to bitswap with a local peer. The connection kept getting cut for other unrelated reasons but bitswap wouldn't just reconnect as I'd expect it to.