ivanzlenko commented on code in PR #7139: URL: https://github.com/apache/ozone/pull/7139#discussion_r1800490553
########## hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerMetrics.java: ########## @@ -204,9 +227,11 @@ public void incrementCurrentIterationContainerMoveMetric( } } + /** + * Moved containers in last iteration. Review Comment: ```suggestion * Moved containers in the last iteration. ``` ########## hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerStatusInfo.java: ########## @@ -51,4 +53,21 @@ public HddsProtos.ContainerBalancerConfigurationProto getConfiguration() { public List<ContainerBalancerTaskIterationStatusInfo> getIterationsStatusInfo() { return iterationsStatusInfo; } + + /** + * Map to proto. Review Comment: ```suggestion * Converts an instance into the protobuf compatible object. ``` ########## hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerMetrics.java: ########## @@ -154,6 +159,29 @@ public void resetDataSizeMovedGBInLatestIteration() { -getDataSizeMovedGBInLatestIteration()); } + /** + * Gets the amount of data moved by Container Balancer in the latest iteration. + * @return size in bytes + */ + public long getDataSizeMovedInLatestIteration() { + return dataSizeMovedBytesInLatestIteration.value(); + } + + /** + * Increment data size moved in last iteration. Review Comment: ```suggestion * Increment data size moved in the last iteration. ``` ########## hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerMetrics.java: ########## @@ -154,6 +159,29 @@ public void resetDataSizeMovedGBInLatestIteration() { -getDataSizeMovedGBInLatestIteration()); } + /** + * Gets the amount of data moved by Container Balancer in the latest iteration. + * @return size in bytes + */ + public long getDataSizeMovedInLatestIteration() { + return dataSizeMovedBytesInLatestIteration.value(); + } + + /** + * Increment data size moved in last iteration. + * @param bytes bytes to add + */ + public void incrementDataSizeMovedInLatestIteration(long bytes) { + this.dataSizeMovedBytesInLatestIteration.incr(bytes); + } + + /** + * Reset data size moved in last iteration. Review Comment: ```suggestion * Reset data size moved in the last iteration. ``` ########## hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTask.java: ########## @@ -385,6 +389,14 @@ public List<ContainerBalancerTaskIterationStatusInfo> getCurrentIterationsStatis return resultList; } + private long getCurrentIterationDuration() { + if (currentIterationStarted == null) { + return -1L; Review Comment: Let's introduce a constant for clarity sake. ########## hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTaskIterationStatusInfo.java: ########## @@ -18,86 +18,184 @@ package org.apache.hadoop.hdds.scm.container.balancer; +import org.apache.hadoop.hdds.protocol.proto.StorageContainerLocationProtocolProtos; + +import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.UUID; +import java.util.stream.Collectors; /** * Information about balancer task iteration. */ public class ContainerBalancerTaskIterationStatusInfo { private final Integer iterationNumber; private final String iterationResult; - private final long sizeScheduledForMoveGB; - private final long dataSizeMovedGB; + private final long iterationDuration; + private final long sizeScheduledForMove; + private final long dataSizeMoved; private final long containerMovesScheduled; private final long containerMovesCompleted; private final long containerMovesFailed; private final long containerMovesTimeout; - private final Map<UUID, Long> sizeEnteringNodesGB; - private final Map<UUID, Long> sizeLeavingNodesGB; + private final Map<UUID, Long> sizeEnteringNodes; + private final Map<UUID, Long> sizeLeavingNodes; @SuppressWarnings("checkstyle:ParameterNumber") public ContainerBalancerTaskIterationStatusInfo( Integer iterationNumber, String iterationResult, - long sizeScheduledForMoveGB, - long dataSizeMovedGB, + long iterationDuration, + long sizeScheduledForMove, + long dataSizeMoved, long containerMovesScheduled, long containerMovesCompleted, long containerMovesFailed, long containerMovesTimeout, - Map<UUID, Long> sizeEnteringNodesGB, - Map<UUID, Long> sizeLeavingNodesGB) { + Map<UUID, Long> sizeEnteringNodes, + Map<UUID, Long> sizeLeavingNodes) { this.iterationNumber = iterationNumber; this.iterationResult = iterationResult; - this.sizeScheduledForMoveGB = sizeScheduledForMoveGB; - this.dataSizeMovedGB = dataSizeMovedGB; + this.iterationDuration = iterationDuration; + this.sizeScheduledForMove = sizeScheduledForMove; + this.dataSizeMoved = dataSizeMoved; this.containerMovesScheduled = containerMovesScheduled; this.containerMovesCompleted = containerMovesCompleted; this.containerMovesFailed = containerMovesFailed; this.containerMovesTimeout = containerMovesTimeout; - this.sizeEnteringNodesGB = sizeEnteringNodesGB; - this.sizeLeavingNodesGB = sizeLeavingNodesGB; + this.sizeEnteringNodes = sizeEnteringNodes; + this.sizeLeavingNodes = sizeLeavingNodes; } + /** + * Get number of iteration. + * @return iteration number + */ public Integer getIterationNumber() { return iterationNumber; } + /** + * Get iteration result. + * @return iteration result + */ public String getIterationResult() { return iterationResult; } - public long getSizeScheduledForMoveGB() { - return sizeScheduledForMoveGB; + /** + * Get size in bytes scheduled to move in iteration. + * @return size in bytes + */ + public long getSizeScheduledForMove() { + return sizeScheduledForMove; } - public long getDataSizeMovedGB() { - return dataSizeMovedGB; + /** + * Get size in bytes moved in iteration. Review Comment: ```suggestion * Get size in bytes moved in the iteration. ``` ########## hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTask.java: ########## @@ -307,53 +311,53 @@ private void balance() { } private void saveIterationStatistic(Integer iterationNumber, IterationResult iR) { + long iterationDuration = now().toEpochSecond() - currentIterationStarted.toEpochSecond(); ContainerBalancerTaskIterationStatusInfo iterationStatistic = new ContainerBalancerTaskIterationStatusInfo( - iterationNumber, - iR.name(), - getSizeScheduledForMoveInLatestIteration() / OzoneConsts.GB, - metrics.getDataSizeMovedGBInLatestIteration(), - metrics.getNumContainerMovesScheduledInLatestIteration(), - metrics.getNumContainerMovesCompletedInLatestIteration(), - metrics.getNumContainerMovesFailedInLatestIteration(), - metrics.getNumContainerMovesTimeoutInLatestIteration(), - findTargetStrategy.getSizeEnteringNodes() - .entrySet() - .stream() - .filter(Objects::nonNull) - .filter(datanodeDetailsLongEntry -> datanodeDetailsLongEntry.getValue() > 0) - .collect( - Collectors.toMap( - entry -> entry.getKey().getUuid(), - entry -> entry.getValue() / OzoneConsts.GB - ) - ), - findSourceStrategy.getSizeLeavingNodes() - .entrySet() - .stream() - .filter(Objects::nonNull) - .filter(datanodeDetailsLongEntry -> datanodeDetailsLongEntry.getValue() > 0) - .collect( - Collectors.toMap( - entry -> entry.getKey().getUuid(), - entry -> entry.getValue() / OzoneConsts.GB - ) - ) + iterationNumber, + iR.name(), + iterationDuration, + getSizeScheduledForMoveInLatestIteration(), + metrics.getDataSizeMovedInLatestIteration(), + metrics.getNumContainerMovesScheduledInLatestIteration(), + metrics.getNumContainerMovesCompletedInLatestIteration(), + metrics.getNumContainerMovesFailedInLatestIteration(), + metrics.getNumContainerMovesTimeoutInLatestIteration(), + findTargetStrategy.getSizeEnteringNodes() + .entrySet() + .stream() + .filter(Objects::nonNull) + .filter(datanodeDetailsLongEntry -> datanodeDetailsLongEntry.getValue() > 0) + .collect( + Collectors.toMap( + entry -> entry.getKey().getUuid(), + Map.Entry::getValue + ) + ), + findSourceStrategy.getSizeLeavingNodes() + .entrySet() + .stream() + .filter(Objects::nonNull) + .filter(datanodeDetailsLongEntry -> datanodeDetailsLongEntry.getValue() > 0) + .collect( + Collectors.toMap( + entry -> entry.getKey().getUuid(), + Map.Entry::getValue + ) + ) ); iterationsStatistic.add(iterationStatistic); } public List<ContainerBalancerTaskIterationStatusInfo> getCurrentIterationsStatistic() { - - int lastIterationNumber = iterationsStatistic.stream() - .mapToInt(ContainerBalancerTaskIterationStatusInfo::getIterationNumber) - .max() - .orElse(0); - + int lastIterationNumber = iterationsStatistic.isEmpty() ? 0 + : iterationsStatistic.get(iterationsStatistic.size() - 1).getIterationNumber(); Review Comment: ```suggestion : iterationsStatistic.get(--iterationsStatistic.size()).getIterationNumber(); ``` ########## hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTaskIterationStatusInfo.java: ########## @@ -18,86 +18,184 @@ package org.apache.hadoop.hdds.scm.container.balancer; +import org.apache.hadoop.hdds.protocol.proto.StorageContainerLocationProtocolProtos; + +import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.UUID; +import java.util.stream.Collectors; /** * Information about balancer task iteration. */ public class ContainerBalancerTaskIterationStatusInfo { private final Integer iterationNumber; private final String iterationResult; - private final long sizeScheduledForMoveGB; - private final long dataSizeMovedGB; + private final long iterationDuration; + private final long sizeScheduledForMove; + private final long dataSizeMoved; private final long containerMovesScheduled; private final long containerMovesCompleted; private final long containerMovesFailed; private final long containerMovesTimeout; - private final Map<UUID, Long> sizeEnteringNodesGB; - private final Map<UUID, Long> sizeLeavingNodesGB; + private final Map<UUID, Long> sizeEnteringNodes; + private final Map<UUID, Long> sizeLeavingNodes; @SuppressWarnings("checkstyle:ParameterNumber") public ContainerBalancerTaskIterationStatusInfo( Integer iterationNumber, String iterationResult, - long sizeScheduledForMoveGB, - long dataSizeMovedGB, + long iterationDuration, + long sizeScheduledForMove, + long dataSizeMoved, long containerMovesScheduled, long containerMovesCompleted, long containerMovesFailed, long containerMovesTimeout, - Map<UUID, Long> sizeEnteringNodesGB, - Map<UUID, Long> sizeLeavingNodesGB) { + Map<UUID, Long> sizeEnteringNodes, + Map<UUID, Long> sizeLeavingNodes) { this.iterationNumber = iterationNumber; this.iterationResult = iterationResult; - this.sizeScheduledForMoveGB = sizeScheduledForMoveGB; - this.dataSizeMovedGB = dataSizeMovedGB; + this.iterationDuration = iterationDuration; + this.sizeScheduledForMove = sizeScheduledForMove; + this.dataSizeMoved = dataSizeMoved; this.containerMovesScheduled = containerMovesScheduled; this.containerMovesCompleted = containerMovesCompleted; this.containerMovesFailed = containerMovesFailed; this.containerMovesTimeout = containerMovesTimeout; - this.sizeEnteringNodesGB = sizeEnteringNodesGB; - this.sizeLeavingNodesGB = sizeLeavingNodesGB; + this.sizeEnteringNodes = sizeEnteringNodes; + this.sizeLeavingNodes = sizeLeavingNodes; } + /** + * Get number of iteration. Review Comment: ```suggestion * Get number of iterations. ``` ########## hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTask.java: ########## @@ -262,7 +266,7 @@ private void balance() { } IterationResult iR = doIteration(); - saveIterationStatistic(i, iR); + saveIterationStatistic(i + 1, iR); Review Comment: ```suggestion saveIterationStatistic(++i, iR); ``` ########## hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTaskIterationStatusInfo.java: ########## @@ -18,86 +18,184 @@ package org.apache.hadoop.hdds.scm.container.balancer; +import org.apache.hadoop.hdds.protocol.proto.StorageContainerLocationProtocolProtos; + +import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.UUID; +import java.util.stream.Collectors; /** * Information about balancer task iteration. */ public class ContainerBalancerTaskIterationStatusInfo { private final Integer iterationNumber; private final String iterationResult; - private final long sizeScheduledForMoveGB; - private final long dataSizeMovedGB; + private final long iterationDuration; + private final long sizeScheduledForMove; + private final long dataSizeMoved; private final long containerMovesScheduled; private final long containerMovesCompleted; private final long containerMovesFailed; private final long containerMovesTimeout; - private final Map<UUID, Long> sizeEnteringNodesGB; - private final Map<UUID, Long> sizeLeavingNodesGB; + private final Map<UUID, Long> sizeEnteringNodes; + private final Map<UUID, Long> sizeLeavingNodes; @SuppressWarnings("checkstyle:ParameterNumber") public ContainerBalancerTaskIterationStatusInfo( Integer iterationNumber, String iterationResult, - long sizeScheduledForMoveGB, - long dataSizeMovedGB, + long iterationDuration, + long sizeScheduledForMove, + long dataSizeMoved, long containerMovesScheduled, long containerMovesCompleted, long containerMovesFailed, long containerMovesTimeout, - Map<UUID, Long> sizeEnteringNodesGB, - Map<UUID, Long> sizeLeavingNodesGB) { + Map<UUID, Long> sizeEnteringNodes, + Map<UUID, Long> sizeLeavingNodes) { this.iterationNumber = iterationNumber; this.iterationResult = iterationResult; - this.sizeScheduledForMoveGB = sizeScheduledForMoveGB; - this.dataSizeMovedGB = dataSizeMovedGB; + this.iterationDuration = iterationDuration; + this.sizeScheduledForMove = sizeScheduledForMove; + this.dataSizeMoved = dataSizeMoved; this.containerMovesScheduled = containerMovesScheduled; this.containerMovesCompleted = containerMovesCompleted; this.containerMovesFailed = containerMovesFailed; this.containerMovesTimeout = containerMovesTimeout; - this.sizeEnteringNodesGB = sizeEnteringNodesGB; - this.sizeLeavingNodesGB = sizeLeavingNodesGB; + this.sizeEnteringNodes = sizeEnteringNodes; + this.sizeLeavingNodes = sizeLeavingNodes; } + /** + * Get number of iteration. + * @return iteration number + */ public Integer getIterationNumber() { return iterationNumber; } + /** + * Get iteration result. + * @return iteration result + */ public String getIterationResult() { return iterationResult; } - public long getSizeScheduledForMoveGB() { - return sizeScheduledForMoveGB; + /** + * Get size in bytes scheduled to move in iteration. + * @return size in bytes + */ + public long getSizeScheduledForMove() { + return sizeScheduledForMove; } - public long getDataSizeMovedGB() { - return dataSizeMovedGB; + /** + * Get size in bytes moved in iteration. + * @return size in bytes + */ + public long getDataSizeMoved() { + return dataSizeMoved; } + /** + * Get number of scheduled containers to move. Review Comment: ```suggestion * Get number of containers scheduled to move. ``` ########## hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTaskIterationStatusInfo.java: ########## @@ -18,86 +18,184 @@ package org.apache.hadoop.hdds.scm.container.balancer; +import org.apache.hadoop.hdds.protocol.proto.StorageContainerLocationProtocolProtos; + +import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.UUID; +import java.util.stream.Collectors; /** * Information about balancer task iteration. */ public class ContainerBalancerTaskIterationStatusInfo { private final Integer iterationNumber; private final String iterationResult; - private final long sizeScheduledForMoveGB; - private final long dataSizeMovedGB; + private final long iterationDuration; + private final long sizeScheduledForMove; + private final long dataSizeMoved; private final long containerMovesScheduled; private final long containerMovesCompleted; private final long containerMovesFailed; private final long containerMovesTimeout; - private final Map<UUID, Long> sizeEnteringNodesGB; - private final Map<UUID, Long> sizeLeavingNodesGB; + private final Map<UUID, Long> sizeEnteringNodes; + private final Map<UUID, Long> sizeLeavingNodes; @SuppressWarnings("checkstyle:ParameterNumber") public ContainerBalancerTaskIterationStatusInfo( Integer iterationNumber, String iterationResult, - long sizeScheduledForMoveGB, - long dataSizeMovedGB, + long iterationDuration, + long sizeScheduledForMove, + long dataSizeMoved, long containerMovesScheduled, long containerMovesCompleted, long containerMovesFailed, long containerMovesTimeout, - Map<UUID, Long> sizeEnteringNodesGB, - Map<UUID, Long> sizeLeavingNodesGB) { + Map<UUID, Long> sizeEnteringNodes, + Map<UUID, Long> sizeLeavingNodes) { this.iterationNumber = iterationNumber; this.iterationResult = iterationResult; - this.sizeScheduledForMoveGB = sizeScheduledForMoveGB; - this.dataSizeMovedGB = dataSizeMovedGB; + this.iterationDuration = iterationDuration; + this.sizeScheduledForMove = sizeScheduledForMove; + this.dataSizeMoved = dataSizeMoved; this.containerMovesScheduled = containerMovesScheduled; this.containerMovesCompleted = containerMovesCompleted; this.containerMovesFailed = containerMovesFailed; this.containerMovesTimeout = containerMovesTimeout; - this.sizeEnteringNodesGB = sizeEnteringNodesGB; - this.sizeLeavingNodesGB = sizeLeavingNodesGB; + this.sizeEnteringNodes = sizeEnteringNodes; + this.sizeLeavingNodes = sizeLeavingNodes; } + /** + * Get number of iteration. + * @return iteration number + */ public Integer getIterationNumber() { return iterationNumber; } + /** + * Get iteration result. + * @return iteration result + */ public String getIterationResult() { return iterationResult; } - public long getSizeScheduledForMoveGB() { - return sizeScheduledForMoveGB; + /** + * Get size in bytes scheduled to move in iteration. Review Comment: ```suggestion * Get size in bytes scheduled to move in the iteration. ``` ########## hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTaskIterationStatusInfo.java: ########## @@ -18,86 +18,184 @@ package org.apache.hadoop.hdds.scm.container.balancer; +import org.apache.hadoop.hdds.protocol.proto.StorageContainerLocationProtocolProtos; + +import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.UUID; +import java.util.stream.Collectors; /** * Information about balancer task iteration. */ public class ContainerBalancerTaskIterationStatusInfo { private final Integer iterationNumber; private final String iterationResult; - private final long sizeScheduledForMoveGB; - private final long dataSizeMovedGB; + private final long iterationDuration; + private final long sizeScheduledForMove; + private final long dataSizeMoved; private final long containerMovesScheduled; private final long containerMovesCompleted; private final long containerMovesFailed; private final long containerMovesTimeout; - private final Map<UUID, Long> sizeEnteringNodesGB; - private final Map<UUID, Long> sizeLeavingNodesGB; + private final Map<UUID, Long> sizeEnteringNodes; + private final Map<UUID, Long> sizeLeavingNodes; @SuppressWarnings("checkstyle:ParameterNumber") public ContainerBalancerTaskIterationStatusInfo( Integer iterationNumber, String iterationResult, - long sizeScheduledForMoveGB, - long dataSizeMovedGB, + long iterationDuration, + long sizeScheduledForMove, + long dataSizeMoved, long containerMovesScheduled, long containerMovesCompleted, long containerMovesFailed, long containerMovesTimeout, - Map<UUID, Long> sizeEnteringNodesGB, - Map<UUID, Long> sizeLeavingNodesGB) { + Map<UUID, Long> sizeEnteringNodes, + Map<UUID, Long> sizeLeavingNodes) { this.iterationNumber = iterationNumber; this.iterationResult = iterationResult; - this.sizeScheduledForMoveGB = sizeScheduledForMoveGB; - this.dataSizeMovedGB = dataSizeMovedGB; + this.iterationDuration = iterationDuration; + this.sizeScheduledForMove = sizeScheduledForMove; + this.dataSizeMoved = dataSizeMoved; this.containerMovesScheduled = containerMovesScheduled; this.containerMovesCompleted = containerMovesCompleted; this.containerMovesFailed = containerMovesFailed; this.containerMovesTimeout = containerMovesTimeout; - this.sizeEnteringNodesGB = sizeEnteringNodesGB; - this.sizeLeavingNodesGB = sizeLeavingNodesGB; + this.sizeEnteringNodes = sizeEnteringNodes; + this.sizeLeavingNodes = sizeLeavingNodes; } + /** + * Get number of iteration. + * @return iteration number + */ public Integer getIterationNumber() { return iterationNumber; } + /** + * Get iteration result. + * @return iteration result + */ public String getIterationResult() { return iterationResult; } - public long getSizeScheduledForMoveGB() { - return sizeScheduledForMoveGB; + /** + * Get size in bytes scheduled to move in iteration. + * @return size in bytes + */ + public long getSizeScheduledForMove() { + return sizeScheduledForMove; } - public long getDataSizeMovedGB() { - return dataSizeMovedGB; + /** + * Get size in bytes moved in iteration. + * @return size in bytes + */ + public long getDataSizeMoved() { + return dataSizeMoved; } + /** + * Get number of scheduled containers to move. + * @return number of scheduled containers to move Review Comment: ```suggestion * @return number of containers scheduled to move ``` -- 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: issues-unsubscr...@ozone.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@ozone.apache.org For additional commands, e-mail: issues-h...@ozone.apache.org