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.

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

refactor swarm dialing logic #38

Merged
merged 6 commits into from
Jun 1, 2016
Merged

refactor swarm dialing logic #38

merged 6 commits into from
Jun 1, 2016

Conversation

whyrusleeping
Copy link
Contributor

No description provided.

@whyrusleeping
Copy link
Contributor Author

A basic idea of the change:
Instead of using channels allocated in the Dial function for both (per peer, and fd consuming) types of rate limiting, we create a rateLimiter object that manages scheduling dials for all outgoing connections. It uses a single lock, and the only internal logic it does is for new dials, check if you can take a token, if so, start the dial in a goroutine, if you cant take a token right now, add the dial to a queue to be started as soon as a token is available. This is functionally equivalent to the existing implementation, except for the fact that it uses vastly fewer goroutines. This simplification in concurrency lets us more confidently use a channel of addresses as input to the dial instead of an array of them. Using a channel of addresses with the current implementation increases the locking complexity (And chance for accidental deadlocks) immensely.

@whyrusleeping
Copy link
Contributor Author

when reviewing this, i recommend looking over the changes in swarm_dial.go, particularly in dialAddrs, then take a look at the dialLimiter code afterwards. It should make more sense that way.

@whyrusleeping whyrusleeping changed the title a sketch of what redoing the swarm dialer might look like refactor swarm dialing logic Apr 21, 2016
*/

////// TEMP UNTIL PEERSTORE GETS UPGRADED
// Ref: https://github.com/ipfs/go-libp2p-peer/pull/1
Copy link
Contributor

Choose a reason for hiding this comment

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

+1 for tracking issue ❤️

@hackergrrl
Copy link
Contributor

Just a few comments and getting CI to pass -- otherwise LGTM 🐴! Lots of good godocs and thorough tests!

@whyrusleeping whyrusleeping merged commit 4a2bc51 into master Jun 1, 2016
@whyrusleeping whyrusleeping deleted the sketch/dial-redo branch June 1, 2016 18:41
marten-seemann pushed a commit that referenced this pull request Dec 20, 2021
marten-seemann pushed a commit that referenced this pull request Dec 20, 2021
* limits addresses for a peer (at most 4 chosen) - fix #39
* clears addresses before dialing back - fix #38
* global rate limit of 30 responses per (1 - 1.25 min) - fix #36
* only dial back on the source IP - fix #32
marten-seemann pushed a commit that referenced this pull request Jan 17, 2022
marten-seemann pushed a commit that referenced this pull request Apr 21, 2022
refactor swarm dialing logic
marten-seemann pushed a commit that referenced this pull request Apr 21, 2022
don't backoff dialing when the context is canceled
marten-seemann pushed a commit that referenced this pull request Apr 22, 2022
marten-seemann added a commit that referenced this pull request Apr 22, 2022
Fix missing transport parameter in dialed connection
marten-seemann pushed a commit that referenced this pull request Apr 26, 2022
Make sure we actually read and write as much data as we expect
marten-seemann added a commit that referenced this pull request May 20, 2022
stop using goprocess for shutdown
@MarcoPolo MarcoPolo mentioned this pull request Jul 7, 2022
41 tasks
marten-seemann pushed a commit that referenced this pull request Aug 9, 2022
Replace b58-encoded keys with b32 (no padding) + test against multiple ds. Resolves #38.
marten-seemann added a commit that referenced this pull request Aug 15, 2022
This happens when empty sub-scopes are moved around between scopes.
marten-seemann pushed a commit that referenced this pull request Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants