Replace dead peers & increase replacement cache size #59
Conversation
|
ping @Stebalien @aschmahmann |
table.go
Outdated
|
|
||
| if !notEmpty { | ||
| log.Infof("failed to replace missing peer=%s as all candidates were invalid", p) | ||
| } |
There was a problem hiding this comment.
Are we doing anything with the replacement peer?
There was a problem hiding this comment.
- peer to be replaced -> we use it to calculate the CPL. But, we can just pass in the CPL in the channel rather than the peer.
- The peer we replace with -> we just "validate it". The way things are wried right now, a side effect of the validation is that that the peer gets added to the RT because of the DHT connectedness notification handler. I've documented that in the
cleanupfunction. Should I document it here as well ?
|
There is a bigger issue here. If we are already connected to a candidate, we wont add it to the RT because the connected notification in the DHT will not fire for it. |
table_cleanup.go
Outdated
| i := rt.bucketIdForPeer(pinfo.Id) | ||
| if rt.buckets[i].getPeer(pinfo.Id) != nil { | ||
| log.Infof("successfully validated missing peer=%s, marking it as active", pinfo.Id) | ||
| rt.addPeer(pinfo.Id) |
There was a problem hiding this comment.
Can't we just set the peer state to active here instead of calling addPeer?
| } | ||
|
|
||
| // replaces a peer using a valid peer from the replacement cache | ||
| func (rt *RoutingTable) startPeerReplacement() { |
There was a problem hiding this comment.
Do we need another goroutine or can we just have a single "run" goroutine?
There was a problem hiding this comment.
Replacement is also requested by the DHT in addition to the cleanup routine. What do you have in mind when you say have a single "run" go-routine ?
There was a problem hiding this comment.
I mean merge the cleanup and replacement goroutines to reduce the number fo components updating the routing table. But it's not really an important change.
|
Have changed the logs to Debug. |
@Stebalien @aschmahmann
For #58.
goprocesslib completely as just context is fine here.