[
https://issues.apache.org/jira/browse/HDFS-16949?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17706504#comment-17706504
]
ASF GitHub Bot commented on HDFS-16949:
---------------------------------------
goiri commented on code in PR #5495:
URL: https://github.com/apache/hadoop/pull/5495#discussion_r1152180882
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/lib/MutableInverseQuantiles.java:
##########
@@ -0,0 +1,102 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.metrics2.lib;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.metrics2.MetricsInfo;
+import org.apache.hadoop.metrics2.util.Quantile;
+import org.apache.hadoop.metrics2.util.SampleQuantiles;
+import
org.apache.hadoop.thirdparty.com.google.common.util.concurrent.ThreadFactoryBuilder;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.TimeUnit;
+import static org.apache.hadoop.metrics2.lib.Interns.info;
+
+/**
+ * Watches a stream of long values, maintaining online estimates of specific
+ * quantiles with provably low error bounds. Inverse quantiles are meant for
+ * highly accurate low-percentile (e.g. 1st, 5th) latency metrics.
+ * InverseQuantiles are used for metrics where higher the value better it is.
+ * ( eg: data transfer rate ).
+ * The 1st percentile here corresponds to the 99th inverse percentile metric,
+ * 5th percentile to 95th and so on.
+ */
[email protected]
[email protected]
+public class MutableInverseQuantiles extends MutableQuantiles{
+
+ @VisibleForTesting
+ public static final Quantile[] INVERSE_QUANTILES = { new Quantile(0.50,
0.050),
+ new Quantile(0.25, 0.025), new Quantile(0.10, 0.010),
+ new Quantile(0.05, 0.005), new Quantile(0.01, 0.001) };
Review Comment:
It looks like you are using the same structure all the time: quantile,
error=quantile/10.
To make this more readable, I wonder if we could do something like:
```
class PercentileQuantile extend Quantile {
PercentileQuantile(double percentile) {
super(percentile/100.0, percentile/1000.0);
}
}
public static final Quantile[] INVERSE_QUANTILES = {
new PercentileQuantile(50), // 50th
new PercentileQuantile(25), // 25th
new PercentileQuantile(10), // 10th
new PercentileQuantile(5), // 5th
new PercentileQuantile(1), // 1th
}
```
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/lib/MutableInverseQuantiles.java:
##########
@@ -0,0 +1,102 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.metrics2.lib;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.metrics2.MetricsInfo;
+import org.apache.hadoop.metrics2.util.Quantile;
+import org.apache.hadoop.metrics2.util.SampleQuantiles;
+import
org.apache.hadoop.thirdparty.com.google.common.util.concurrent.ThreadFactoryBuilder;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.TimeUnit;
+import static org.apache.hadoop.metrics2.lib.Interns.info;
+
+/**
+ * Watches a stream of long values, maintaining online estimates of specific
+ * quantiles with provably low error bounds. Inverse quantiles are meant for
+ * highly accurate low-percentile (e.g. 1st, 5th) latency metrics.
+ * InverseQuantiles are used for metrics where higher the value better it is.
+ * ( eg: data transfer rate ).
+ * The 1st percentile here corresponds to the 99th inverse percentile metric,
+ * 5th percentile to 95th and so on.
+ */
[email protected]
[email protected]
+public class MutableInverseQuantiles extends MutableQuantiles{
+
+ @VisibleForTesting
+ public static final Quantile[] INVERSE_QUANTILES = { new Quantile(0.50,
0.050),
+ new Quantile(0.25, 0.025), new Quantile(0.10, 0.010),
+ new Quantile(0.05, 0.005), new Quantile(0.01, 0.001) };
+
+ private ScheduledFuture<?> scheduledTask;
+
+ private static final ScheduledExecutorService SCHEDULAR = Executors
+ .newScheduledThreadPool(1, new ThreadFactoryBuilder().setDaemon(true)
+ .setNameFormat("MutableInverseQuantiles-%d").build());
+
+ /**
+ * Instantiates a new {@link MutableInverseQuantiles} for a metric that
rolls itself
+ * over on the specified time interval.
+ *
+ * @param name of the metric
+ * @param description long-form textual description of the metric
+ * @param sampleName type of items in the stream (e.g., "Ops")
+ * @param valueName type of the values
+ * @param interval rollover interval (in seconds) of the estimator
Review Comment:
Add seconds to the variable name.
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/lib/MutableInverseQuantiles.java:
##########
@@ -0,0 +1,102 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.metrics2.lib;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.metrics2.MetricsInfo;
+import org.apache.hadoop.metrics2.util.Quantile;
+import org.apache.hadoop.metrics2.util.SampleQuantiles;
+import
org.apache.hadoop.thirdparty.com.google.common.util.concurrent.ThreadFactoryBuilder;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.TimeUnit;
+import static org.apache.hadoop.metrics2.lib.Interns.info;
+
+/**
+ * Watches a stream of long values, maintaining online estimates of specific
+ * quantiles with provably low error bounds. Inverse quantiles are meant for
+ * highly accurate low-percentile (e.g. 1st, 5th) latency metrics.
+ * InverseQuantiles are used for metrics where higher the value better it is.
+ * ( eg: data transfer rate ).
+ * The 1st percentile here corresponds to the 99th inverse percentile metric,
+ * 5th percentile to 95th and so on.
+ */
[email protected]
[email protected]
+public class MutableInverseQuantiles extends MutableQuantiles{
+
+ @VisibleForTesting
+ public static final Quantile[] INVERSE_QUANTILES = { new Quantile(0.50,
0.050),
+ new Quantile(0.25, 0.025), new Quantile(0.10, 0.010),
+ new Quantile(0.05, 0.005), new Quantile(0.01, 0.001) };
+
+ private ScheduledFuture<?> scheduledTask;
+
+ private static final ScheduledExecutorService SCHEDULAR = Executors
+ .newScheduledThreadPool(1, new ThreadFactoryBuilder().setDaemon(true)
+ .setNameFormat("MutableInverseQuantiles-%d").build());
+
+ /**
+ * Instantiates a new {@link MutableInverseQuantiles} for a metric that
rolls itself
+ * over on the specified time interval.
+ *
+ * @param name of the metric
+ * @param description long-form textual description of the metric
+ * @param sampleName type of items in the stream (e.g., "Ops")
+ * @param valueName type of the values
+ * @param interval rollover interval (in seconds) of the estimator
+ */
+ public MutableInverseQuantiles(String name, String description, String
sampleName,
+ String valueName, int interval) {
+ String ucName = StringUtils.capitalize(name);
+ String usName = StringUtils.capitalize(sampleName);
+ String uvName = StringUtils.capitalize(valueName);
+ String desc = StringUtils.uncapitalize(description);
+ String lsName = StringUtils.uncapitalize(sampleName);
+ String lvName = StringUtils.uncapitalize(valueName);
+
+ setNumInfo(info(ucName + "Num" + usName, String.format(
+ "Number of %s for %s with %ds interval", lsName, desc, interval)));
+ // Construct the MetricsInfos for the inverse quantiles, converting to
inverse percentiles
+ setQuantileInfos(INVERSE_QUANTILES.length);
+ String nameTemplate = ucName + "%dthInversePercentile" + uvName;
+ String descTemplate = "%d inverse percentile " + lvName + " with " +
interval
+ + " second interval for " + desc;
+ for (int i = 0; i < INVERSE_QUANTILES.length; i++) {
+ int inversePercentile = (int) (100 * (1 -
INVERSE_QUANTILES[i].quantile));
Review Comment:
Do we have issues with percentiles like 99.9th when rounding?
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/lib/MutableInverseQuantiles.java:
##########
@@ -0,0 +1,102 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.metrics2.lib;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.metrics2.MetricsInfo;
+import org.apache.hadoop.metrics2.util.Quantile;
+import org.apache.hadoop.metrics2.util.SampleQuantiles;
+import
org.apache.hadoop.thirdparty.com.google.common.util.concurrent.ThreadFactoryBuilder;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.TimeUnit;
+import static org.apache.hadoop.metrics2.lib.Interns.info;
+
+/**
+ * Watches a stream of long values, maintaining online estimates of specific
+ * quantiles with provably low error bounds. Inverse quantiles are meant for
+ * highly accurate low-percentile (e.g. 1st, 5th) latency metrics.
+ * InverseQuantiles are used for metrics where higher the value better it is.
+ * ( eg: data transfer rate ).
+ * The 1st percentile here corresponds to the 99th inverse percentile metric,
+ * 5th percentile to 95th and so on.
+ */
[email protected]
[email protected]
+public class MutableInverseQuantiles extends MutableQuantiles{
+
+ @VisibleForTesting
+ public static final Quantile[] INVERSE_QUANTILES = { new Quantile(0.50,
0.050),
+ new Quantile(0.25, 0.025), new Quantile(0.10, 0.010),
+ new Quantile(0.05, 0.005), new Quantile(0.01, 0.001) };
+
+ private ScheduledFuture<?> scheduledTask;
+
+ private static final ScheduledExecutorService SCHEDULAR = Executors
+ .newScheduledThreadPool(1, new ThreadFactoryBuilder().setDaemon(true)
+ .setNameFormat("MutableInverseQuantiles-%d").build());
+
+ /**
+ * Instantiates a new {@link MutableInverseQuantiles} for a metric that
rolls itself
+ * over on the specified time interval.
+ *
+ * @param name of the metric
+ * @param description long-form textual description of the metric
+ * @param sampleName type of items in the stream (e.g., "Ops")
+ * @param valueName type of the values
+ * @param interval rollover interval (in seconds) of the estimator
+ */
+ public MutableInverseQuantiles(String name, String description, String
sampleName,
+ String valueName, int interval) {
+ String ucName = StringUtils.capitalize(name);
+ String usName = StringUtils.capitalize(sampleName);
+ String uvName = StringUtils.capitalize(valueName);
+ String desc = StringUtils.uncapitalize(description);
+ String lsName = StringUtils.uncapitalize(sampleName);
+ String lvName = StringUtils.uncapitalize(valueName);
+
+ setNumInfo(info(ucName + "Num" + usName, String.format(
+ "Number of %s for %s with %ds interval", lsName, desc, interval)));
+ // Construct the MetricsInfos for the inverse quantiles, converting to
inverse percentiles
+ setQuantileInfos(INVERSE_QUANTILES.length);
+ String nameTemplate = ucName + "%dthInversePercentile" + uvName;
+ String descTemplate = "%d inverse percentile " + lvName + " with " +
interval
+ + " second interval for " + desc;
+ for (int i = 0; i < INVERSE_QUANTILES.length; i++) {
+ int inversePercentile = (int) (100 * (1 -
INVERSE_QUANTILES[i].quantile));
+ addQuantileInfo(i, info(String.format(nameTemplate, inversePercentile),
+ String.format(descTemplate, inversePercentile)));
+ }
+
+ setEstimator(new SampleQuantiles(INVERSE_QUANTILES));
+ setInterval(interval);
+ scheduledTask = SCHEDULAR.scheduleWithFixedDelay(new RolloverSample(this),
+ interval, interval, TimeUnit.SECONDS);
+ }
+
+ public void stop() {
+ if (scheduledTask != null) {
+ scheduledTask.cancel(false);
+ }
+ scheduledTask = null;
Review Comment:
I would just do the `scheduledTask = null;` inside the if.
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/lib/MutableQuantiles.java:
##########
@@ -162,7 +206,7 @@ public synchronized void setEstimator(QuantileEstimator
quantileEstimator) {
* Runnable used to periodically roll over the internal
* {@link SampleQuantiles} every interval.
*/
- private static class RolloverSample implements Runnable {
+ static class RolloverSample implements Runnable {
Review Comment:
VisibleForTesting?
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/lib/MutableQuantiles.java:
##########
@@ -133,7 +135,49 @@ public synchronized void add(long value) {
estimator.insert(value);
}
- public int getInterval() {
+ /**
+ * Set info about the metrics.
+ *
+ * @param numInfo info about the metrics.
+ */
+ public synchronized void setNumInfo(MetricsInfo numInfo) {
+ this.numInfo = numInfo;
+ }
+
+ /**
+ * Initialize quantileInfos array.
+ *
+ * @param length of the quantileInfos array.
+ */
+ public synchronized void setQuantileInfos(int length) {
+ this.quantileInfos = new MetricsInfo[length];
+ }
+
+ /**
+ * Add entry to quantileInfos array.
+ *
+ * @param i array index.
+ * @param info info to be added to quantileInfos array.
+ */
+ public synchronized void addQuantileInfo(int i, MetricsInfo info) {
+ this.quantileInfos[i] = info;
+ }
+
+ /**
+ * Set the rollover interval (in seconds) of the estimator.
+ *
+ * @param interval (in seconds) of the estimator.
+ */
+ public synchronized void setInterval(int interval) {
+ this.interval = interval;
Review Comment:
```
this.intervalSecs = pIntervalSecs;
```
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/util/TestSampleQuantiles.java:
##########
@@ -118,4 +119,40 @@ public void testQuantileError() throws IOException {
}
}
}
+
+ /**
+ * Correctness test that checks that absolute error of the estimate for
inverse quantiles
+ * is within specified error bounds for some randomly permuted streams of
items.
+ */
+ @Test
+ public void testInverseQuantiles() throws IOException {
+ SampleQuantiles inverseQuantilesEstimator = new
SampleQuantiles(MutableInverseQuantiles.INVERSE_QUANTILES);
+ final int count = 100000;
+ Random r = new Random(0xDEADDEAD);
+ Long[] values = new Long[count];
+ for (int i = 0; i < count; i++) {
+ values[i] = (long) (i + 1);
+ }
+ // Do 10 shuffle/insert/check cycles
Review Comment:
Add break lines to make this easier to read.
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/util/TestSampleQuantiles.java:
##########
@@ -118,4 +119,40 @@ public void testQuantileError() throws IOException {
}
}
}
+
+ /**
+ * Correctness test that checks that absolute error of the estimate for
inverse quantiles
+ * is within specified error bounds for some randomly permuted streams of
items.
+ */
+ @Test
+ public void testInverseQuantiles() throws IOException {
+ SampleQuantiles inverseQuantilesEstimator = new
SampleQuantiles(MutableInverseQuantiles.INVERSE_QUANTILES);
+ final int count = 100000;
+ Random r = new Random(0xDEADDEAD);
+ Long[] values = new Long[count];
+ for (int i = 0; i < count; i++) {
+ values[i] = (long) (i + 1);
+ }
+ // Do 10 shuffle/insert/check cycles
+ for (int i = 0; i < 10; i++) {
+ System.out.println("Starting run " + i);
Review Comment:
Avoid system.out
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/util/TestSampleQuantiles.java:
##########
@@ -118,4 +119,40 @@ public void testQuantileError() throws IOException {
}
}
}
+
+ /**
+ * Correctness test that checks that absolute error of the estimate for
inverse quantiles
+ * is within specified error bounds for some randomly permuted streams of
items.
+ */
+ @Test
+ public void testInverseQuantiles() throws IOException {
+ SampleQuantiles inverseQuantilesEstimator = new
SampleQuantiles(MutableInverseQuantiles.INVERSE_QUANTILES);
+ final int count = 100000;
+ Random r = new Random(0xDEADDEAD);
Review Comment:
Where are you using this?
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/lib/MutableQuantiles.java:
##########
@@ -52,9 +52,9 @@ public class MutableQuantiles extends MutableMetric {
new Quantile(0.75, 0.025), new Quantile(0.90, 0.010),
new Quantile(0.95, 0.005), new Quantile(0.99, 0.001) };
- private final MetricsInfo numInfo;
- private final MetricsInfo[] quantileInfos;
- private final int interval;
+ private MetricsInfo numInfo;
+ private MetricsInfo[] quantileInfos;
+ private int interval;
Review Comment:
As we are at it, let's name this `intervalSecs`
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeMetrics.java:
##########
@@ -18,10 +18,7 @@
package org.apache.hadoop.hdfs.server.datanode;
import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_HEARTBEAT_INTERVAL_KEY;
-import static org.apache.hadoop.test.MetricsAsserts.assertCounter;
-import static org.apache.hadoop.test.MetricsAsserts.assertQuantileGauges;
-import static org.apache.hadoop.test.MetricsAsserts.getLongCounter;
-import static org.apache.hadoop.test.MetricsAsserts.getMetrics;
+import static org.apache.hadoop.test.MetricsAsserts.*;
Review Comment:
Expand
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeMetrics.java:
##########
@@ -413,7 +410,7 @@ public Boolean get() {
final long endWriteValue = getLongCounter("TotalWriteTime", rbNew);
final long endReadValue = getLongCounter("TotalReadTime", rbNew);
assertCounter("ReadTransferRateNumOps", 1L, rbNew);
- assertQuantileGauges("ReadTransferRate" + "60s", rbNew, "Rate");
+ assertInverseQuantileGauges("ReadTransferRate" + "60s", rbNew,
"Rate");
Review Comment:
"ReadTransferRate60s"
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/util/TestSampleQuantiles.java:
##########
@@ -118,4 +119,40 @@ public void testQuantileError() throws IOException {
}
}
}
+
+ /**
+ * Correctness test that checks that absolute error of the estimate for
inverse quantiles
+ * is within specified error bounds for some randomly permuted streams of
items.
+ */
+ @Test
+ public void testInverseQuantiles() throws IOException {
+ SampleQuantiles inverseQuantilesEstimator = new
SampleQuantiles(MutableInverseQuantiles.INVERSE_QUANTILES);
+ final int count = 100000;
Review Comment:
if you make this long, you save a bunch of casts.
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/util/TestSampleQuantiles.java:
##########
@@ -118,4 +119,40 @@ public void testQuantileError() throws IOException {
}
}
}
+
+ /**
+ * Correctness test that checks that absolute error of the estimate for
inverse quantiles
+ * is within specified error bounds for some randomly permuted streams of
items.
+ */
+ @Test
+ public void testInverseQuantiles() throws IOException {
+ SampleQuantiles inverseQuantilesEstimator = new
SampleQuantiles(MutableInverseQuantiles.INVERSE_QUANTILES);
+ final int count = 100000;
+ Random r = new Random(0xDEADDEAD);
+ Long[] values = new Long[count];
+ for (int i = 0; i < count; i++) {
+ values[i] = (long) (i + 1);
Review Comment:
You could just do:
```
for (long i = 0; i < count; i++) {
values[i] = i + 1;
}
```
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/lib/MutableInverseQuantiles.java:
##########
@@ -0,0 +1,102 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.metrics2.lib;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.metrics2.MetricsInfo;
+import org.apache.hadoop.metrics2.util.Quantile;
+import org.apache.hadoop.metrics2.util.SampleQuantiles;
+import
org.apache.hadoop.thirdparty.com.google.common.util.concurrent.ThreadFactoryBuilder;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.TimeUnit;
+import static org.apache.hadoop.metrics2.lib.Interns.info;
+
+/**
+ * Watches a stream of long values, maintaining online estimates of specific
+ * quantiles with provably low error bounds. Inverse quantiles are meant for
+ * highly accurate low-percentile (e.g. 1st, 5th) latency metrics.
+ * InverseQuantiles are used for metrics where higher the value better it is.
+ * ( eg: data transfer rate ).
+ * The 1st percentile here corresponds to the 99th inverse percentile metric,
+ * 5th percentile to 95th and so on.
+ */
[email protected]
[email protected]
+public class MutableInverseQuantiles extends MutableQuantiles{
+
+ @VisibleForTesting
+ public static final Quantile[] INVERSE_QUANTILES = { new Quantile(0.50,
0.050),
+ new Quantile(0.25, 0.025), new Quantile(0.10, 0.010),
+ new Quantile(0.05, 0.005), new Quantile(0.01, 0.001) };
+
+ private ScheduledFuture<?> scheduledTask;
+
+ private static final ScheduledExecutorService SCHEDULAR = Executors
+ .newScheduledThreadPool(1, new ThreadFactoryBuilder().setDaemon(true)
+ .setNameFormat("MutableInverseQuantiles-%d").build());
+
+ /**
+ * Instantiates a new {@link MutableInverseQuantiles} for a metric that
rolls itself
+ * over on the specified time interval.
+ *
+ * @param name of the metric
+ * @param description long-form textual description of the metric
+ * @param sampleName type of items in the stream (e.g., "Ops")
+ * @param valueName type of the values
+ * @param interval rollover interval (in seconds) of the estimator
Review Comment:
We could even go further with TimeUnit but that might be overkilled.
> Update ReadTransferRate to ReadLatencyPerGB for effective percentile metrics
> ----------------------------------------------------------------------------
>
> Key: HDFS-16949
> URL: https://issues.apache.org/jira/browse/HDFS-16949
> Project: Hadoop HDFS
> Issue Type: Bug
> Components: datanode
> Reporter: Ravindra Dingankar
> Assignee: Ravindra Dingankar
> Priority: Minor
> Labels: pull-request-available
> Fix For: 3.3.0, 3.4.0
>
>
> HDFS-16917 added ReadTransferRate quantiles to calculate the rate which data
> is read per unit of time.
> With percentiles the values are sorted in ascending order and hence for the
> transfer rate p90 gives us the value where 90 percent rates are lower
> (worse), p99 gives us the value where 99 percent values are lower (worse).
> Note that value(p90) < p(99) thus p99 is a better transfer rate as compared
> to p90.
> However as the percentile increases the value should become worse in order to
> know how good our system is.
> Hence instead of calculating the data read transfer rate, we should calculate
> it's inverse. We will instead calculate the time taken for a GB of data to be
> read. ( seconds / GB )
> After this the p90 value will give us 90 percentage of total values where the
> time taken is less than value(p90), similarly for p99 and others.
> Also p(90) < p(99) and here p(99) will become a worse value (taking more time
> each byte) as compared to p(90)
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
