From 2c6c9e315ca19695277c5d9c1146346f0aca5eb3 Mon Sep 17 00:00:00 2001
From: Akira Ajisaka <aajisaka@apache.org>
Date: Fri, 9 Oct 2015 11:10:29 +0900
Subject: [PATCH] HADOOP-11104. org.apache.hadoop.metrics2.lib.MetricsRegistry
 needs numerical parameter checking. Contributed by Ray Chiang.

(cherry picked from commit e1bf8b3df6f019b92a3dad37c977f40397324e75)
---
 hadoop-common-project/hadoop-common/CHANGES.txt  |  3 +++
 .../hadoop/metrics2/lib/MetricsRegistry.java     |  9 +++++++--
 .../hadoop/metrics2/lib/TestMetricsRegistry.java | 16 ++++++++++++++++
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt
index b41f690ca33..1dc42e90192 100644
--- a/hadoop-common-project/hadoop-common/CHANGES.txt
+++ b/hadoop-common-project/hadoop-common/CHANGES.txt
@@ -291,6 +291,9 @@ Release 2.8.0 - UNRELEASED
     HADOOP-12350. WASB Logging: Improve WASB Logging around deletes, reads and
     writes (Dushyanth via cnauroth)
 
+    HADOOP-11104. org.apache.hadoop.metrics2.lib.MetricsRegistry needs numerical
+    parameter checking. (Ray Chiang via aajisaka)
+
   OPTIMIZATIONS
 
     HADOOP-11785. Reduce the number of listStatus operation in distcp
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/lib/MetricsRegistry.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/lib/MetricsRegistry.java
index 4b561f2fb56..6e7e5ab7cc1 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/lib/MetricsRegistry.java
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/lib/MetricsRegistry.java
@@ -188,16 +188,21 @@ public class MetricsRegistry {
    * @param valueName of the metric (e.g., "Time" or "Latency")
    * @param interval rollover interval of estimator in seconds
    * @return a new quantile estimator object
+   * @throws MetricsException if interval is not a positive integer
    */
   public synchronized MutableQuantiles newQuantiles(String name, String desc,
       String sampleName, String valueName, int interval) {
     checkMetricName(name);
-    MutableQuantiles ret = 
+    if (interval <= 0) {
+      throw new MetricsException("Interval should be positive.  Value passed" +
+          " is: " + interval);
+    }
+    MutableQuantiles ret =
         new MutableQuantiles(name, desc, sampleName, valueName, interval);
     metricsMap.put(name, ret);
     return ret;
   }
-  
+
   /**
    * Create a mutable metric with stats
    * @param name  of the metric
diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/lib/TestMetricsRegistry.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/lib/TestMetricsRegistry.java
index af1ff96b9a5..d91692807eb 100644
--- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/lib/TestMetricsRegistry.java
+++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/lib/TestMetricsRegistry.java
@@ -122,6 +122,22 @@ public class TestMetricsRegistry {
     });
   }
 
+  /**
+   * Test adding illegal parameters
+   */
+  @Test
+  public void testAddIllegalParameters() {
+    final MetricsRegistry r = new MetricsRegistry("IllegalParamTest");
+
+    expectMetricsException("Interval should be positive.  Value passed is: -20",
+        new Runnable() {
+      @Override
+      public void run() {
+          r.newQuantiles("q1", "New Quantile 1", "qq1", "qv1", (int)-20);
+      }
+    });
+  }
+
   @Ignore
   private void expectMetricsException(String prefix, Runnable fun) {
     try {