Tejaskriya commented on code in PR #6241:
URL: https://github.com/apache/ozone/pull/6241#discussion_r1537976134
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java:
##########
@@ -1047,67 +1047,131 @@ public StartContainerBalancerResponseProto
startContainerBalancer(
Optional<Integer> maxDatanodesPercentageToInvolvePerIteration,
Optional<Long> maxSizeToMovePerIterationInGB,
Optional<Long> maxSizeEnteringTarget,
- Optional<Long> maxSizeLeavingSource) throws IOException {
+ Optional<Long> maxSizeLeavingSource,
+ Optional<Integer> balancingInterval,
+ Optional<Integer> moveTimeout,
+ Optional<Integer> moveReplicationTimeout,
+ Optional<Boolean> networkTopologyEnable,
+ Optional<String> includeNodes,
+ Optional<String> excludeNodes) throws IOException {
getScm().checkAdminAccess(getRemoteUser(), false);
ContainerBalancerConfiguration cbc =
scm.getConfiguration().getObject(ContainerBalancerConfiguration.class);
Map<String, String> auditMap = Maps.newHashMap();
- if (threshold.isPresent()) {
- double tsd = threshold.get();
- auditMap.put("threshold", String.valueOf(tsd));
- Preconditions.checkState(tsd >= 0.0D && tsd < 100.0D,
- "threshold should be specified in range [0.0, 100.0).");
- cbc.setThreshold(tsd);
- }
- if (maxSizeToMovePerIterationInGB.isPresent()) {
- long mstm = maxSizeToMovePerIterationInGB.get();
- auditMap.put("maxSizeToMovePerIterationInGB", String.valueOf(mstm));
- Preconditions.checkState(mstm > 0,
- "maxSizeToMovePerIterationInGB must be positive.");
- cbc.setMaxSizeToMovePerIteration(mstm * OzoneConsts.GB);
- }
- if (maxDatanodesPercentageToInvolvePerIteration.isPresent()) {
- int mdti = maxDatanodesPercentageToInvolvePerIteration.get();
- auditMap.put("maxDatanodesPercentageToInvolvePerIteration",
- String.valueOf(mdti));
- Preconditions.checkState(mdti >= 0,
- "maxDatanodesPercentageToInvolvePerIteration must be " +
- "greater than equal to zero.");
- Preconditions.checkState(mdti <= 100,
- "maxDatanodesPercentageToInvolvePerIteration must be " +
- "lesser than or equal to 100.");
- cbc.setMaxDatanodesPercentageToInvolvePerIteration(mdti);
- }
- if (iterations.isPresent()) {
- int i = iterations.get();
- auditMap.put("iterations", String.valueOf(i));
- Preconditions.checkState(i > 0 || i == -1,
- "number of iterations must be positive or" +
- " -1 (for running container balancer infinitely).");
- cbc.setIterations(i);
- }
+ ResultCodes resultCodes = ResultCodes.FAILED_TO_START_CONTAINER_BALANCER;
+ try {
+ if (threshold.isPresent()) {
+ double tsd = threshold.get();
+ auditMap.put("threshold", String.valueOf(tsd));
+ if (tsd < 0.0D || tsd >= 100.0D) {
+ throw new SCMException("Threshold should be specified in the range
[0.0, 100.0).", resultCodes);
+ }
+ cbc.setThreshold(tsd);
+ }
- if (maxSizeEnteringTarget.isPresent()) {
- long mset = maxSizeEnteringTarget.get();
- auditMap.put("maxSizeEnteringTarget", String.valueOf(mset));
- Preconditions.checkState(mset > 0,
- "maxSizeEnteringTarget must be " +
- "greater than zero.");
- cbc.setMaxSizeEnteringTarget(mset * OzoneConsts.GB);
- }
+ if (maxSizeToMovePerIterationInGB.isPresent()) {
+ long mstm = maxSizeToMovePerIterationInGB.get();
+ auditMap.put("maxSizeToMovePerIterationInGB", String.valueOf(mstm));
+ if (mstm < 0) {
+ throw new SCMException("Max Size To Move Per Iteration In GB must be
positive.", resultCodes);
+ }
+ cbc.setMaxSizeToMovePerIteration(mstm * OzoneConsts.GB);
+ }
- if (maxSizeLeavingSource.isPresent()) {
- long msls = maxSizeLeavingSource.get();
- auditMap.put("maxSizeLeavingSource", String.valueOf(msls));
- Preconditions.checkState(msls > 0,
- "maxSizeLeavingSource must be " +
- "greater than zero.");
- cbc.setMaxSizeLeavingSource(msls * OzoneConsts.GB);
- }
+ if (maxDatanodesPercentageToInvolvePerIteration.isPresent()) {
+ int mdti = maxDatanodesPercentageToInvolvePerIteration.get();
+ auditMap.put("maxDatanodesPercentageToInvolvePerIteration",
+ String.valueOf(mdti));
+ if (mdti < 0 || mdti > 100) {
+ throw new SCMException("Max Datanodes Percentage To Involve Per
Iteration" +
+ "should be specified in the range [0, 100]", resultCodes);
+ }
+ cbc.setMaxDatanodesPercentageToInvolvePerIteration(mdti);
+ }
- ContainerBalancer containerBalancer = scm.getContainerBalancer();
- try {
+ if (iterations.isPresent()) {
+ int i = iterations.get();
+ auditMap.put("iterations", String.valueOf(i));
+ if (i < -1 || i == 0) {
+ throw new SCMException("Number of Iterations must be positive or" +
+ " -1 (for running container balancer infinitely).", resultCodes);
+ }
+ cbc.setIterations(i);
+ }
+
+ if (maxSizeEnteringTarget.isPresent()) {
+ long mset = maxSizeEnteringTarget.get();
+ auditMap.put("maxSizeEnteringTarget", String.valueOf(mset));
+ if (mset < 0) {
+ throw new SCMException("Max Size Entering Target must be " +
+ "greater than zero.", resultCodes);
+ }
+ cbc.setMaxSizeEnteringTarget(mset * OzoneConsts.GB);
+ }
+
+ if (maxSizeLeavingSource.isPresent()) {
+ long msls = maxSizeLeavingSource.get();
+ auditMap.put("maxSizeLeavingSource", String.valueOf(msls));
+ if (msls < 0) {
+ throw new SCMException("Max Size Leaving Source must be " +
+ "greater than zero.", resultCodes);
+ }
+ cbc.setMaxSizeLeavingSource(msls * OzoneConsts.GB);
+ }
+
+ if (balancingInterval.isPresent()) {
+ int bi = balancingInterval.get();
+ auditMap.put("balancingInterval", String.valueOf(bi));
+ if (bi < 0) {
+ throw new SCMException("Balancing Interval must be greater than
zero.", resultCodes);
+ }
+ cbc.setBalancingInterval(Duration.ofMinutes(bi));
+ }
+
+ if (moveTimeout.isPresent()) {
+ int mt = moveTimeout.get();
+ auditMap.put("moveTimeout", String.valueOf(mt));
+ if (mt < 0) {
Review Comment:
Are we sure that these values can be 0 too?
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java:
##########
@@ -1047,67 +1047,131 @@ public StartContainerBalancerResponseProto
startContainerBalancer(
Optional<Integer> maxDatanodesPercentageToInvolvePerIteration,
Optional<Long> maxSizeToMovePerIterationInGB,
Optional<Long> maxSizeEnteringTarget,
- Optional<Long> maxSizeLeavingSource) throws IOException {
+ Optional<Long> maxSizeLeavingSource,
+ Optional<Integer> balancingInterval,
+ Optional<Integer> moveTimeout,
+ Optional<Integer> moveReplicationTimeout,
+ Optional<Boolean> networkTopologyEnable,
+ Optional<String> includeNodes,
+ Optional<String> excludeNodes) throws IOException {
getScm().checkAdminAccess(getRemoteUser(), false);
ContainerBalancerConfiguration cbc =
scm.getConfiguration().getObject(ContainerBalancerConfiguration.class);
Map<String, String> auditMap = Maps.newHashMap();
- if (threshold.isPresent()) {
- double tsd = threshold.get();
- auditMap.put("threshold", String.valueOf(tsd));
- Preconditions.checkState(tsd >= 0.0D && tsd < 100.0D,
- "threshold should be specified in range [0.0, 100.0).");
- cbc.setThreshold(tsd);
- }
- if (maxSizeToMovePerIterationInGB.isPresent()) {
- long mstm = maxSizeToMovePerIterationInGB.get();
- auditMap.put("maxSizeToMovePerIterationInGB", String.valueOf(mstm));
- Preconditions.checkState(mstm > 0,
- "maxSizeToMovePerIterationInGB must be positive.");
- cbc.setMaxSizeToMovePerIteration(mstm * OzoneConsts.GB);
- }
- if (maxDatanodesPercentageToInvolvePerIteration.isPresent()) {
- int mdti = maxDatanodesPercentageToInvolvePerIteration.get();
- auditMap.put("maxDatanodesPercentageToInvolvePerIteration",
- String.valueOf(mdti));
- Preconditions.checkState(mdti >= 0,
- "maxDatanodesPercentageToInvolvePerIteration must be " +
- "greater than equal to zero.");
- Preconditions.checkState(mdti <= 100,
- "maxDatanodesPercentageToInvolvePerIteration must be " +
- "lesser than or equal to 100.");
- cbc.setMaxDatanodesPercentageToInvolvePerIteration(mdti);
- }
- if (iterations.isPresent()) {
- int i = iterations.get();
- auditMap.put("iterations", String.valueOf(i));
- Preconditions.checkState(i > 0 || i == -1,
- "number of iterations must be positive or" +
- " -1 (for running container balancer infinitely).");
- cbc.setIterations(i);
- }
+ ResultCodes resultCodes = ResultCodes.FAILED_TO_START_CONTAINER_BALANCER;
+ try {
+ if (threshold.isPresent()) {
+ double tsd = threshold.get();
+ auditMap.put("threshold", String.valueOf(tsd));
+ if (tsd < 0.0D || tsd >= 100.0D) {
+ throw new SCMException("Threshold should be specified in the range
[0.0, 100.0).", resultCodes);
+ }
+ cbc.setThreshold(tsd);
+ }
- if (maxSizeEnteringTarget.isPresent()) {
- long mset = maxSizeEnteringTarget.get();
- auditMap.put("maxSizeEnteringTarget", String.valueOf(mset));
- Preconditions.checkState(mset > 0,
- "maxSizeEnteringTarget must be " +
- "greater than zero.");
- cbc.setMaxSizeEnteringTarget(mset * OzoneConsts.GB);
- }
+ if (maxSizeToMovePerIterationInGB.isPresent()) {
+ long mstm = maxSizeToMovePerIterationInGB.get();
+ auditMap.put("maxSizeToMovePerIterationInGB", String.valueOf(mstm));
+ if (mstm < 0) {
+ throw new SCMException("Max Size To Move Per Iteration In GB must be
positive.", resultCodes);
+ }
+ cbc.setMaxSizeToMovePerIteration(mstm * OzoneConsts.GB);
+ }
- if (maxSizeLeavingSource.isPresent()) {
- long msls = maxSizeLeavingSource.get();
- auditMap.put("maxSizeLeavingSource", String.valueOf(msls));
- Preconditions.checkState(msls > 0,
- "maxSizeLeavingSource must be " +
- "greater than zero.");
- cbc.setMaxSizeLeavingSource(msls * OzoneConsts.GB);
- }
+ if (maxDatanodesPercentageToInvolvePerIteration.isPresent()) {
+ int mdti = maxDatanodesPercentageToInvolvePerIteration.get();
+ auditMap.put("maxDatanodesPercentageToInvolvePerIteration",
+ String.valueOf(mdti));
+ if (mdti < 0 || mdti > 100) {
+ throw new SCMException("Max Datanodes Percentage To Involve Per
Iteration" +
+ "should be specified in the range [0, 100]", resultCodes);
+ }
+ cbc.setMaxDatanodesPercentageToInvolvePerIteration(mdti);
+ }
- ContainerBalancer containerBalancer = scm.getContainerBalancer();
- try {
+ if (iterations.isPresent()) {
+ int i = iterations.get();
+ auditMap.put("iterations", String.valueOf(i));
+ if (i < -1 || i == 0) {
+ throw new SCMException("Number of Iterations must be positive or" +
+ " -1 (for running container balancer infinitely).", resultCodes);
+ }
+ cbc.setIterations(i);
+ }
+
+ if (maxSizeEnteringTarget.isPresent()) {
+ long mset = maxSizeEnteringTarget.get();
+ auditMap.put("maxSizeEnteringTarget", String.valueOf(mset));
+ if (mset < 0) {
+ throw new SCMException("Max Size Entering Target must be " +
+ "greater than zero.", resultCodes);
+ }
+ cbc.setMaxSizeEnteringTarget(mset * OzoneConsts.GB);
+ }
+
+ if (maxSizeLeavingSource.isPresent()) {
+ long msls = maxSizeLeavingSource.get();
+ auditMap.put("maxSizeLeavingSource", String.valueOf(msls));
+ if (msls < 0) {
+ throw new SCMException("Max Size Leaving Source must be " +
+ "greater than zero.", resultCodes);
+ }
+ cbc.setMaxSizeLeavingSource(msls * OzoneConsts.GB);
+ }
+
+ if (balancingInterval.isPresent()) {
+ int bi = balancingInterval.get();
+ auditMap.put("balancingInterval", String.valueOf(bi));
+ if (bi < 0) {
+ throw new SCMException("Balancing Interval must be greater than
zero.", resultCodes);
+ }
+ cbc.setBalancingInterval(Duration.ofMinutes(bi));
+ }
+
+ if (moveTimeout.isPresent()) {
+ int mt = moveTimeout.get();
+ auditMap.put("moveTimeout", String.valueOf(mt));
+ if (mt < 0) {
Review Comment:
Are we sure that these values can be 0 too?
--
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]
