Added perPeerRateLimit env config #1580
Conversation
p2p/net/swarm/limiter.go
Outdated
| return newDialLimiterWithParams(df, fd, DefaultPerPeerRateLimit) | ||
| perPeerRateLimit := DefaultPerPeerRateLimit | ||
| if env := os.Getenv("LIBP2P_SWARM_PEER_RATE_LIMIT"); env != "" { | ||
| if n, err := strconv.ParseInt(env, 10, 32); err == nil { |
There was a problem hiding this comment.
we should at least log the error here.
Also, why the 32?
There was a problem hiding this comment.
Just to unify with the code above.
|
sorry accidental early approval; other than the log though, i think this is mostly fine. Even better to parse the envvar once in init and set the default. |
|
Not a big fan of making this configurable via environment variable. |
|
Yes, I agree with you, adding environment variables can keep the existing code unchanged for the purpose of modifying it.
|
Agreed. @millken mind making the package level global change please? It’s better since then this is exposed prominently in the pkg reference, and your code can read your env var and set the variable. |
Updated |
config/config.go
Outdated
| @@ -104,6 +104,9 @@ type Config struct { | |||
|
|
|||
| EnableHolePunching bool | |||
| HolePunchingOptions []holepunch.Option | |||
|
|
|||
| FDLimit int | |||
| PerPeerLimit int | |||
There was a problem hiding this comment.
There's no way for a libp2p user to directly set values in this config.
There was a problem hiding this comment.
Sorry, forgot to add, update again.
There was a problem hiding this comment.
I don't think we should have global config options for this. This seems overly specific.
Why don't you just add a variable, as @MarcoPolo suggested in #1580 (comment).
Ok, only change |
|
Thanks! |
Expose DefaultPerPeerRateLimit as var
Fix #1579