Periodic Pinging and active fixLowPeers #533
Conversation
subscriber_notifee.go
Outdated
| @@ -125,7 +125,10 @@ func handlePeerIdentificationCompletedEvent(dht *IpfsDHT, e event.EvtPeerIdentif | |||
| return | |||
| } else if valid { | |||
| dht.peerFound(dht.ctx, e.Peer, false) | |||
| dht.fixLowPeers() | |||
| select { | |||
There was a problem hiding this comment.
nit: can we put this in a function?
dht.go
Outdated
| @@ -181,6 +188,10 @@ func New(ctx context.Context, h host.Host, options ...Option) (*IpfsDHT, error) | |||
| // go-routine to make sure we ALWAYS have RT peer addresses in the peerstore | |||
| // since RT membership is decoupled from connectivity | |||
| go dht.persistRTPeersInPeerStore() | |||
|
|
|||
| // listens to the fix low peers chan and tries to fix the Routing Table | |||
| go dht.fixLowPeersRoutine() | |||
There was a problem hiding this comment.
later: given that we're using goprocess, let's manage this with goprocess.
dht_bootstrap.go
Outdated
| for _, ps := range dht.routingTable.GetPeerInfos() { | ||
| // ping the peer if it's due for a ping and evict it if the ping fails | ||
| if time.Since(ps.LastSuccessfulOutboundQuery) > dht.maxLastSuccessfulOutboundThreshold { | ||
| livelinessCtx, cancel := context.WithTimeout(ctx, 10*time.Second) |
| // ping Routing Table peers that haven't been hear of/from in the interval they should have been. | ||
| for _, ps := range dht.routingTable.GetPeerInfos() { | ||
| // ping the peer if it's due for a ping and evict it if the ping fails | ||
| if time.Since(ps.LastSuccessfulOutboundQuery) > dht.maxLastSuccessfulOutboundThreshold { |
There was a problem hiding this comment.
We're not treating this as a successful query. I think we need to record a separate "last seen"? Otherwise, we'll keep re-connecting once the peer hits this limit.
There was a problem hiding this comment.
As discussed offline, this is not needed for now as we will probably evict flaky-connectivity peers from the DHT because they wont respond back to us with good latencies. Please see #536 for more details.
|
@Stebalien I have addressed all the review comments and am merging this because of the approval. Lmk if concerns crop up and I'll address them in a new PR. |
@aschmahmann
For #521 and makes fixLowPeers more active.
RT peer at libp2p/go-libp2p-kbucket#69.