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

Fix bug in periodic peer pinging #579

Merged
merged 5 commits into from
Apr 9, 2020
Merged

Fix bug in periodic peer pinging #579

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
82c2333
fix bug in periodic peer pinging
aarshkshah1992 Apr 9, 2020
616ef86
changes to peer mantainence logic
aarshkshah1992 Apr 9, 2020
8741e6e
changes as per review
aarshkshah1992 Apr 9, 2020
5a03734
chore: update kbucket
Stebalien Apr 9, 2020
781a61f
fix: use duration instead of floats for time
Stebalien Apr 9, 2020
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
8 changes: 4 additions & 4 deletions dht.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,9 @@ type IpfsDHT struct {
// networks).
enableProviders, enableValues bool

// maxLastSuccessfulOutboundThreshold is the max threshold/upper limit for the value of "lastSuccessfulOutboundQuery"
// of the peer in the bucket above which we will evict it to make place for a new peer if the bucket
// is full
maxLastSuccessfulOutboundThreshold time.Duration
// maxLastSuccessfulOutboundThreshold is the max threshold/upper limit on the time duration
// between the current time and the last time a peer was useful to us.
maxLastSuccessfulOutboundThreshold float64
Copy link
Contributor

Choose a reason for hiding this comment

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

why make this a float? it's only used here to compare with a time duration?

Copy link
Contributor Author

@aarshkshah1992 aarshkshah1992 Apr 9, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Because it's calculated using a formula that translates to a float64. Take a look at:

go-libp2p-kad-dht/dht.go

Line 291 in 82c2333

maxLastSuccessfulOutboundThreshold := l1 / l2 * float64(cfg.routingTable.refreshInterval)


fixLowPeersChan chan struct{}
}
Expand Down Expand Up @@ -294,6 +293,7 @@ func makeRoutingTable(dht *IpfsDHT, cfg config) (*kb.RoutingTable, error) {
self := kb.ConvertPeerID(dht.host.ID())

rt, err := kb.NewRoutingTable(cfg.bucketSize, self, time.Minute, dht.host.Peerstore(), maxLastSuccessfulOutboundThreshold)
dht.maxLastSuccessfulOutboundThreshold = maxLastSuccessfulOutboundThreshold
cmgr := dht.host.ConnManager()

rt.PeerAdded = func(p peer.ID) {
Expand Down
2 changes: 1 addition & 1 deletion dht_bootstrap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func (dht *IpfsDHT) startRefreshing() {
// 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 {
if float64(time.Since(ps.LastSuccessfulOutboundQuery)) > dht.maxLastSuccessfulOutboundThreshold {
livelinessCtx, cancel := context.WithTimeout(ctx, peerPingTimeout)
if err := dht.host.Connect(livelinessCtx, peer.AddrInfo{ID: ps.Id}); err != nil {
logger.Debugw("evicting peer after failed ping", "peer", ps.Id, "error", err)
Expand Down