Skip to content
This repository has been archived by the owner on May 26, 2022. It is now read-only.

Fix hang in BackoffDiscovery.FindPeers when requesting limit lower than number of peers available #69

Merged
merged 1 commit into from
Jun 14, 2021

Conversation

iand
Copy link
Contributor

@iand iand commented May 4, 2021

Fixes #67

select {
case pch <- ai:
default:
// skip if we have asked for a lower limit than the number of peers known
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if it's ok here to return more than options.Limit values. The consumer could rely on this limit, as it's documented here:
https://github.com/libp2p/go-libp2p-core/blob/525a0b13017263bde889a3295fa2e4212d7af8c5/discovery/options.go#L35-L41

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you clarify your comment, it seems ambiguous and I am not sure if you are asking if it's ok to do something different to this code or if this code is ok as is. This code respects the limit by skipping any that exceed the number requested.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I don't understand any more what I was saying 4 weeks ago either ;)

@BigLep
Copy link

BigLep commented May 17, 2021

@iand : can you please incorporate comments so we can get this merged?

Copy link
Contributor

@marten-seemann marten-seemann left a comment

Choose a reason for hiding this comment

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

LGTM.

@BigLep
Copy link

BigLep commented Jun 14, 2021

@marten-seemann : can you please merge this and get it bubbled to go-ipfs?

@marten-seemann marten-seemann merged commit 0726f63 into libp2p:master Jun 14, 2021
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.

Possible deadlock in FindPeers
3 participants