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

Adjust PeerSet logic in the DHT lookup process #802

Merged

Conversation

cortze
Copy link
Contributor

@cortze cortze commented Dec 7, 2022

The current logic of the PeerSet (taking the name from the previous go-libp2p-core/peer.Set) defines the result of the lookup process based on the first AddrInfo that we discover from the lookup for each provider.

We have spotted in RFM17.1 that this logic would severely impact the effect of extending the provider Multiaddress' TTL #795, forcing in some occasions to make that second DHT lookup to map the PeerID with the Maddrs when it wouldn't be necessary because other PR Holders still have a valid Maddrs.

This PR aims to solve that weird behavior from the PeerSet by notifying as well whenever a second PR for the same provider comes with the Maddrs.

to go from a lookup result that looks like this (distribution of the content of the PRs as a result of the DHT lookups over time):

image

to something that looks like this:

image

Let me know what you think about it.

TODO: I still need to check why the modification doesn't pass the dht_test.go test.

CC: @yiannisbot @guillaumemichel @dennis-tra

@guseggert
Copy link
Contributor

@cortze Sorry this wasn't merged, do you remember if there was a reason we didn't merge this? Seems merge-able to me, if we can't find a reason then I'll just merge it.

@cortze
Copy link
Contributor Author

cortze commented Mar 13, 2023

@cortze Sorry this wasn't merged, do you remember if there was a reason we didn't merge this? Seems merge-able to me, if we can't find a reason then I'll just merge it.

AFAIK there wasn't any blocker for this PR, but I couldn't merge it myself.

@guillaumemichel guillaumemichel merged commit 0239a32 into libp2p:master Mar 13, 2023
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.

3 participants