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.

Sign up for GitHub

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

feat: ping peers on routing table refresh #810

Merged
merged 5 commits into from
Feb 11, 2023
Merged

feat: ping peers on routing table refresh #810

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
eb6d2f7
feat: ping peers on routing table refresh
dennis-tra Feb 3, 2023
16823c3
use ping protocol instead of DHT ping
dennis-tra Feb 5, 2023
d5ea78f
extract global ping timeout constant
dennis-tra Feb 5, 2023
5e68507
Revert "use ping protocol instead of DHT ping"
dennis-tra Feb 7, 2023
62c832e
refactor: use the FIND_NODE RPC to ping peer
dennis-tra Feb 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions dht.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,10 +365,15 @@ func makeRtRefreshManager(dht *IpfsDHT, cfg dhtcfg.Config, maxLastSuccessfulOutb
return err
}

pingFnc := func(ctx context.Context, p peer.ID) error {
return dht.protoMessenger.Ping(ctx, p)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm using the ping package to probe the remote peer instead of the ping message from the /ipfs/kad/1.0.0 protocol

Not deeply familiar with the codebase. Just double checking, is this really not using the Kademlia Ping mechanism?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After yesterdays discussion, we changed it back to the Kademlia PING message. So this is indeed the Kademlia Ping.

}

r, err := rtrefresh.NewRtRefreshManager(
dht.host, dht.routingTable, cfg.RoutingTable.AutoRefresh,
keyGenFnc,
queryFnc,
pingFnc,
cfg.RoutingTable.RefreshQueryTimeout,
cfg.RoutingTable.RefreshInterval,
maxLastSuccessfulOutboundThreshold,
Expand Down
14 changes: 13 additions & 1 deletion rtrefresh/rt_refresh_manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ type RtRefreshManager struct {
enableAutoRefresh bool // should run periodic refreshes ?
refreshKeyGenFnc func(cpl uint) (string, error) // generate the key for the query to refresh this cpl
refreshQueryFnc func(ctx context.Context, key string) error // query to run for a refresh.
refreshPingFnc func(ctx context.Context, p peer.ID) error // request to check liveness of remote peer
refreshQueryTimeout time.Duration // timeout for one refresh query

// interval between two periodic refreshes.
Expand All @@ -57,6 +58,7 @@ type RtRefreshManager struct {
func NewRtRefreshManager(h host.Host, rt *kbucket.RoutingTable, autoRefresh bool,
refreshKeyGenFnc func(cpl uint) (string, error),
refreshQueryFnc func(ctx context.Context, key string) error,
refreshPingFnc func(ctx context.Context, p peer.ID) error,
refreshQueryTimeout time.Duration,
refreshInterval time.Duration,
successfulOutboundQueryGracePeriod time.Duration,
Expand All @@ -73,6 +75,7 @@ func NewRtRefreshManager(h host.Host, rt *kbucket.RoutingTable, autoRefresh bool
enableAutoRefresh: autoRefresh,
refreshKeyGenFnc: refreshKeyGenFnc,
refreshQueryFnc: refreshQueryFnc,
refreshPingFnc: refreshPingFnc,

refreshQueryTimeout: refreshQueryTimeout,
refreshInterval: refreshInterval,
Expand Down Expand Up @@ -178,12 +181,21 @@ func (r *RtRefreshManager) loop() {
wg.Add(1)
go func(ps kbucket.PeerInfo) {
defer wg.Done()

livelinessCtx, cancel := context.WithTimeout(r.ctx, peerPingTimeout)
defer cancel()

if err := r.h.Connect(livelinessCtx, peer.AddrInfo{ID: ps.Id}); err != nil {
logger.Debugw("evicting peer after failed connection", "peer", ps.Id, "error", err)
r.rt.RemovePeer(ps.Id)
return
}

if err := r.refreshPingFnc(livelinessCtx, ps.Id); err != nil {
logger.Debugw("evicting peer after failed ping", "peer", ps.Id, "error", err)
r.rt.RemovePeer(ps.Id)
return
}
cancel()
}(ps)
}
}
Expand Down