-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Simplify libp2p ResourceMgr computed defaults (no System.StreamsInbound limits and allow more System.ConnsInbound per MB) #9587
Conversation
Reviewers @MarcoPolo and @ajnavarro : please look at all of This is not intended to be merged as is, but rather facilitate the discussion. |
This is since can't rely on go-libp2p currently.
@ajnavarro : I think this is closer to something we'd want to merge for 0.18.1. If you agree, feel free to take over and merge. |
I was thinking: 1Mb per connection isn't too much? |
Depends on the latency. If you have a 80 Mbit/s connection and a latency of 100ms, then your buffer should be about 1MiB for optimal throughput. 1MiB is how much data is inflight before being acknowledged, it’s the bandwidth-delay product. We should (and I think we already do in go-libp2p) reserve more memory as we need more buffer space. |
@ajnavarro : I have incorporated the comments and I think gotten this into a better state. (My intent was to get this conceptually right. I'm sure I have broken Go code here.). Can you please take this over to get it across the line? |
@@ -0,0 +1,36 @@ | |||
# Kubo changelog v0.18 | |||
|
|||
## v0.18.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: we dont want a separate file, let's move this to docs/changelogs/v0.18
, like we did with 0.13.1.
@ajnavarro I've already added my changes to 0.18.1
section in #9543 which is in master
now, feel free to reuse
Superseded by #9593 |
This is in response to libp2p/go-libp2p#2010 for simplification I think we want to be able to do for Kubo.
I believe will allow the resource manager to get out of the way more but still protect against script-kiddies opening many connections/streams against a node.
In order for this work, established (non-transient) connections will need register memory usage with the resource manager/accountant, ideally across all transports and muxers. Per libp2p/go-libp2p#2010 (comment), there are some cases where an established libp2p connection doesn't increase memory accounted for by the resource manager/accountant. In the meantime we can set System.ConssInbound based on ResourceMgr.MaxMemory and then strip this out when libp2p/go-libp2p#2010 lands.