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

chore: update go-libp2p-kad-dht #10116

Merged
merged 5 commits into from
Sep 21, 2023
Merged

chore: update go-libp2p-kad-dht #10116

merged 5 commits into from
Sep 21, 2023

Conversation

Jorropo
Copy link
Contributor

@Jorropo Jorropo commented Sep 5, 2023

No description provided.

@Jorropo Jorropo added the skip/changelog This change does NOT require a changelog entry label Sep 5, 2023
@Jorropo Jorropo requested a review from a team as a code owner September 5, 2023 15:50
@BigLep
Copy link
Contributor

BigLep commented Sep 5, 2023

2023-09-05 maintainer conversation: add changelog entry that links to probelab analysis on this in the past and can say that latency is expected to improve.

@BigLep BigLep mentioned this pull request Sep 5, 2023
14 tasks
@BigLep
Copy link
Contributor

BigLep commented Sep 6, 2023

@yiannisbot or @dennis-tra : I'd like to get in either the Kubo changelog about the expected DHT latency improvements with having multiaddrs in more responses. I assume there is past work / research / analysis we can link to. Could you please drop some link(s)? Thanks!

@BigLep
Copy link
Contributor

BigLep commented Sep 7, 2023

2023-09-07 conversation: we expect to update to a new go-libp2p-kad-dht which doesn't have the filtering fixes. If that blows up, we'll retract and use the same version as the last Kubo release.

@dennis-tra
Copy link
Contributor

@BigLep, this is the study that suggested the DHT change: https://github.com/plprobelab/network-measurements/blob/master/results/rfm17.1-sharing-prs-with-multiaddresses.md

We don't have specific numbers on the latency improvements. Conceptually, we expect a reduction in the number of secondary DHT lookups, which should substantially improve the performance.

@yiannisbot
Copy link
Member

Also worth noting that we'll be monitoring performance changes through this plot: https://probelab.io/ipfsdht/#dht-lookup-performance-overall-plot

@Jorropo
Copy link
Contributor Author

Jorropo commented Sep 18, 2023

@dennis-tra
Copy link
Contributor

That's right. Taking the impact of private addresses into account is a more recent development that came up after that study finished. That study mainly focussed on keeping Multiaddresses around for a longer period of time.

@Jorropo
Copy link
Contributor Author

Jorropo commented Sep 18, 2023

Taking the impact of private addresses into account is a more recent development that came up after that study finished. That study mainly focussed on keeping Multiaddresses around for a longer period of time.

That part is staying in, I'm backporting this.

@dennis-tra
Copy link
Contributor

What is staying in, and what are you backporting to where? I cannot follow.

@BigLep BigLep removed the skip/changelog This change does NOT require a changelog entry label Sep 20, 2023
@BigLep
Copy link
Contributor

BigLep commented Sep 20, 2023

I'm removing skip/changelog since this need a changelog.

@BigLep
Copy link
Contributor

BigLep commented Sep 20, 2023

@Jorropo : I was going to draft some changelog verbiage here, but it's not immediately clear to me what is or isn't happening in this kad-dht release vs. what Kubo had previously. Please go ahead and add.

@Jorropo Jorropo force-pushed the update-kad-dht branch 2 times, most recently from de404d3 to a362053 Compare September 21, 2023 16:34
@Jorropo Jorropo merged commit c079a09 into master Sep 21, 2023
19 checks passed
@Jorropo Jorropo deleted the update-kad-dht branch September 21, 2023 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants