Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent Routing.Type=auto from enabling dhtserver mode if have too low of limits #9548

Open
3 tasks
BigLep opened this issue Jan 14, 2023 · 5 comments
Open
3 tasks

Comments

@BigLep
Copy link
Contributor

BigLep commented Jan 14, 2023

Done Criteria

A Kubo node with Routing.Type=auto doesn't attempt to be a DHT server if the following additional conditions aren't met:

  1. Swarm.ResourceMgr.System.ConnsInbound > 800 AND
  2. Swarm.ResourceMgr.System.StreamsInbound > 800 AND
  3. Swarm.ResourceMgr.Protocol.kaddht.StreamsInbound > 800 AND
  4. Swarm.ConnMgr is enabled

(Existing logic of ensuring the node is publicly dialable will still apply.)

Why Important

If a "dhtserver" node advertises itself to the DHT but has too-low of hard Swarm.ResourceMgr limits, then it won't be able to function and be of value to the DHT and instead degrades performance.

Notes

@BigLep BigLep mentioned this issue Jan 14, 2023
@Jorropo Jorropo self-assigned this Jan 16, 2023
@lidel
Copy link
Member

lidel commented Jan 16, 2023

@Jorropo this will be only applied when custom Swarm.ResourceMgr.Limits is present in the config, right?

I assume we don't want to apply this to people running with the implicit defaults, otherwise many current DHT servers like mine will disappear when they updade, causing higher load on remaining DHT servers.

Example: I run my 0.18.0-rc2 node with implicit defaults ConnMgr (32/96) and ResourceMgr defaults as well.

Have 32GB RAM and ResourceMgr calculated ConnsInbound to be ~558, not 800:

{
  "Conns": 1000000000,
  "ConnsInbound": 558,
  "ConnsOutbound": 1000000000,
  "FD": 524288,
  "Memory": 8300000000,
  "Streams": 1000000000,
  "StreamsInbound": 8939,
  "StreamsOutbound": 1000000000
}

@Jorropo
Copy link
Contributor

Jorropo commented Jan 16, 2023

@lidel because of #9555 the default scalling config will always be above thoses limit, so yes this indeed only applies to manually configured nodes.
(note the checks are >= not >)

@BigLep BigLep assigned ajnavarro and unassigned Jorropo Jan 17, 2023
@BigLep
Copy link
Contributor Author

BigLep commented Jan 17, 2023

@ajnavarro : per 2023-01-17 standup, base your work on #9555

Jorropo added a commit to Jorropo/go-ipfs that referenced this issue Jan 17, 2023
Jorropo added a commit to Jorropo/go-ipfs that referenced this issue Jan 17, 2023
Jorropo added a commit to Jorropo/go-ipfs that referenced this issue Jan 18, 2023
ajnavarro added a commit that referenced this issue Jan 18, 2023
…oo low of limits

Closes #9548

Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
ajnavarro added a commit that referenced this issue Jan 18, 2023
…oo low of limits

Closes #9548

Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
ajnavarro added a commit that referenced this issue Jan 18, 2023
…oo low of limits

Closes #9548

Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
@lidel lidel closed this as completed in 73ebad1 Jan 19, 2023
@lidel
Copy link
Member

lidel commented Jan 19, 2023

@Jorropo i think you've put wrong isssue in commit message of 73ebad1 🙃 – reopening, as this will be fixed by #9561 instead.

@lidel lidel reopened this Jan 19, 2023
@ajnavarro
Copy link
Member

Wrong comment on commit

@ajnavarro ajnavarro reopened this Jan 25, 2023
@ajnavarro ajnavarro removed their assignment Feb 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: 🥞 Todo
Development

Successfully merging a pull request may close this issue.

4 participants