tasanuma commented on a change in pull request #3753:
URL: https://github.com/apache/hadoop/pull/3753#discussion_r773564004
##########
File path:
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/metrics/DataNodeDiskMetrics.java
##########
@@ -127,6 +143,16 @@ public void run() {
detectAndUpdateDiskOutliers(metadataOpStats, readIoStats,
writeIoStats);
+
+ // Sort the slow disks by latency.
+ if (maxSlowDisksToBeExcluded > 0) {
+ ArrayList<DiskLatency> diskLatencies = new ArrayList<>();
+ for (Map.Entry<String, Map<DiskOp, Double>> diskStats :
+ diskOutliersStats.entrySet()) {
+ diskLatencies.add(new DiskLatency(diskStats.getKey(),
diskStats.getValue()));
+ }
+ sortSlowDisks(diskLatencies);
Review comment:
You could use `Collections.sort` to make it more concise.
```suggestion
Collections.sort(diskLatencies, (o1, o2)
-> Double.compare(o2.getMaxLatency(), o1.getMaxLatency()));
slowDisksToBeExcluded =
diskLatencies.stream().limit(maxSlowDisksToBeExcluded)
.map(DiskLatency::getSlowDisk).collect(Collectors.toList());
```
##########
File path:
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/metrics/DataNodeDiskMetrics.java
##########
@@ -127,6 +143,16 @@ public void run() {
detectAndUpdateDiskOutliers(metadataOpStats, readIoStats,
writeIoStats);
+
+ // Sort the slow disks by latency.
+ if (maxSlowDisksToBeExcluded > 0) {
+ ArrayList<DiskLatency> diskLatencies = new ArrayList<>();
+ for (Map.Entry<String, Map<DiskOp, Double>> diskStats :
+ diskOutliersStats.entrySet()) {
+ diskLatencies.add(new DiskLatency(diskStats.getKey(),
diskStats.getValue()));
+ }
+ sortSlowDisks(diskLatencies);
Review comment:
`sortSlowDisks` seems to do more than just sort slow disks, it also
limits them with `maxSlowDisksToBeExcluded` and sets them to
`slowDisksToBeExcluded`. It would be better to give this method a more
appropriate name.
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml
##########
@@ -2483,6 +2483,15 @@
</description>
</property>
+<property>
+ <name>dfs.datanode.max.slowdisks.to.be.excluded</name>
Review comment:
IMHO, the passive voice is a bit wordy for configurations. I prefer the
active voice, like `dfs.datanode.max.slowdisks.to.exclude`.
--
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]
