From 754c74ef26f4fdfc01d0e29c0ca4a592e5f973d3 Mon Sep 17 00:00:00 2001 From: Steve Loeppky Date: Wed, 18 Jan 2023 16:09:04 -0800 Subject: [PATCH 1/6] Clarity: no user supplied rcmgr limits of 0 The goal of this commit is to make clear that we don't support user-supplied limits of 0 currently. Apply (https://github.com/libp2p/go-libp2p/blob/master/p2p/host/resource-manager/limit.go#L102 ) overrides any 0 value. With the current code, a zero-limit in userSuppliedOverrideLimitConfig will be overridden by the computed default limits. I want to be able to do: ``` defaultComputedLimitConfig.Apply(userSuppliedOverrideLimitConfig) ``` but that won't override any of the computed defaults. I'd like to see this fixed, but this at least make clear the situation. --- core/node/libp2p/rcmgr.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/core/node/libp2p/rcmgr.go b/core/node/libp2p/rcmgr.go index 5d23a874da8..fc4c2b49159 100644 --- a/core/node/libp2p/rcmgr.go +++ b/core/node/libp2p/rcmgr.go @@ -52,7 +52,8 @@ func ResourceManager(cfg config.SwarmConfig) interface{} { return nil, opts, fmt.Errorf("opening IPFS_PATH: %w", err) } - limitConfig, err := createDefaultLimitConfig(cfg) + var limitConfig rcmgr.LimitConfig + defaultComputedLimitConfig, err := createDefaultLimitConfig(cfg) if err != nil { return nil, opts, err } @@ -61,10 +62,15 @@ func ResourceManager(cfg config.SwarmConfig) interface{} { // is documented in docs/config.md. // Any changes here should be reflected there. if cfg.ResourceMgr.Limits != nil { - l := *cfg.ResourceMgr.Limits - // This effectively overrides the computed default LimitConfig with any vlues from cfg.ResourceMgr.Limits - l.Apply(limitConfig) - limitConfig = l + userSuppliedOverrideLimitConfig := *cfg.ResourceMgr.Limits + // This effectively overrides the computed default LimitConfig with any vlues from cfg.ResourceMgr.Limits. + // Because of how how Apply works, any 0 value for a user supplied override + // will be overriden with a computed default value. + // There currently isn't a way for a user to to supply a 0-value override. + userSuppliedOverrideLimitConfig.Apply(defaultComputedLimitConfig) + limitConfig = userSuppliedOverrideLimitConfig + } else { + limitConfig = defaultComputedLimitConfig } limiter := rcmgr.NewFixedLimiter(limitConfig) From 25c31182427f9ef7bb57668d2937e57d7262b6c7 Mon Sep 17 00:00:00 2001 From: Steve Loeppky Date: Wed, 18 Jan 2023 16:11:56 -0800 Subject: [PATCH 2/6] Update rcmgr.go --- core/node/libp2p/rcmgr.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/node/libp2p/rcmgr.go b/core/node/libp2p/rcmgr.go index fc4c2b49159..5197866859a 100644 --- a/core/node/libp2p/rcmgr.go +++ b/core/node/libp2p/rcmgr.go @@ -63,7 +63,7 @@ func ResourceManager(cfg config.SwarmConfig) interface{} { // Any changes here should be reflected there. if cfg.ResourceMgr.Limits != nil { userSuppliedOverrideLimitConfig := *cfg.ResourceMgr.Limits - // This effectively overrides the computed default LimitConfig with any vlues from cfg.ResourceMgr.Limits. + // This effectively overrides the computed default LimitConfig with any non-zerod values from cfg.ResourceMgr.Limits. // Because of how how Apply works, any 0 value for a user supplied override // will be overriden with a computed default value. // There currently isn't a way for a user to to supply a 0-value override. From b14b1e303ea2e23600017120e96af4750b29a429 Mon Sep 17 00:00:00 2001 From: Steve Loeppky Date: Wed, 18 Jan 2023 16:12:15 -0800 Subject: [PATCH 3/6] Update rcmgr.go --- core/node/libp2p/rcmgr.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/node/libp2p/rcmgr.go b/core/node/libp2p/rcmgr.go index 5197866859a..eddb4fe2a78 100644 --- a/core/node/libp2p/rcmgr.go +++ b/core/node/libp2p/rcmgr.go @@ -63,7 +63,7 @@ func ResourceManager(cfg config.SwarmConfig) interface{} { // Any changes here should be reflected there. if cfg.ResourceMgr.Limits != nil { userSuppliedOverrideLimitConfig := *cfg.ResourceMgr.Limits - // This effectively overrides the computed default LimitConfig with any non-zerod values from cfg.ResourceMgr.Limits. + // This effectively overrides the computed default LimitConfig with any non-zero values from cfg.ResourceMgr.Limits. // Because of how how Apply works, any 0 value for a user supplied override // will be overriden with a computed default value. // There currently isn't a way for a user to to supply a 0-value override. From ea0055aa7f6f0d13aaf1ca8f1cca18bdbb39a136 Mon Sep 17 00:00:00 2001 From: Steve Loeppky Date: Thu, 19 Jan 2023 08:26:39 -0800 Subject: [PATCH 4/6] Update core/node/libp2p/rcmgr.go Co-authored-by: Antonio Navarro Perez --- core/node/libp2p/rcmgr.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/node/libp2p/rcmgr.go b/core/node/libp2p/rcmgr.go index eddb4fe2a78..15b6adad2ac 100644 --- a/core/node/libp2p/rcmgr.go +++ b/core/node/libp2p/rcmgr.go @@ -66,7 +66,7 @@ func ResourceManager(cfg config.SwarmConfig) interface{} { // This effectively overrides the computed default LimitConfig with any non-zero values from cfg.ResourceMgr.Limits. // Because of how how Apply works, any 0 value for a user supplied override // will be overriden with a computed default value. - // There currently isn't a way for a user to to supply a 0-value override. + // There currently isn't a way for a user to supply a 0-value override. userSuppliedOverrideLimitConfig.Apply(defaultComputedLimitConfig) limitConfig = userSuppliedOverrideLimitConfig } else { From 1562af8426caa41660e2d51458c7e59f3adddfb6 Mon Sep 17 00:00:00 2001 From: Steve Loeppky Date: Thu, 19 Jan 2023 08:32:16 -0800 Subject: [PATCH 5/6] Update config.md --- docs/config.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/config.md b/docs/config.md index aae2017d0a8..da919d84450 100644 --- a/docs/config.md +++ b/docs/config.md @@ -1844,6 +1844,9 @@ The `Swarm.ResourceMgr.Limits` override the default limits described above. Any override `BaseLimits` or limit s from `Swarm.ResourceMgr.Limits` that aren't specified will use the [computed default limits](./libp2p-resource-management.md#computed-default-limits). +Until [ipfs/kubo#9564](https://github.com/ipfs/kubo/issues/9564) is addressed, there isn't a way to set an override limit of zero. +0 is currently ignored. 0 currently means use to use the [computed default limits](./libp2p-resource-management.md#computed-default-limits). + Example #1: setting limits for a specific scope ```json { From 0d103e9b553176342edb27dd0cf79b6ae3027933 Mon Sep 17 00:00:00 2001 From: Steve Loeppky Date: Fri, 20 Jan 2023 14:29:10 -0800 Subject: [PATCH 6/6] Whitespace formatting --- core/node/libp2p/rcmgr.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/node/libp2p/rcmgr.go b/core/node/libp2p/rcmgr.go index 15b6adad2ac..6b0cee07dd7 100644 --- a/core/node/libp2p/rcmgr.go +++ b/core/node/libp2p/rcmgr.go @@ -64,13 +64,13 @@ func ResourceManager(cfg config.SwarmConfig) interface{} { if cfg.ResourceMgr.Limits != nil { userSuppliedOverrideLimitConfig := *cfg.ResourceMgr.Limits // This effectively overrides the computed default LimitConfig with any non-zero values from cfg.ResourceMgr.Limits. - // Because of how how Apply works, any 0 value for a user supplied override + // Because of how how Apply works, any 0 value for a user supplied override // will be overriden with a computed default value. // There currently isn't a way for a user to supply a 0-value override. userSuppliedOverrideLimitConfig.Apply(defaultComputedLimitConfig) limitConfig = userSuppliedOverrideLimitConfig } else { - limitConfig = defaultComputedLimitConfig + limitConfig = defaultComputedLimitConfig } limiter := rcmgr.NewFixedLimiter(limitConfig)