siddhantsangwan commented on code in PR #3423:
URL: https://github.com/apache/ozone/pull/3423#discussion_r879165642
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java:
##########
@@ -830,12 +927,47 @@ public String getServiceName() {
}
/**
- * Starts ContainerBalancer as an SCMService.
+ * Starts ContainerBalancer as an SCMService. Validates state, reads and
+ * validates persisted configuration, and then starts the balancing
+ * thread.
+ * @throws IllegalContainerBalancerStateException if balancer should not
+ * run according to persisted configuration
+ * @throws InvalidContainerBalancerConfigurationException if failed to
+ * retrieve persisted configuration, or the configuration is null
*/
@Override
- public void start() {
- if (shouldRun()) {
+ public void start() throws IllegalContainerBalancerStateException,
+ InvalidContainerBalancerConfigurationException {
+ lock.lock();
+ try {
+ // should be leader-ready, out of safe mode, and not running already
+ validateState(false);
+ ContainerBalancerConfigurationProto proto;
+ try {
+ proto = readConfiguration(ContainerBalancerConfigurationProto.class);
+ } catch (IOException e) {
+ throw new InvalidContainerBalancerConfigurationException("Could not " +
+ "retrieve persisted configuration while starting " +
+ "Container Balancer as an SCMService. Will not start now.", e);
+ }
+ if (proto == null) {
+ throw new InvalidContainerBalancerConfigurationException("Persisted " +
+ "configuration for ContainerBalancer is null during start. Will " +
+ "not start now.");
+ }
Review Comment:
I am considering this as an exceptional situation because `start` should
have been called from `notifyStatusChanged()` only after checking that balancer
should run. So we should be able to get persisted configuration here as well,
instead of getting null.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
