siddhantsangwan commented on code in PR #8491:
URL: https://github.com/apache/ozone/pull/8491#discussion_r2097757164


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/MoveManager.java:
##########
@@ -264,10 +259,9 @@ CompletableFuture<MoveResult> move(
    */
   private void notifyContainerOpCompleted(ContainerReplicaOp 
containerReplicaOp,
       ContainerID containerID) {
-    Pair<CompletableFuture<MoveResult>, MoveDataNodePair> pair =
-        pendingMoves.get(containerID);
+    MoveOperation pair = pendingMoves.get(containerID);

Review Comment:
   It's not a `pair` anymore, so I suggest renaming the variable to something 
more suitable.



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/MoveManager.java:
##########
@@ -411,17 +404,18 @@ private void sendReplicateCommand(
    *
    * @param containerInfo Container to be deleted
    * @param datanode The datanode on which the replica should be deleted
+   * @param scheduledTime The time at which the replicate command for the 
container was scheduled
    */
   private void sendDeleteCommand(
-      final ContainerInfo containerInfo, final DatanodeDetails datanode)
+      final ContainerInfo containerInfo, final DatanodeDetails datanode,
+      long scheduledTime)

Review Comment:
   ```suggestion
         long moveStartTime)
   ```



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/MoveManager.java:
##########
@@ -411,17 +404,18 @@ private void sendReplicateCommand(
    *
    * @param containerInfo Container to be deleted
    * @param datanode The datanode on which the replica should be deleted
+   * @param scheduledTime The time at which the replicate command for the 
container was scheduled
    */
   private void sendDeleteCommand(
-      final ContainerInfo containerInfo, final DatanodeDetails datanode)
+      final ContainerInfo containerInfo, final DatanodeDetails datanode,
+      long scheduledTime)
       throws ContainerReplicaNotFoundException, ContainerNotFoundException,
       NotLeaderException {
     int replicaIndex = getContainerReplicaIndex(
         containerInfo.containerID(), datanode);
     long deleteTimeout = moveTimeout - replicationTimeout;
-    long now = clock.millis();
     replicationManager.sendDeleteCommand(
-        containerInfo, replicaIndex, datanode, true, now + deleteTimeout);
+        containerInfo, replicaIndex, datanode, true, scheduledTime + 
deleteTimeout);

Review Comment:
   The deadline should be `moveStartTime + moveTimeout`, we don't need 
`deleteTimeout`.



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/MoveManager.java:
##########
@@ -454,6 +448,45 @@ void setReplicationTimeout(long replicationTimeout) {
     this.replicationTimeout = replicationTimeout;
   }
 
+  /**
+   * All details about a move operation.
+   */
+  private static class MoveOperation {
+    private CompletableFuture<MoveResult> result;
+    private MoveDataNodePair moveDataNodePair;
+    private long replicateScheduledTime;

Review Comment:
   ```suggestion
       private long moveStartTime;
   ```



-- 
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]

Reply via email to