fix: Prevent Routing.Type=auto from enabling dhtserver mode if have too low of limits #9561
Conversation
ajnavarro
force-pushed
the
patch/use-dhtclient-with-few-connections
branch
from
39ba963
to
635efcb
Compare
…oo low of limits Closes #9548 Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
ajnavarro
force-pushed
the
patch/use-dhtclient-with-few-connections
branch
from
635efcb
to
3aa60c5
Compare
Jorropo
changed the title
cmd/ipfs/daemon.go
Outdated
| // Check limits to see if we have enough to run DHT as a server. | ||
| var sysConnInbound int | ||
| var sysStreamInbound int | ||
| var dhtStreamsInbound int | ||
| if cfg.Swarm.ResourceMgr.Limits != nil { |
There was a problem hiding this comment.
I don't think this is right (but maybe I have it wrong).
We want to look at the limits that were set on the resource manager (it's it's enabled). The limits that are set is the combination of computed defaults and user-supplied overrides.
There was a problem hiding this comment.
From a conversation with @Jorropo:
Jorropo: "just" reading the *config.Config object and select the DHT
Antonio: the config is not giving you the final limits
Jorropo: the scalled limits are always good
So I'm just reading the custom resource manager configuration added by the user as the result.
This validation should be done only if resourceManager is active, adding that.
| switch routingOption { | ||
| case routingOptionSupernodeKwd: | ||
| return errors.New("supernode routing was never fully implemented and has been removed") | ||
| case routingOptionDefaultKwd, routingOptionAutoKwd: | ||
| mode := dht.ModeAuto | ||
| if (sysConnInbound != 0 && sysConnInbound <= config.DefaultResourceMgrMinInboundConns) || |
There was a problem hiding this comment.
Let's not let the "zero" weirdness here in here. When we're looking at the actual limits that the resource manager is using, zero means zero.
There was a problem hiding this comment.
These are limits coming from the configuration, so zero means not set.
There was a problem hiding this comment.
Can't really do better without libp2p/go-libp2p#2000 yet.
Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
|
Per 2023-01-19 standup, we're going to focus on doing this right rather than rushing something. This means having a way to authoritatively know what limits the resource manager is actually configured with. |
|
Closing because we are changing how RM is used (limiting by memory and FDs) so this is now a low priority. #9580 |
Depends on #9555
Fixes #9548