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

fullrt dht bug fixes #719

Merged
merged 6 commits into from May 27, 2021
Merged

fullrt dht bug fixes #719

merged 6 commits into from May 27, 2021

Conversation

aschmahmann
Copy link
Contributor

@aschmahmann aschmahmann commented May 25, 2021

Building off of #717, although this PR has been updated to fix issues with #717 in addition to a few more

gammazero
gammazero previously approved these changes May 26, 2021
@gammazero gammazero dismissed their stale review May 26, 2021 01:14

Needs change

@gammazero
Copy link
Contributor

@aschmahmann I realized when I stopped initializing the timer to time.Hour, and stopped the timer instead, that the first use of if !t.Stop() { <-t.C } would deadlock, since the channel was already drained, so needed to make another change.

fullrt/dht.go Outdated Show resolved Hide resolved
fullrt/dht.go Outdated Show resolved Hide resolved
@aschmahmann aschmahmann changed the title Fix/fullrt log progress fullrt dht bug fixes May 26, 2021
@gammazero gammazero self-requested a review May 26, 2021 16:03
fullrt/dht.go Show resolved Hide resolved
fullrt/dht.go Show resolved Hide resolved
fullrt/dht_test.go Show resolved Hide resolved
fullrt/dht.go Outdated Show resolved Hide resolved
fullrt/dht.go Outdated Show resolved Hide resolved
Copy link
Contributor

@gammazero gammazero left a comment

Choose a reason for hiding this comment

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

LGTM

fullrt/dht.go Show resolved Hide resolved
aschmahmann and others added 6 commits May 27, 2021 13:27
…ppy mode

The execOnMany function was able to exit prematurely, leaving its child goroutines running.  These would write to a channel that closed after execOnMany returned in findProvidersAsyncRoutine. The function is now able to run both in a sloppy mode where it allows goroutines to be cleaned up after the function has completed as well a safer non-sloppy mode where goroutines will complete before the function returns. The sloppy mode is used for DHT "get" operations like FindProviders and SearchValues whereas the non-sloppy mode is used for "put" operations like PutValue and Provide (along with their bulk operation equivalents).

This fixes ipfs/kubo#8146
@aschmahmann aschmahmann merged commit 301a6e4 into master May 27, 2021
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.

None yet

2 participants