ivanzlenko commented on code in PR #7139:
URL: https://github.com/apache/ozone/pull/7139#discussion_r1803399015
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerMetrics.java:
##########
@@ -131,14 +134,16 @@ void
incrementNumContainerMovesScheduledInLatestIteration(long valueToAdd) {
this.numContainerMovesScheduledInLatestIteration.incr(valueToAdd);
}
+ /**
+ * Reset number of containers scheduled to move in last iteration.
Review Comment:
```suggestion
* Reset the number of containers scheduled to move in the last iteration.
```
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTask.java:
##########
@@ -118,7 +121,7 @@ public class ContainerBalancerTask implements Runnable {
private int nextIterationIndex;
private boolean delayStart;
private List<ContainerBalancerTaskIterationStatusInfo> iterationsStatistic;
-
+ private OffsetDateTime currentIterationStarted;
Review Comment:
```suggestion
private OffsetDateTime currentIterationStarted;
```
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerMetrics.java:
##########
@@ -218,14 +243,19 @@ public long
getNumContainerMovesTimeoutInLatestIteration() {
return numContainerMovesTimeoutInLatestIteration.value();
}
+ /**
+ * Increment number timeouted container moves.
+ */
public void incrementNumContainerMovesTimeoutInLatestIteration(
long valueToAdd) {
this.numContainerMovesTimeoutInLatestIteration.incr(valueToAdd);
}
+ /**
+ * Reset number timeouted container moves.
Review Comment:
```suggestion
* Reset the number of containers which transfer finished with the timeout.
```
##########
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 iterations.
+ * @return iteration number
+ */
public Integer getIterationNumber() {
return iterationNumber;
}
+ /**
+ * Get iteration result.
Review Comment:
```suggestion
* Get the iteration result.
```
##########
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 iterations.
+ * @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 the iteration.
+ * @return size in bytes
+ */
+ public long getSizeScheduledForMove() {
+ return sizeScheduledForMove;
}
- public long getDataSizeMovedGB() {
- return dataSizeMovedGB;
+ /**
+ * Get size in bytes moved in the iteration.
+ * @return size in bytes
+ */
+ public long getDataSizeMoved() {
+ return dataSizeMoved;
}
+ /**
+ * Get number of containers scheduled to move.
+ * @return number of containers scheduled to move
+ */
public long getContainerMovesScheduled() {
return containerMovesScheduled;
}
+ /**
+ * Get number of successfully moved containers.
+ * @return number of successfully moved containers
+ */
public long getContainerMovesCompleted() {
return containerMovesCompleted;
}
+ /**
+ * Get number of unsuccessfully moved containers.
+ * @return number of unsuccessfully moved containers
+ */
public long getContainerMovesFailed() {
return containerMovesFailed;
}
+ /**
+ * Get number of moved with timeout containers.
Review Comment:
```suggestion
* Get the number of moved with timeout containers.
```
##########
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 the last iteration.
Review Comment:
```suggestion
* Reset the number of containers containers moved in the last iteration.
```
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTask.java:
##########
@@ -365,7 +369,7 @@ public List<ContainerBalancerTaskIterationStatusInfo>
getCurrentIterationsStatis
.filter(datanodeDetailsLongEntry ->
datanodeDetailsLongEntry.getValue() > 0)
.collect(Collectors.toMap(
entry -> entry.getKey().getUuid(),
- entry -> entry.getValue() / OzoneConsts.GB
+ Map.Entry::getValue
)
),
findSourceStrategy.getSizeLeavingNodes()
Review Comment:
Can we move it into separate variable? The same for above.
##########
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 iterations.
+ * @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 the iteration.
+ * @return size in bytes
+ */
+ public long getSizeScheduledForMove() {
+ return sizeScheduledForMove;
}
- public long getDataSizeMovedGB() {
- return dataSizeMovedGB;
+ /**
+ * Get size in bytes moved in the iteration.
+ * @return size in bytes
+ */
+ public long getDataSizeMoved() {
+ return dataSizeMoved;
}
+ /**
+ * Get number of containers scheduled to move.
+ * @return number of containers scheduled to move
+ */
public long getContainerMovesScheduled() {
return containerMovesScheduled;
}
+ /**
+ * Get number of successfully moved containers.
+ * @return number of successfully moved containers
+ */
public long getContainerMovesCompleted() {
return containerMovesCompleted;
}
+ /**
+ * Get number of unsuccessfully moved containers.
Review Comment:
```suggestion
* Get the number of unsuccessfully moved containers.
```
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTask.java:
##########
@@ -118,7 +121,7 @@ public class ContainerBalancerTask implements Runnable {
private int nextIterationIndex;
private boolean delayStart;
private List<ContainerBalancerTaskIterationStatusInfo> iterationsStatistic;
-
+ private OffsetDateTime currentIterationStarted;
Review Comment:
Doesn't feel thread-safe
##########
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
Review Comment:
```suggestion
int lastIterationNumber = iterationsStatistic.isEmpty()
? 0
```
##########
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) {
Review Comment:
```suggestion
private void saveIterationStatistic(Integer iterationNumber,
IterationResult iterationResult) {
```
##########
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(),
Review Comment:
Why not just pass metrics here?
##########
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 iterations.
Review Comment:
```suggestion
* Get the number of iterations.
```
##########
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 iterations.
+ * @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 the iteration.
+ * @return size in bytes
+ */
+ public long getSizeScheduledForMove() {
+ return sizeScheduledForMove;
}
- public long getDataSizeMovedGB() {
- return dataSizeMovedGB;
+ /**
+ * Get size in bytes moved in the iteration.
+ * @return size in bytes
+ */
+ public long getDataSizeMoved() {
+ return dataSizeMoved;
}
+ /**
+ * Get number of containers scheduled to move.
Review Comment:
```suggestion
* Get the number of containers scheduled to move.
```
##########
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()
Review Comment:
Can we calculate this in a separate variable?
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerMetrics.java:
##########
@@ -218,14 +243,19 @@ public long
getNumContainerMovesTimeoutInLatestIteration() {
return numContainerMovesTimeoutInLatestIteration.value();
}
+ /**
+ * Increment number timeouted container moves.
Review Comment:
```suggestion
* Increase the number of containers which transfer finished with the
timeout.
```
##########
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 iterations.
+ * @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 the iteration.
+ * @return size in bytes
+ */
+ public long getSizeScheduledForMove() {
+ return sizeScheduledForMove;
}
- public long getDataSizeMovedGB() {
- return dataSizeMovedGB;
+ /**
+ * Get size in bytes moved in the iteration.
+ * @return size in bytes
+ */
+ public long getDataSizeMoved() {
+ return dataSizeMoved;
}
+ /**
+ * Get number of containers scheduled to move.
+ * @return number of containers scheduled to move
+ */
public long getContainerMovesScheduled() {
return containerMovesScheduled;
}
+ /**
+ * Get number of successfully moved containers.
+ * @return number of successfully moved containers
+ */
public long getContainerMovesCompleted() {
return containerMovesCompleted;
}
+ /**
+ * Get number of unsuccessfully moved containers.
+ * @return number of unsuccessfully moved containers
+ */
public long getContainerMovesFailed() {
return containerMovesFailed;
}
+ /**
+ * Get number of moved with timeout containers.
+ * @return number of moved with timeout containers
+ */
public long getContainerMovesTimeout() {
return containerMovesTimeout;
}
- public Map<UUID, Long> getSizeEnteringNodesGB() {
- return sizeEnteringNodesGB;
+ /**
+ * Get nodeId to size entering from node map.
Review Comment:
Can you re-phrase this? I had a very hard time to understand the meaning
behind this javaDoc and method in general.
##########
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 iterations.
+ * @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 the iteration.
+ * @return size in bytes
+ */
+ public long getSizeScheduledForMove() {
+ return sizeScheduledForMove;
}
- public long getDataSizeMovedGB() {
- return dataSizeMovedGB;
+ /**
+ * Get size in bytes moved in the iteration.
+ * @return size in bytes
+ */
+ public long getDataSizeMoved() {
+ return dataSizeMoved;
}
+ /**
+ * Get number of containers scheduled to move.
+ * @return number of containers scheduled to move
+ */
public long getContainerMovesScheduled() {
return containerMovesScheduled;
}
+ /**
+ * Get number of successfully moved containers.
+ * @return number of successfully moved containers
+ */
public long getContainerMovesCompleted() {
return containerMovesCompleted;
}
+ /**
+ * Get number of unsuccessfully moved containers.
+ * @return number of unsuccessfully moved containers
+ */
public long getContainerMovesFailed() {
return containerMovesFailed;
}
+ /**
+ * Get number of moved with timeout containers.
+ * @return number of moved with timeout containers
+ */
public long getContainerMovesTimeout() {
return containerMovesTimeout;
}
- public Map<UUID, Long> getSizeEnteringNodesGB() {
- return sizeEnteringNodesGB;
+ /**
+ * Get nodeId to size entering from node map.
+ * @return nodeId to size entering from node map
+ */
+ public Map<UUID, Long> getSizeEnteringNodes() {
+ return sizeEnteringNodes;
+ }
+
+ /**
+ * Get nodeId to size leaving from node map.
Review Comment:
Can you re-phrase this? I had a very hard time to understand the meaning
behind this javaDoc and method in general.
##########
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 iterations.
+ * @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 the iteration.
+ * @return size in bytes
+ */
+ public long getSizeScheduledForMove() {
+ return sizeScheduledForMove;
}
- public long getDataSizeMovedGB() {
- return dataSizeMovedGB;
+ /**
+ * Get size in bytes moved in the iteration.
+ * @return size in bytes
+ */
+ public long getDataSizeMoved() {
+ return dataSizeMoved;
}
+ /**
+ * Get number of containers scheduled to move.
+ * @return number of containers scheduled to move
+ */
public long getContainerMovesScheduled() {
return containerMovesScheduled;
}
+ /**
+ * Get number of successfully moved containers.
Review Comment:
```suggestion
* Get the number of successfully moved containers.
```
##########
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()
Review Comment:
Can we calculate this in a separate variable?
--
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]
