[
https://issues.apache.org/jira/browse/HDFS-16846?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17639760#comment-17639760
]
ASF GitHub Bot commented on HDFS-16846:
---------------------------------------
tasanuma commented on code in PR #5143:
URL: https://github.com/apache/hadoop/pull/5143#discussion_r1033087189
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestDatanodeManager.java:
##########
@@ -1009,13 +1011,25 @@ private void verifyPendingRecoveryTasks(
.thenReturn(tasks);
}
- if (numECBlocks > 0) {
+ if (numEcBlocksToBeReplicated > 0) {
+ Mockito.when(nodeInfo.getNumberOfECBlocksToBeReplicated())
+ .thenReturn(numEcBlocksToBeReplicated);
+
+ List<BlockTargetPair> ecReplicatedTasks =
+ Collections.nCopies(
+ Math.min(numECTasksToBeReplicated,
numEcBlocksToBeReplicated),
+ new BlockTargetPair(null, null));
+ Mockito.when(nodeInfo.getReplicationCommand(numECTasksToBeReplicated))
Review Comment:
```suggestion
Mockito.when(nodeInfo.getErasureCodeCommand(numECTasksToBeReplicated))
```
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestDatanodeManager.java:
##########
@@ -1040,28 +1054,28 @@ private void verifyPendingRecoveryTasks(
idx++;
}
- if (numECTasks > 0) {
+ if (numECTasksToBeErasureCoded > 0) {
assertTrue(cmds[idx] instanceof BlockECReconstructionCommand);
BlockECReconstructionCommand cmd =
(BlockECReconstructionCommand) cmds[idx];
- assertEquals(numECTasks, cmd.getECTasks().size());
+ assertEquals(numECTasksToBeErasureCoded, cmd.getECTasks().size());
}
Mockito.verify(nodeInfo).getReplicationCommand(numReplicationTasks);
- Mockito.verify(nodeInfo).getErasureCodeCommand(numECTasks);
+ Mockito.verify(nodeInfo).getErasureCodeCommand(numECTasksToBeErasureCoded);
Review Comment:
```suggestion
Mockito.verify(nodeInfo).getECReplicatedCommand(numECTasksToBeReplicated);
Mockito.verify(nodeInfo).getErasureCodeCommand(numECTasksToBeErasureCoded);
```
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestDatanodeManager.java:
##########
@@ -1026,7 +1040,7 @@ private void verifyPendingRecoveryTasks(
SlowPeerReports.EMPTY_REPORT, SlowDiskReports.EMPTY_REPORT);
long expectedNumCmds = Arrays.stream(
- new int[]{numReplicationTasks, numECTasks})
+ new int[]{numReplicationTasks, numECTasksToBeErasureCoded})
Review Comment:
We need to consider `numEcBlocksToBeReplicated`.
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestDatanodeManager.java:
##########
@@ -1040,28 +1054,28 @@ private void verifyPendingRecoveryTasks(
idx++;
}
- if (numECTasks > 0) {
+ if (numECTasksToBeErasureCoded > 0) {
assertTrue(cmds[idx] instanceof BlockECReconstructionCommand);
BlockECReconstructionCommand cmd =
(BlockECReconstructionCommand) cmds[idx];
- assertEquals(numECTasks, cmd.getECTasks().size());
+ assertEquals(numECTasksToBeErasureCoded, cmd.getECTasks().size());
}
Mockito.verify(nodeInfo).getReplicationCommand(numReplicationTasks);
- Mockito.verify(nodeInfo).getErasureCodeCommand(numECTasks);
+ Mockito.verify(nodeInfo).getErasureCodeCommand(numECTasksToBeErasureCoded);
}
@Test
public void testPendingRecoveryTasks() throws IOException {
// Tasks are slitted according to the ratio between queue lengths.
- verifyPendingRecoveryTasks(20, 20, 20, 30, 10, 10, false);
- verifyPendingRecoveryTasks(40, 10, 20, 30, 16, 4, false);
+ verifyPendingRecoveryTasks(20, 0, 20, 20, 30, 10, 0, 10, false);
+ verifyPendingRecoveryTasks(40, 0, 10, 20, 30, 16, 0, 4, false);
// Approximately load tasks if the ratio between queue length is large.
- verifyPendingRecoveryTasks(400, 1, 20, 30, 20, 1, false);
+ verifyPendingRecoveryTasks(400, 0, 1, 20, 30, 20, 0, 1, false);
// Tasks use dfs.namenode.replication.max-streams-hard-limit for
decommissioning node
- verifyPendingRecoveryTasks(30, 30, 20, 30, 15, 15, true);
+ verifyPendingRecoveryTasks(20, 10, 10, 20, 40, 10, 10, 5, true);
Review Comment:
Could you please add more test cases? Currently, the following case fails
due to the problem of `verifyPendingRecoveryTasks`.
```java
verifyPendingRecoveryTasks(0, 10, 10, 20, 40, 0, 10, 5, true);
```
> EC: Only EC blocks should be effected by max-streams-hard-limit configuration
> -----------------------------------------------------------------------------
>
> Key: HDFS-16846
> URL: https://issues.apache.org/jira/browse/HDFS-16846
> Project: Hadoop HDFS
> Issue Type: Improvement
> Components: ec
> Affects Versions: 3.4.0
> Reporter: caozhiqiang
> Assignee: caozhiqiang
> Priority: Major
> Labels: pull-request-available
>
> In [HDFS-16613|https://issues.apache.org/jira/browse/HDFS-16613], the
> dfs.namenode.replication.max-streams-hard-limit configuration will only
> affect decommissioning DataNode, but will not distinguish between replication
> blocks and EC blocks. Even if DataNodes have only replication files, they
> will always generate high network traffic. So this configuration should only
> effect EC blocks.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
