Skip to content

Commit 19432f5

Browse files
author
Ravindra Dingankar
committed
review comments
1 parent 0fceb1b commit 19432f5

File tree

4 files changed

+52
-38
lines changed

4 files changed

+52
-38
lines changed

‎hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/lib/MutableInverseQuantiles.java‎

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import org.apache.hadoop.classification.InterfaceAudience;
2222
import org.apache.hadoop.classification.InterfaceStability;
2323
import org.apache.hadoop.classification.VisibleForTesting;
24-
import org.apache.hadoop.metrics2.MetricsInfo;
2524
import org.apache.hadoop.metrics2.util.Quantile;
2625
import org.apache.hadoop.metrics2.util.SampleQuantiles;
2726
import org.apache.hadoop.thirdparty.com.google.common.util.concurrent.ThreadFactoryBuilder;
@@ -44,10 +43,16 @@
4443
@InterfaceStability.Evolving
4544
public class MutableInverseQuantiles extends MutableQuantiles{
4645

46+
static class InversePercentile extends Quantile {
47+
InversePercentile(double inversePercentile) {
48+
super(inversePercentile/100, inversePercentile/1000);
49+
}
50+
}
51+
4752
@VisibleForTesting
48-
public static final Quantile[] INVERSE_QUANTILES = { new Quantile(0.50, 0.050),
49-
new Quantile(0.25, 0.025), new Quantile(0.10, 0.010),
50-
new Quantile(0.05, 0.005), new Quantile(0.01, 0.001) };
53+
public static final Quantile[] INVERSE_QUANTILES = { new InversePercentile(50),
54+
new InversePercentile(25), new InversePercentile(10),
55+
new InversePercentile(5), new InversePercentile(1) };
5156

5257
private ScheduledFuture<?> scheduledTask;
5358

@@ -59,14 +64,14 @@ public class MutableInverseQuantiles extends MutableQuantiles{
5964
* Instantiates a new {@link MutableInverseQuantiles} for a metric that rolls itself
6065
* over on the specified time interval.
6166
*
62-
* @param name of the metric
63-
* @param description long-form textual description of the metric
64-
* @param sampleName type of items in the stream (e.g., "Ops")
65-
* @param valueName type of the values
66-
* @param interval rollover interval (in seconds) of the estimator
67+
* @param name of the metric
68+
* @param description long-form textual description of the metric
69+
* @param sampleName type of items in the stream (e.g., "Ops")
70+
* @param valueName type of the values
71+
* @param intervalSecs rollover interval (in seconds) of the estimator
6772
*/
6873
public MutableInverseQuantiles(String name, String description, String sampleName,
69-
String valueName, int interval) {
74+
String valueName, int intervalSecs) {
7075
String ucName = StringUtils.capitalize(name);
7176
String usName = StringUtils.capitalize(sampleName);
7277
String uvName = StringUtils.capitalize(valueName);
@@ -75,11 +80,11 @@ public MutableInverseQuantiles(String name, String description, String sampleNam
7580
String lvName = StringUtils.uncapitalize(valueName);
7681

7782
setNumInfo(info(ucName + "Num" + usName, String.format(
78-
"Number of %s for %s with %ds interval", lsName, desc, interval)));
83+
"Number of %s for %s with %ds interval", lsName, desc, intervalSecs)));
7984
// Construct the MetricsInfos for the inverse quantiles, converting to inverse percentiles
8085
setQuantileInfos(INVERSE_QUANTILES.length);
8186
String nameTemplate = ucName + "%dthInversePercentile" + uvName;
82-
String descTemplate = "%d inverse percentile " + lvName + " with " + interval
87+
String descTemplate = "%d inverse percentile " + lvName + " with " + intervalSecs
8388
+ " second interval for " + desc;
8489
for (int i = 0; i < INVERSE_QUANTILES.length; i++) {
8590
int inversePercentile = (int) (100 * (1 - INVERSE_QUANTILES[i].quantile));
@@ -88,15 +93,15 @@ public MutableInverseQuantiles(String name, String description, String sampleNam
8893
}
8994

9095
setEstimator(new SampleQuantiles(INVERSE_QUANTILES));
91-
setInterval(interval);
96+
setInterval(intervalSecs);
9297
scheduledTask = SCHEDULAR.scheduleWithFixedDelay(new RolloverSample(this),
93-
interval, interval, TimeUnit.SECONDS);
98+
intervalSecs, intervalSecs, TimeUnit.SECONDS);
9499
}
95100

96101
public void stop() {
97102
if (scheduledTask != null) {
98103
scheduledTask.cancel(false);
104+
scheduledTask = null;
99105
}
100-
scheduledTask = null;
101106
}
102107
}

‎hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/lib/MutableQuantiles.java‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public class MutableQuantiles extends MutableMetric {
5454

5555
private MetricsInfo numInfo;
5656
private MetricsInfo[] quantileInfos;
57-
private int interval;
57+
private int intervalSecs;
5858

5959
private QuantileEstimator estimator;
6060
private long previousCount = 0;
@@ -138,10 +138,10 @@ public synchronized void add(long value) {
138138
/**
139139
* Set info about the metrics.
140140
*
141-
* @param numInfo info about the metrics.
141+
* @param pNumInfo info about the metrics.
142142
*/
143-
public synchronized void setNumInfo(MetricsInfo numInfo) {
144-
this.numInfo = numInfo;
143+
public synchronized void setNumInfo(MetricsInfo pNumInfo) {
144+
this.numInfo = pNumInfo;
145145
}
146146

147147
/**
@@ -166,19 +166,19 @@ public synchronized void addQuantileInfo(int i, MetricsInfo info) {
166166
/**
167167
* Set the rollover interval (in seconds) of the estimator.
168168
*
169-
* @param interval (in seconds) of the estimator.
169+
* @param pIntervalSecs (in seconds) of the estimator.
170170
*/
171-
public synchronized void setInterval(int interval) {
172-
this.interval = interval;
171+
public synchronized void setInterval(int pIntervalSecs) {
172+
this.intervalSecs = pIntervalSecs;
173173
}
174174

175175
/**
176176
* Get the rollover interval (in seconds) of the estimator.
177177
*
178-
* @return interval (in seconds) of the estimator.
178+
* @return intervalSecs (in seconds) of the estimator.
179179
*/
180180
public synchronized int getInterval() {
181-
return interval;
181+
return intervalSecs;
182182
}
183183

184184
public void stop() {

‎hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/util/TestSampleQuantiles.java‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -93,27 +93,30 @@ public void testClear() throws IOException {
9393
public void testQuantileError() throws IOException {
9494
final int count = 100000;
9595
Random r = new Random(0xDEADDEAD);
96-
Long[] values = new Long[count];
96+
int[] values = new int[count];
9797
for (int i = 0; i < count; i++) {
98-
values[i] = (long) (i + 1);
98+
values[i] = i + 1;
9999
}
100+
100101
// Do 10 shuffle/insert/check cycles
101102
for (int i = 0; i < 10; i++) {
102-
System.out.println("Starting run " + i);
103+
104+
// Shuffle
103105
Collections.shuffle(Arrays.asList(values), r);
104106
estimator.clear();
107+
108+
// Insert
105109
for (int j = 0; j < count; j++) {
106110
estimator.insert(values[j]);
107111
}
108112
Map<Quantile, Long> snapshot;
109113
snapshot = estimator.snapshot();
114+
115+
// Check
110116
for (Quantile q : quantiles) {
111117
long actual = (long) (q.quantile * count);
112118
long error = (long) (q.error * count);
113119
long estimate = snapshot.get(q);
114-
System.out
115-
.println(String.format("Expected %d with error %d, estimated %d",
116-
actual, error, estimate));
117120
assertThat(estimate <= actual + error).isTrue();
118121
assertThat(estimate >= actual - error).isTrue();
119122
}
@@ -129,27 +132,29 @@ public void testInverseQuantiles() throws IOException {
129132
SampleQuantiles inverseQuantilesEstimator = new SampleQuantiles(MutableInverseQuantiles.INVERSE_QUANTILES);
130133
final int count = 100000;
131134
Random r = new Random(0xDEADDEAD);
132-
Long[] values = new Long[count];
135+
int[] values = new int[count];
133136
for (int i = 0; i < count; i++) {
134-
values[i] = (long) (i + 1);
137+
values[i] = i + 1;
135138
}
139+
136140
// Do 10 shuffle/insert/check cycles
137141
for (int i = 0; i < 10; i++) {
138-
System.out.println("Starting run " + i);
142+
// Shuffle
139143
Collections.shuffle(Arrays.asList(values), r);
140144
inverseQuantilesEstimator.clear();
145+
146+
// Insert
141147
for (int j = 0; j < count; j++) {
142148
inverseQuantilesEstimator.insert(values[j]);
143149
}
144150
Map<Quantile, Long> snapshot;
145151
snapshot = inverseQuantilesEstimator.snapshot();
152+
153+
// Check
146154
for (Quantile q : MutableInverseQuantiles.INVERSE_QUANTILES) {
147155
long actual = (long) (q.quantile * count);
148156
long error = (long) (q.error * count);
149157
long estimate = snapshot.get(q);
150-
System.out.println(String.format("For inverse quantile %f " +
151-
"Expected %d with error %d, estimated %d",
152-
(1 - q.quantile), actual, error, estimate));
153158
assertThat(estimate <= actual + error).isTrue();
154159
assertThat(estimate >= actual - error).isTrue();
155160
}

‎hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeMetrics.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,11 @@
1818
package org.apache.hadoop.hdfs.server.datanode;
1919

2020
import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_HEARTBEAT_INTERVAL_KEY;
21-
import static org.apache.hadoop.test.MetricsAsserts.*;
21+
import static org.apache.hadoop.test.MetricsAsserts.assertCounter;
22+
import static org.apache.hadoop.test.MetricsAsserts.assertInverseQuantileGauges;
23+
import static org.apache.hadoop.test.MetricsAsserts.assertQuantileGauges;
24+
import static org.apache.hadoop.test.MetricsAsserts.getLongCounter;
25+
import static org.apache.hadoop.test.MetricsAsserts.getMetrics;
2226
import static org.junit.Assert.*;
2327

2428
import java.io.Closeable;
@@ -410,7 +414,7 @@ public Boolean get() {
410414
final long endWriteValue = getLongCounter("TotalWriteTime", rbNew);
411415
final long endReadValue = getLongCounter("TotalReadTime", rbNew);
412416
assertCounter("ReadTransferRateNumOps", 1L, rbNew);
413-
assertInverseQuantileGauges("ReadTransferRate" + "60s", rbNew, "Rate");
417+
assertInverseQuantileGauges("ReadTransferRate60s", rbNew, "Rate");
414418
return endWriteValue > startWriteValue
415419
&& endReadValue > startReadValue;
416420
}

0 commit comments

Comments
 (0)