-
Notifications
You must be signed in to change notification settings - Fork 593
HDDS-13183. Create Cluster Capacity page UI. #9584
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
base: master
Are you sure you want to change the base?
Conversation
(cherry picked from commit cc5cf02)
devmadhuu
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.
Thanks @devabhishekpal for the patch.
- I think , we decided to use datanode dropdown as combo of textbox and dropdown, where user can enter also to search in long list of drop down if needed.
- We need export option for datanodes card atleast.
- By default, datanodes in dropdown needs to be populated in descending order of pending deletion size.
- We should gray out the list of failed datanodes in dropdown as their pending deletion data is no longer useful for user to be displayed. This should be possible using css.
- How are we planning to handle the case when the API response will be having duplicate nodes with same name. Pls check the API behavior in cases when same name DNs joins the cluster and if API will have response because API response contains datanode names as well as UUID, and UUID is only being treated as unique.
| } | ||
| ], | ||
| "totalNodesQueried": 7, | ||
| "totalNodeQueriesFailed": 2 |
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.
If nodes query failed count is 2, then db.json should reflect the output, I think it gives -1. Check the API behavior.
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.
Yes, 2 nodes should show -1 for pendingBlockSize
priyeshkaratha
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.
Thanks @devabhishekpal for working on the patch.
I have two comments regarding current UI implementation for showing DN pendingDeletion and related refreshing strategy. Please have a look on that.
| } | ||
| }, [selectedDatanode, storageDistribution.data.dataNodeUsage]); | ||
|
|
||
| const loadDNData = () => { |
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.
Currently, the UI shows loading because data is cleared during the 30-second refresh interval. To fix this, we should persist the existing data in the UI while the background update is in progress, only replacing it once the new value is available. Additionally, for the initial load, we should poll at a faster 2-second interval until the first value is retrieved to avoid a long initial loading state. Whenever use do refresh of the page call API which will give in progress state and that time also do a polling with some small interval. So that it can be more UI friendly.
API results will be fast for even 100 node cluster so it usually takes less time for normal clusters.
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.
I agree with the shorter refresh interval until data is loaded i.e the status is FINISHED
However we should not persist old data while background refresh is going on. This would be against general UI guidelines, any loading should be indicated in the UI as well since the users won't know otherwise if anything goes wrong (say the loading failed in the background, or some error occurred - but since the UI is showing the old data and not indicating any loading process users might not even be aware something failed until they see the logs.
Hence even if the loading takes time for a very large cluster (which it shouldn't as you said it is a fast enough process) we should show it as loading in the UI as well so that the user is aware of a task going on.
| : ( | ||
| <span> | ||
| PENDING DELETION | ||
| <WrappedInfoIcon title="DN pending deletion data is not yet available. It will be fetched once the pending deletion scan is finished on all datanodes." /> |
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.
This can be shown only once during initial calculations. Once data is available you can cache it and after that you can show different info.
What changes were proposed in this pull request?
Create the Cluster Capacity page UI
Please describe your PR in detail:
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-13183
How was this patch tested?
Patch was tested manually.