[Tracking issue] go-libp2p resource manager critical post release fixes

[BigLep]

This is the tracking issue for the streams of work that need to get corrected/fixed as result of the Kubo 0.17 release with libp2p resource manager enabled by default: #8761

Must complete before 0.18 RC

Theme 2: reports of default limits not working well for users

• cannot reserve inbound connection: resource limit exceeded #9432 . There are multiple problems buried in here per cannot reserve inbound connection: resource limit exceeded #9432 (comment). This theme is about raising the default limits.
  • Fix: Increase MaxMemory param to use 1/4 of total memory. #9446

Theme 3: improve RM errors coming from other peers

This is related to reports of a disabled go-libp2p resource manager still managing resources. In fact this is an error message from a remote go-libp2p peer exceeding their own resource limits and when the message is printed on the local peer its hard to differentiate between the do.

Things we can do:

1. Adjust messaging for resource manager on the local Kubo side so it's easier to differentiate between local and remote peer resource manager errors.
   • It looks like the message already is differentiable. Feel free to do more though.
2. Adds some docs on how to differentiate and point to the go-libp2p issue for handling this better in go-libp2: quic: Error from peer should be explicit that it is coming from remote peer rather than a local error libp2p/go-libp2p#1928
   • docs: libp2p resource management #9468

Theme 4: confusion around "magic values"

This is about how "4611686018427388000" looks like a random number when it is actually our "infinity".

Things we can do:

1. Allow -1 to denote infinity for the go-libp2p resource manager. Requested: Enable specifying infinite limits with a more "intuitive" magic number like -1 libp2p/go-libp2p#1935
2. Document what Kubo's magic value is:
   • docs: libp2p resource management #9468
3. Using another number that is more obvious a magic number like 999999999999999999
   • It then also possible to search and find the number int he code. (Right now we are computing it which is what causes it not to show up in search)
     • Fix: RM: Set no-limit value to 1e9 (1000000000). #9469

Options not on the table:

1. ipfs swarm limits all to go from "4611686018427388000" to "infinity" on the output because that isn't valid JSON.
2. Similarly we can't add a comment after the magic number because that isn't valid json and the go json parser we use doesn't allow parsing non-spec compliant things like comments.

Theme 5: be clearer on startup about what limits are being set and why

Add a log message like:

First computing default go-libp2p resource manager limits based on Swarm.ResourceManager.maxMemory of {Swarm.ResourceManager.maxMemory} and then applying any user-supplied overrides on top. Run ipfs swarm limits all to see the resulting limits.

• Fix: RM: Improve init RM message and fix final memory value. #9470

Theme 6: Wrong tone about resource manager getting in the way (being a bug) vs. being a feature

UX angle about being clear that in general this is a feature not a bug:

• cannot reserve inbound connection: resource limit exceeded #9432 (comment)
  • fix(rcmgr): improve error phrasing #9444
• docs: libp2p resource management #9468

Theme 7: clarity around the "error message" meaning

There isn't clarity around what "system: cannot reserve inbound connection: resource limit exceeded". means. For this example, it means Swarm.ResourceMgr.Limits.System.ConnsInbound is exceeded.

Things we can do:

1. Provide docs on how to interpret this message.
   • docs: libp2p resource management #9468
2. Reverse engineer the message based on https://github.com/libp2p/go-libp2p/blob/master/p2p/host/resource-manager/scope.go so we can map it back to Swarm.ResourceMgr.Limits.$scope.$limit. If we do that, we can then print what the limit value is.
   • 2022-12-06 maintainer conversation: not going to do this

Theme 8: Provide actionable advice when resource limits are hit

When a resource limit is hit, we point users to https://github.com/ipfs/kubo/blob/master/docs/config.md#swarmresourcemgr. We could provide a better documentation path for how someone debugs this situation.

• docs: libp2p resource management #9468

Theme 9: have the ConnMgr limits be set under Swarm.ResourceMgr.Limits.System.ConnsInbound

As discussed in #9468, this allows low priority idle connections to get cleaned up to make space for higher priority connections

Things we can do:

1. Lower the default limits
   • Lower default connection limits (Swarm.ConnMgr) #9420
   • feat(connmgr): lower the default connection pool #9467
2. Log when ConnMgr limits aren'tunder Swarm.ResourceMgr.Limits.System.ConnsInbound

Theme 10: resource manager doing its job of protecting a node is alarming

This was broughtup throughout #9432, but generally users have found the resource manager "ERRORs" spammy. That they are printed as ERRORs also runs counter to our narrative about this being a feature. Should a feature doing its job be an error?

Things we can do:

Theme 11: fix bugs in the swarm stats command

• swarm stats --min-used-limit-perc doesn't work for "all" scope #9473

Theme 12: remove additional footguns around (soft) ConnMgr and (hard) ResourceMgr limits and their interactions

• Computed default ResourceMgr limits account for ConnMgr HighWater and are sufficiently high #9545
• Fail if Swarm.ConnMgr.HighWater is above Swarm.ResourceMgr hard limits #9549

Theme 13: clarify and improve handling of zeroes

• Make it possible to set 0-value user-supplied resource manage limits #9564
• Resource Manager: ipfs swarm limit <scope> --reset is setting to zero all other scopes. #9559

Ideally completing before the 0.18 final release

Theme 1: usability issues in entering config

It's too easy for someone to enter invalid config and not get any feedback that they have done so.

• fix: Avoid unknown fields on config #9438
  • This will happen in 2023Q1 and won't making it in for 0.18

Theme 11: Improve usability in the swarm stats command

• Improve the utility of the swarm stats command for determining which limits are being hit #9474

[BigLep]

@ajnavarro : are there any other critical followups with resource manager we should be tracking?

[BigLep]

(I pasted a comment here that I meant to have in #9432. I moved it over: #9432 (comment) ).

[ajnavarro]

@BigLep Adding libp2p/go-libp2p#1928 to the list

[ajnavarro]

After fixing some problems on the routing system when using the parallel router, I'll continue with #9438

[BigLep]

@ajnavarro : I updated the issue description with the latest info and started a PR with some dedicated libp2p resource management docs: #9468

[ajnavarro]

I updated the main issue comment adding all the work that is being done for RM.

Pending work to be reviewed and merged:

• Fix: RM: Improve init RM message and fix final memory value. #9470
• Fix: Increase MaxMemory param to use 1/4 of total memory. #9446
• docs: libp2p resource management #9468
• Fix: RM: Set no-limit value to 1e9 (1000000000). #9469
• feat(connmgr): lower the default connection pool #9467

[ajnavarro]

A new one to the bucket:

• Fix: Change RM log output to WARN level #9472

[BigLep]

Two more items identified to address (added to issue description):
#9545
#9549

[BigLep]

Two more added (theme 13) around zero handling:

• Make it possible to set 0-value user-supplied resource manage limits #9564
• Resource Manager: ipfs swarm limit <scope> --reset is setting to zero all other scopes. #9559

[ajnavarro]

Added some deeper possible solutions after talking with @MarcoPolo about how to improve RM integration on Kubo: #9580

[ShadowJonathan]

I just stumbled into theme 3 in #9432, I'll repeat what I said there:

The log entries could be made more clear if (remote) is turned into (from remote), else it's easy to confuse this as being related to remote something, not that it is coming from the remote node.

Wrapping it in quotes to show that it is "verbatim" would probably help as well, to deal with errors foreign to kubo. Printing the remote node version (if its known) would maybe also help debugging it further.

[BigLep]

In the latest round of consolidating/organizing the resource manager/accountant work, I'm closing this in favor of #9650