[ 
https://issues.apache.org/jira/browse/HDFS-17284?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17800328#comment-17800328
 ] 

ASF GitHub Bot commented on HDFS-17284:
---------------------------------------

tasanuma commented on code in PR #6348:
URL: https://github.com/apache/hadoop/pull/6348#discussion_r1436119492


##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestDatanodeManager.java:
##########
@@ -1126,4 +1128,66 @@ public MockDfsNetworkTopology(){
       super();
     }
   }
+
+  @Test
+  public void testComputeReconstructedTaskNum() throws IOException {
+    verifyComputeReconstructedTaskNum(100, 100, 150, 250, 100);
+    verifyComputeReconstructedTaskNum(200, 100000, 200000, 300000, 400000);
+    verifyComputeReconstructedTaskNum(1000000, 100, 150, 250, 100);
+    verifyComputeReconstructedTaskNum(14000000, 200, 200, 400, 200);
+
+  }
+  public void verifyComputeReconstructedTaskNum(int xmitsInProgress, int 
numReplicationBlocks,
+      int maxTransfers, int numECTasksToBeReplicated, int 
numBlocksToBeErasureCoded)
+      throws IOException {
+    FSNamesystem fsn = Mockito.mock(FSNamesystem.class);
+    Mockito.when(fsn.hasWriteLock()).thenReturn(true);
+    Configuration conf = new Configuration();
+    conf.setInt(DFSConfigKeys.DFS_NAMENODE_REPLICATION_MAX_STREAMS_KEY, 
maxTransfers);
+    DatanodeManager dm = Mockito.spy(mockDatanodeManager(fsn, conf));
+
+    DatanodeDescriptor nodeInfo = Mockito.mock(DatanodeDescriptor.class);
+    Mockito.when(nodeInfo.isRegistered()).thenReturn(true);
+    Mockito.when(nodeInfo.getStorageInfos()).thenReturn(new 
DatanodeStorageInfo[0]);
+
+    
Mockito.when(nodeInfo.getNumberOfReplicateBlocks()).thenReturn(numReplicationBlocks);
+    
Mockito.when(nodeInfo.getNumberOfECBlocksToBeReplicated()).thenReturn(numECTasksToBeReplicated);
+    Mockito.when(nodeInfo.getNumberOfBlocksToBeErasureCoded())
+        .thenReturn(numBlocksToBeErasureCoded);
+
+    // Create an ArgumentCaptor to capture the counts for numReplicationTasks,
+    // numEcReplicatedTasks,numECReconstructedTasks.
+    ArgumentCaptor<Integer> captor = ArgumentCaptor.forClass(Integer.class);
+    Mockito.when(nodeInfo.getErasureCodeCommand(ArgumentMatchers.anyInt()))
+        .thenReturn(Collections.nCopies(0, null));
+    Mockito.when(nodeInfo.getReplicationCommand(ArgumentMatchers.anyInt()))
+        .thenReturn(Collections.nCopies(0, null));
+    Mockito.when(nodeInfo.getECReplicatedCommand(ArgumentMatchers.anyInt()))
+        .thenReturn(Collections.nCopies(0, null));
+
+    DatanodeRegistration nodeReg = Mockito.mock(DatanodeRegistration.class);
+    Mockito.when(dm.getDatanode(nodeReg)).thenReturn(nodeInfo);
+
+
+    dm.handleHeartbeat(nodeReg, new StorageReport[1], "bp-123", 0, 0,
+        10, xmitsInProgress, 0, null, SlowPeerReports.EMPTY_REPORT,
+        SlowDiskReports.EMPTY_REPORT);
+
+    Mockito.verify(nodeInfo).getReplicationCommand(captor.capture());
+    int numReplicationTasks = captor.getValue();
+
+    Mockito.verify(nodeInfo).getECReplicatedCommand(captor.capture());
+    int numEcReplicatedTasks = captor.getValue();
+
+    Mockito.verify(nodeInfo).getErasureCodeCommand(captor.capture());
+    int numECReconstructedTasks = captor.getValue();
+
+    // Verify that when DN xmitsInProgress exceeds maxTransfers,
+    // the number of tasks should be <= 0.
+    if(xmitsInProgress >= maxTransfers){
+      assertTrue(numReplicationTasks <= 0);
+      assertTrue(numEcReplicatedTasks <= 0);
+      assertTrue(numECReconstructedTasks <= 0);
+    }

Review Comment:
   To ensure that `verifyComputeReconstructedTaskNum(200, 100000, 200000, 
300000, 400000)` doesn't cause an overflow, it would be necessary to check the 
reverse situation as well.
   ```suggestion
       } else {
         assertTrue(numReplicationTasks >= 0);
         assertTrue(numEcReplicatedTasks >= 0);
         assertTrue(numECReconstructedTasks >= 0);
       }
   ```
   
   Other than that, it looks good to me.





> Fix int overflow in calculating numEcReplicatedTasks and numReplicationTasks 
> during block recovery
> --------------------------------------------------------------------------------------------------
>
>                 Key: HDFS-17284
>                 URL: https://issues.apache.org/jira/browse/HDFS-17284
>             Project: Hadoop HDFS
>          Issue Type: Bug
>          Components: namenode
>            Reporter: Hualong Zhang
>            Assignee: Hualong Zhang
>            Priority: Major
>              Labels: pull-request-available
>
> Fix int overflow in calculating numEcReplicatedTasks and numReplicationTasks 
> during block recovery



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org

Reply via email to