-
Notifications
You must be signed in to change notification settings - Fork 502
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
HDDS-11388. Fix unnecessary call to the DB for ContainerBalancer#getBalancerStatusInfo #7224
Conversation
…alancerStatusInfo
Seems container balancer acceptance test failed after this patch. Let me take a look first. Edit: This is because |
@ivandika3 let me know when it's ready for review. |
@siddhantsangwan Hi, this patch is ready for review. Thank you. |
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 is because shouldRun is not set in the in-memory ContainerBalancerConfigurationProto causing proto exception.
@ivandika3 good find there. This patch looks good. There's a conflict with testBalancer.robot
that needs to be fixed.
# Conflicts: # hadoop-ozone/dist/src/main/smoketest/balancer/testBalancer.robot
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.
LGTM, pending green CI.
Thank you for the review @siddhantsangwan |
What changes were proposed in this pull request?
ContainerBalancer#getBalancerStatusInfo
can get the current running balancer configuration from the in-memory configuration, which is in sync with the backing DB. Also added lock ingetBalancerStatusInfo
.What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-11388
How was this patch tested?
Unit test.