-
Notifications
You must be signed in to change notification settings - Fork 25
Change CPU panels' units from percent to sishort (20% -> 200m) #163
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
Conversation
|
Ooops the description of this PR is for #164, will fix it from a computer when given the chance (EDIT: DONE!) |
jnywong
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain from your perspective why changing the units from % to (nano, micro, milli)-cores would be more user-friendly?
I think it's more technically correct to express the units as (nano, micro, milli)-cores, but less intuitive for someone looking at these dashboards to get a vague sense of % compute used out of the available requested.
I'm inclined to keep this as % unless you can convince me otherwise! I think at the very least we should add a tooltip description that unit of CPU compute here is broadly the number of cores used in a given time period.
|
Thank you @jnywong for taking the time!!
There are some panels related to CPU that I didnt change in this PR that measured the fraction of CPU used up on a node - a part-divided-by-whole situation - there i agree fully on using percent! However, for the panels updated, the unit is in actual CPU cores in absolute terms, so if we use percent it means 50% represents 0.5 CPU cores. It seems confusing to me that the percent relates to 1 CPU core specifically - unrelated to the pod containers requested CPU cores, or the node's available CPU cores for pods' containers, or the nodes total CPU capacity etc. Is 50% and 250% to represent 0.5 CPU and 2.5 CPU is better here than 500m and 2.5? From maintaining k8s container requests and limit, i appreciate sticking with using the latter, and I figure people reading these panels may want to do just that as well. Separately, I also think there is merit to reserving the use of percent for part-divided-by-whole situations where it is very clear what the whole is from the context. |
|
Thanks for the thoughtful response! Yes from the k8s requests/limits point of view this makes sense. There are definitely times where a user is using a Dask Gateway cluster and you get values like 1500% which look funny! For a hub admin (who does not operate k8s) looking at the User/Group diagnostics and interested in how efficiently users are requesting CPU, maybe they can benefit from a unitless/% based panels with a time series of memory and CPU usage/requests (in a different PR :D)? Then I would be happy to merge this PR with CPU cores, as long as the tooltip description explains that the SI units in the Y axis ( |
320a678 to
24e544b
Compare
jnywong
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fantastic! Thanks for the small changes, they make a big difference 🚀
|
Thank you @jnywong!!! ❤️ 🎉 🚀 |
The groups panel "CPU Usage" isn't screenshotted, because I haven't got it to work yet due to #165.
Unchanged panels with CPU related data include:
