goiri commented on code in PR #4606:
URL: https://github.com/apache/hadoop/pull/4606#discussion_r926936268
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/metrics/RBFMetrics.java:
##########
@@ -544,28 +548,30 @@ public String getNodeUsage() {
final Map<String, Map<String, Object>> info = new HashMap<>();
try {
- RouterRpcServer rpcServer = this.router.getRpcServer();
- DatanodeInfo[] live = rpcServer.getDatanodeReport(
- DatanodeReportType.LIVE, false, timeOut);
-
- if (live.length > 0) {
- float totalDfsUsed = 0;
- float[] usages = new float[live.length];
- int i = 0;
- for (DatanodeInfo dn : live) {
- usages[i++] = dn.getDfsUsedPercent();
- totalDfsUsed += dn.getDfsUsedPercent();
- }
- totalDfsUsed /= live.length;
- Arrays.sort(usages);
- median = usages[usages.length / 2];
- max = usages[usages.length - 1];
- min = usages[0];
-
- for (i = 0; i < usages.length; i++) {
- dev += (usages[i] - totalDfsUsed) * (usages[i] - totalDfsUsed);
+ if (this.enableGetDNUsage) {
Review Comment:
I would do:
```
DatanodeInfo[] live = null;
if (this.enableGetDNUsage) {
RouterRpcServer rpcServer = this.router.getRpcServer();
DatanodeInfo[] live = rpcServer.getDatanodeReport(DatanodeReportType.LIVE,
false, timeOut);
} else {
LOG.debug("Getting information is disabled."); // similar message
}
if (live != null && live.length > 0) {
```
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/metrics/RBFMetrics.java:
##########
@@ -544,28 +548,30 @@ public String getNodeUsage() {
final Map<String, Map<String, Object>> info = new HashMap<>();
try {
- RouterRpcServer rpcServer = this.router.getRpcServer();
- DatanodeInfo[] live = rpcServer.getDatanodeReport(
- DatanodeReportType.LIVE, false, timeOut);
-
- if (live.length > 0) {
- float totalDfsUsed = 0;
- float[] usages = new float[live.length];
- int i = 0;
- for (DatanodeInfo dn : live) {
- usages[i++] = dn.getDfsUsedPercent();
- totalDfsUsed += dn.getDfsUsedPercent();
- }
- totalDfsUsed /= live.length;
- Arrays.sort(usages);
- median = usages[usages.length / 2];
- max = usages[usages.length - 1];
- min = usages[0];
-
- for (i = 0; i < usages.length; i++) {
- dev += (usages[i] - totalDfsUsed) * (usages[i] - totalDfsUsed);
+ if (this.enableGetDNUsage) {
+ RouterRpcServer rpcServer = this.router.getRpcServer();
+ DatanodeInfo[] live = rpcServer.getDatanodeReport(
+ DatanodeReportType.LIVE, false, timeOut);
+
+ if (live.length > 0) {
+ float totalDfsUsed = 0;
+ float[] usages = new float[live.length];
+ int i = 0;
+ for (DatanodeInfo dn : live) {
+ usages[i++] = dn.getDfsUsedPercent();
Review Comment:
What is the expensive part of this whole block? this?
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/metrics/RBFMetrics.java:
##########
@@ -544,28 +548,30 @@ public String getNodeUsage() {
final Map<String, Map<String, Object>> info = new HashMap<>();
try {
- RouterRpcServer rpcServer = this.router.getRpcServer();
- DatanodeInfo[] live = rpcServer.getDatanodeReport(
- DatanodeReportType.LIVE, false, timeOut);
-
- if (live.length > 0) {
- float totalDfsUsed = 0;
- float[] usages = new float[live.length];
- int i = 0;
- for (DatanodeInfo dn : live) {
- usages[i++] = dn.getDfsUsedPercent();
- totalDfsUsed += dn.getDfsUsedPercent();
- }
- totalDfsUsed /= live.length;
- Arrays.sort(usages);
- median = usages[usages.length / 2];
- max = usages[usages.length - 1];
- min = usages[0];
-
- for (i = 0; i < usages.length; i++) {
- dev += (usages[i] - totalDfsUsed) * (usages[i] - totalDfsUsed);
+ if (this.enableGetDNUsage) {
+ RouterRpcServer rpcServer = this.router.getRpcServer();
+ DatanodeInfo[] live = rpcServer.getDatanodeReport(
+ DatanodeReportType.LIVE, false, timeOut);
+
+ if (live.length > 0) {
+ float totalDfsUsed = 0;
+ float[] usages = new float[live.length];
+ int i = 0;
+ for (DatanodeInfo dn : live) {
+ usages[i++] = dn.getDfsUsedPercent();
+ totalDfsUsed += dn.getDfsUsedPercent();
+ }
+ totalDfsUsed /= live.length;
+ Arrays.sort(usages);
+ median = usages[usages.length / 2];
+ max = usages[usages.length - 1];
+ min = usages[0];
+
+ for (i = 0; i < usages.length; i++) {
+ dev += (usages[i] - totalDfsUsed) * (usages[i] - totalDfsUsed);
+ }
+ dev = (float) Math.sqrt(dev / usages.length);
Review Comment:
Apache commons math has a nice StandardDeviation utility.
It might be good to use this directly.
https://commons.apache.org/proper/commons-math/javadocs/api-3.6/org/apache/commons/math3/stat/descriptive/moment/StandardDeviation.html
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/metrics/RBFMetrics.java:
##########
@@ -544,28 +548,30 @@ public String getNodeUsage() {
final Map<String, Map<String, Object>> info = new HashMap<>();
try {
- RouterRpcServer rpcServer = this.router.getRpcServer();
- DatanodeInfo[] live = rpcServer.getDatanodeReport(
- DatanodeReportType.LIVE, false, timeOut);
-
- if (live.length > 0) {
- float totalDfsUsed = 0;
- float[] usages = new float[live.length];
- int i = 0;
- for (DatanodeInfo dn : live) {
- usages[i++] = dn.getDfsUsedPercent();
- totalDfsUsed += dn.getDfsUsedPercent();
- }
- totalDfsUsed /= live.length;
- Arrays.sort(usages);
- median = usages[usages.length / 2];
- max = usages[usages.length - 1];
- min = usages[0];
-
- for (i = 0; i < usages.length; i++) {
- dev += (usages[i] - totalDfsUsed) * (usages[i] - totalDfsUsed);
+ if (this.enableGetDNUsage) {
+ RouterRpcServer rpcServer = this.router.getRpcServer();
+ DatanodeInfo[] live = rpcServer.getDatanodeReport(
+ DatanodeReportType.LIVE, false, timeOut);
+
+ if (live.length > 0) {
+ float totalDfsUsed = 0;
+ float[] usages = new float[live.length];
+ int i = 0;
+ for (DatanodeInfo dn : live) {
+ usages[i++] = dn.getDfsUsedPercent();
Review Comment:
I guess is rpcServer.getDatanodeReport() actually.
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RBFConfigKeys.java:
##########
@@ -315,6 +315,9 @@ public class RBFConfigKeys extends
CommonConfigurationKeysPublic {
FEDERATION_ROUTER_PREFIX + "dn-report.cache-expire";
public static final long DN_REPORT_CACHE_EXPIRE_MS_DEFAULT =
TimeUnit.SECONDS.toMillis(10);
+ public static final String DFS_ROUTER_ENABLE_GET_DN_USAGE_KEY =
+ FEDERATION_ROUTER_PREFIX + "enable.get.dn.usage";
+ public static final boolean DFS_ROUTER_ENABLE_GET_DN_USAGE_DEFAULT = true;
Review Comment:
Can we add a test?
--
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]
