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

feat: add max-radius param with 5% default #1647

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KolbyML
Copy link
Member

@KolbyML KolbyML commented Jan 22, 2025

What was wrong?

For a larger description of the problem can be viewed here #1574

The TL:DR is currently we start at 100 percent radius when the node will end up at 1-3%. This means we download and purge content our node won't end up storing which wastes a lot of CPU cycles, but also takes our uTP transfer limit which could be used for content within our final 1-3% final radius.

This is a big issue as on the state network, currently we are trying to bridge latest state, but our nodes uTP limit is always full so often trin-execution offer's of state diffs are declined as all State nodes are at transfer capacity.

image
^ here is a picture I took awhile ago, at the bottom you can see we successfully offered to 0 peers and the content was declined 8 times. This is due to the State nodes being at uTP transfer capacity (or in other words hitting our rate limit)

Recently this issue has gotten worse, but even if it didn't this PR is required if we want to bridge latest state. Bridging state content is very expensive, because there is just so much state to gossip. If we want Portal and the State network to be successful we need a 10x performance improvement at least in Trin as currently Trin can only handle 5 Mbps up and down which is way to slow, we won't be able to take a load on the network with this.

So this PR is the first step in a road of changes which will hopefully greatly increase the performance in Trin. I don't think we have a choice we need to be performent if we want users.

How was it fixed?

Add a cli called max-radius, by default the max radius is 5 percent, assuming we have a network of over 20 nodes we should have max coverage. Since the nodes radius's should end up below 3% especially as the network storage requirements will keep growing over time. So I don't think we should have a default bigger then 5%

For testing on hive we will use --max-radius 100

I think this solution is the best one for the foreseeable future, we could do a more complex scheme but that wouldn't be worth it right now, instead we should focus on finding other bottlenecks in Trin as this PR is good enough and gets us most of want we want for the foreseeable future. A more complex scheme would give diminishing returns, so I wouldn't expect us to look into it until Trin is very optimized so it is a very low priority to further optimize this with a complex solution.

@KolbyML KolbyML self-assigned this Jan 22, 2025
@KolbyML KolbyML force-pushed the default-max-radius-5-percent branch from d3a94af to 83ceaed Compare January 22, 2025 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant