network: Reconnect to the same peers on startup #1844
Conversation
…st node will try to connect to the same peers
| log.Trace(fmt.Sprintf("%08x attempt to connect to bee %08x", h.BaseAddr()[:4], addr.Address()[:4])) | ||
| h.addPeer(under) | ||
| } else { | ||
| log.Warn(fmt.Sprintf("%08x unable to connect to bee %08x: invalid node URL: %v", h.BaseAddr()[:4], addr.Address()[:4], err)) |
There was a problem hiding this comment.
log.Warn("suggestInitialPeers: invalid enode ID", "self", hex.EncodeToString(h.BaseAddr()[:4], "peer", addr, "err", err)not to use fmt in log
update: I see now this is all over the original code, so TODO cleanup later.
network/hive.go
Outdated
| for _, addr := range conns { | ||
| log.Trace(fmt.Sprintf("%08x hive connect() suggested initial %08x", h.BaseAddr()[:4], addr.Address()[:4])) | ||
| under, err := enode.ParseV4(string(addr.Under())) | ||
| if err == nil { |
There was a problem hiding this comment.
it is gostyle to write like this;
under, err := enode.ParseV4(string(addr.Under()))
if err != nil {
log.Warn("suggestInitialPeers: invalid enode ID", "self", hex.EncodeToString(h.BaseAddr()[:4], "peer", addr, "err", err)
continue
}
h.addPeer(under)
}
network/hive.go
Outdated
|
|
||
| return h.Register(as...) | ||
| func (h *Hive) suggestInitialPeers(conns []*BzzAddr) { |
There was a problem hiding this comment.
this is not just suggestion like from the point of view of kademlia.
pls call it connectPeers(conns ...*BzzAddr) and the argument
| if err := h.Store.Put("peers", peers); err != nil { | ||
| return fmt.Errorf("could not save peers: %v", err) | ||
| } | ||
|
|
There was a problem hiding this comment.
for consistency, i would change line 308 to have "addrs" instead of "peers"
There was a problem hiding this comment.
That will break backward compatibility. Are we sure we want that?
network/hive.go
Outdated
| @@ -265,8 +295,22 @@ func (h *Hive) savePeers() error { | |||
| peers = append(peers, pa) | |||
| return true | |||
| }) | |||
|
|
|||
| h.Kademlia.EachConn(nil, 256, func(p *Peer, i int) bool { | |||
| if p == nil { | |||
There was a problem hiding this comment.
what's the case that we see this happening in?
There was a problem hiding this comment.
Not sure, I copied from the EachAddress call. Maybe is not possible, although if there is no connection, the root of the pot will be nil. Maybe is just a sanity check.
There was a problem hiding this comment.
I've just checked to call EachConn on an empty kademlia and nil is not returned. I also looked up all uses of EachConn and it seems we never check for nullity, so maybe I can get rid of that check.
Well, that I don't know. It seems better in most situations, we save computation time and network traffic. Maybe when the swarm node has been off for a long period, those connected peers are less probable to be still online, but it may be worth trying... |
Issue #1396
When a swarm node was restarted, kademlia connection table changed because the suggested peers were different. This incurred in a big traffic loas as the node had to push part of its chunks to the new peers.
We wanted to prioritize known peers (latest connected peers) when reconnecting. To do that, we are saving the current peers bzz addresses on shutdown, and load them in startup and try to connect them. Of course if we need more peers or some of them can't be contacted, the usual peer suggestion will take place.