Skip to content
This repository has been archived by the owner on Jul 21, 2023. It is now read-only.

feat: add delay support to random walk #101

Merged
merged 3 commits into from
Apr 17, 2019
Merged

Conversation

jacobheun
Copy link
Contributor

@jacobheun jacobheun commented Apr 16, 2019

resolves #100

This adds support for delay in the random walk config. This allows random walk to be run more quickly after startup, while maintaining a longer interval.

I ran this against js-ipfs master with a randomWalk config (all options aside from enabled are default):

randomWalk: {
  enabled: true,
  interval: 300e3,
  delay: 10e3,
  timeout: 10e3
}

With an idle node, I had 100+ connected peers in under a minute. The connected peers eventually stabilized around the max peers of ~300.

logging: I also updated the randomConfig logging to make it easier to target for debugging. Instead of DEBUG=libp2p:dht and sifting through the logs, you can now have DEBUG=libp2p:dht:random-walk* and follow only random walk logs, and error logs.

Note: libp2p fix libp2p/js-libp2p#359 will need to be released before delay can be configured by js-ipfs or other projects.

@jacobheun jacobheun marked this pull request as ready for review April 17, 2019 12:24
Copy link
Contributor

@dirkmc dirkmc left a comment

Choose a reason for hiding this comment

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

LGTM 👍

If random walk is stopped, it could take several minutes before the walk function actually stops, holding open the process. Is it worth refactoring so that random walk has access to the Query that findPeer() uses so that it can call query.stop()?

Copy link
Member

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

LGTM 💪

@vasco-santos
Copy link
Member

@dirkmc I agree that it would be great to add it, as the results obtained from an ongoing query will not be relevant. I think we can add it in a new PR, as this is looking good to go. What do you guys think?

@dirkmc
Copy link
Contributor

dirkmc commented Apr 17, 2019

Agreed - I'm fine with it being done in a new PR

@vasco-santos vasco-santos merged commit 7b70fa7 into master Apr 17, 2019
@ghost ghost removed the status/in-progress In progress label Apr 17, 2019
@vasco-santos vasco-santos deleted the feat/randomWalk-delay branch April 17, 2019 13:29
@vasco-santos
Copy link
Member

Created #102 for tracking that. It should be an easy addition to the random-walk codebase

@jacobheun
Copy link
Contributor Author

Yes it's a good idea, and actually I think the existing randomWalk query logic is wrong right now. I'll look at a fix for it with the timeout update.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proposal: add trigger to randomWalk in addition to interval
3 participants