feat: persist limits to Swarm.ResourceMgr.Limits #8901
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
guseggert
force-pushed
the
feat/rcmgr-config
branch
from
bdb8534
to
42b6071
Compare
lidel
requested changes
Apr 22, 2022
There was a problem hiding this comment.
Thanks @guseggert ! Did a quick pass:
Good:
- removing
limit.json– 👍 swarm limitalways persisting toSwarm.ResourceMgr.Limits– 👍
Needs fixing:
- helptext in
swarm limit --helpneeds updating to reflect that we now always persist - Enabling resource manager via one-liner
ipfs config --json Swarm.ResourceMgr.Enabled trueneeds to work (right now it is not enough, panics due to missingLimits– see comment forcore/node/libp2p/rcmgr.go) - + smaller comments inline
4 tasks
This changes the "ipfs swarm limit" command so that When limit changes are applied via the command line, they are persisted to the repo config, so that they remain in effect when the daemon restarts. This also removes limit.json support, to simplify the behavior. The schema for Swarm.ResourceMgr.Limits is exactly the same as limit.json, so existing limit.json can be dropped into the IPFS config easily using something like: cat ~/.ipfs/config | jq ".Swarm.ResourceMgr.Limits = $(cat limit.json)" | sponge ~/.ipfs/config This upgrades to Resource Manager v0.3.0, which exports the config schema so that we don't have to maintain our own copy of it.
guseggert
force-pushed
the
feat/rcmgr-config
branch
from
42b6071
to
3316b0d
Compare
Makes sure we evaluate the config JSON directly, so we catch any regressions around serialization too.
lidel
approved these changes
Apr 28, 2022
There was a problem hiding this comment.
Looks and works great, thank you, @guseggert 🙌
I noticed that we have no easy way to restore the implicit default once we set a custom one, but willed a separate issue for solving that: #8918 (this is nice-to-have, so could be tackled in future RCs, or pushed till 0.14)
I'm merging this one 🚀
lidel
changed the title
lidel
changed the title
This was referenced Apr 28, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This changes the "ipfs swarm limit" command so that When limit changes
are applied via the command line, they are persisted to the repo
config, so that they remain in effect when the daemon restarts.
This also removes limit.json support, to simplify the behavior. The
schema for Swarm.ResourceMgr.Limits is exactly the same as limit.json,
so existing limit.json can be dropped into the IPFS config easily
using something like:
$ cat ~/.ipfs/config | jq ".Swarm.ResourceMgr.Limits = $(cat limit.json)" | sponge ~/.ipfs/configThis upgrades to Resource Manager v0.3.0, which exports the config
schema so that we don't have to maintain our own copy of it.