apache
HDDS-11367. Improve ozone balancing status command output
What changes were proposed in this pull request?
Minor improvements to command output:
- job task time consumption
- iteration time consumption
- unfinished iteration shouldn't be in iteration history section
- show data units, even it less than gb show it in mb
What is the link to the Apache JIRA
How was this patch tested?
Tested by robot test
@juncevich can you please elaborate on what these points mean?
job task time consumption iteration time consumption
It will help reviewers understand what you're proposing.
Hi, @siddhantsangwan. Sure, i can explain. Balancing job constists from some iterations. Job task time - the whole balancing time, iteration time - particular iteration time. Job time = iteration time + iteration time ...
@juncevich can you please check test timeouts / failures?
org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit
org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerStatusInfo
org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerTask
https://github.com/juncevich/ozone/actions/runs/11380961580/artifacts/2067990610
@juncevich can you please check test timeouts / failures?
org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerStatusInfo org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerTaskhttps://github.com/juncevich/ozone/actions/runs/11380961580/artifacts/2067990610
Thanks for the comment. Tests fixed.
@juncevich Is this ready for another round of reviews?
Yes, I try to fix the review notices ASAP because they are blocking another fix. I didn’t bring it up because it’s a different task
@ivanzlenko No need to use Atomic for OffsetDateTime, because it thread-safe by design.
@juncevich please let me know when you've addressed comments and this is ready for another review.
please let me know when you've addressed comments and this is ready for another review.
@siddhantsangwan I've not understood about comments, can you explain about? PR is ready for another review.
it seems issues @siddhantsangwan mentioned have been addressed. @siddhantsangwan could you take look again, please?
@juncevich if you've addressed all comments and fixed tests, I can start the CI workflow.
@juncevich if you've addressed all comments and fixed tests, I can start the CI workflow.
Great, thanks! Just fixed the flaky test. Everything should be OK.
@siddhantsangwan I've fixed your review notices. Could you look through the changed code? I have to fix the robot test and PR will be ready.
@juncevich Thanks for updating. However, please try not to force push changes as that makes it hard to review the updates.
@juncevich Thanks for updating. However, please try not to force push changes as that makes it hard to review the updates.
@siddhantsangwan I respect your point, but I can't entirely agree with it. You can choose the last commit with all fixes and without 'dirty' changes(commits). Force push makes the commit history more readable. https://github.com/apache/ozone/pull/7139/commits/2e60bf3a340438607dd706ef0cad052853ae56be
@juncevich thanks for updating the patch. LGTM. @siddhantsangwan do you have any other comments?
TestContainerBalancerStatusInfo.testGetCurrentStatisticsWhileBalancingInProgress seems failing.
https://github.com/apache/ozone/actions/runs/12313619015/job/34368477497?pr=7551
https://github.com/apache/ozone/actions/runs/12313619015/job/34368477497?pr=7551
Thanks. It is still flaky. Will check.