Skip to content

Portal Client: Improve node status log #3327

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
May 26, 2025
Merged

Portal Client: Improve node status log #3327

Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
124f462
Add log radius to portal node status log.
bhartnett May 24, 2025
4332c53
Add log radius to portal node status log.
bhartnett May 24, 2025
eefa0b6
Add log radius to portal node status log.
bhartnett May 24, 2025
3e3541b
Add log radius to portal node status log.
bhartnett May 24, 2025
be525fd
Remove log radius percentage.
bhartnett May 26, 2025
08c00b5
Minor fix.
bhartnett May 26, 2025
4fba9ee
Just show the log radius as a number.
bhartnett May 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions portal/network/portal_node.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import
./history/[history_network, history_content],
./state/[state_network, state_content]

from eth/p2p/discoveryv5/routing_table import logDistance

export
beacon_light_client, history_network, state_network, portal_protocol_config, forks

Expand Down Expand Up @@ -203,12 +205,18 @@ proc statusLogLoop(n: PortalNode) {.async: (raises: []).} =
# drop a lot when using the logbase2 scale, namely `/ 2` per 1 logaritmic
# radius drop.
# TODO: Get some float precision calculus?
let radiusPercentage = n.contentDB.dataRadius div (UInt256.high() div u256(100))
let
radius = n.contentDB.dataRadius
radiusPercentage = radius div (UInt256.high() div u256(100))
logRadius = logDistance(radius, u256(0))
logRadiusPercentage = (logRadius * 100) div 256
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I agree that showing the rounded down 0% is not very useful (hence also the original float precision TODO above), I do not think that showing the log radius percentage is any more useful to an user at all. Regular users have to understand log distance to know what this means, and even then, expressing it as a percentage is not so intuitive I think.
And for devs, I think the logRadius is handy, but from that you can deduct quite immediately the rest and do not need logRadiusPercentage.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I agree that showing the rounded down 0% is not very useful (hence also the original float precision TODO above), I do not think that showing the log radius percentage is any more useful to an user at all. Regular users have to understand log distance to know what this means, and even then, expressing it as a percentage is not so intuitive I think. And for devs, I think the logRadius is handy, but from that you can deduct quite immediately the rest and do not need logRadiusPercentage.

After thinking about this a bit more I generally agree. Unfortunately, showing the U256 number as a float isn't currently supported by our libraries and I didn't find an easy solution. Even if we get it working, the decimals may be very small because we will eventually end up having very large amount of data in portal network. I think we should show the logRadius like this: radius / 256. Perhaps it could be named different to logRadius or perhaps it could go in a separate debug log since its not ideal for end users. My thoughts are that it doesn't matter too much and would be useful to just see the logRadius here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've now removed the logRadiusPercentage field and moved the logRadius field to the last position which should be the least visible. Now that I think about it the existing radius field is not very useful for an end user either (not suggesting we remove it though) and I noticed that in the help when we set the radius using the static option we set it using a log radius value. See from the help:

 --radius                  Radius configuration for the portal node. Radius can be either `dynamic` where
                               the node adjusts the radius based on `storage-size` option, or
                               `static:<logRadius>` where the node has a hardcoded logarithmic radius value.
                               Warning: `static:<logRadius>` disables `storage-size` limits and makes the node
                               store a fraction of the network based on set radius. [=Dynamic].

So considering that logRadius is an end user setting on the CLI perhaps its ok to be in the logs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'm perfectly fine with having logRadius in there.


info "Portal node status",
dbSize = $(n.contentDB.size() div 1_000_000) & "MB",
logRadiusPercentage = $logRadiusPercentage & "%",
logRadius,
radiusPercentage = radiusPercentage.toString(10) & "%",
radius = n.contentDB.dataRadius.toHex(),
dbSize = $(n.contentDB.size() div 1000) & "kb"
radius = radius.toHex()

await sleepAsync(60.seconds)
except CancelledError:
Expand Down
Loading