[jira] [Commented] (HADOOP-11104) org.apache.hadoop.metrics2.lib.MetricsRegistry needs numerical parameter checking

["Akira AJISAKA (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/HADOOP-11104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14547797#comment-14547797 ] 

Akira AJISAKA commented on HADOOP-11104:
----------------------------------------

Thanks [~rchiang] for the report and the patch.
I agree with you that we should check if the interval for Quantiles is positive and throw a clear error message. For Counter, Gauge and Stats, I'm thinking we don't need to check the initial value because they works well even if the initial value is negative.

{code}
+    if (interval < 0) {
+      throw new MetricsException("Illegal interval: " + interval);
+    }
{code}
* It would be better to add to the message that the interval should be positive, and document to the javadoc that the method will throw MetricsException if the interval is not more than 0.
* {{interval < 0}} should be {{interval <= 0}}.

> org.apache.hadoop.metrics2.lib.MetricsRegistry needs numerical parameter checking
> ---------------------------------------------------------------------------------
>
>                 Key: HADOOP-11104
>                 URL: https://issues.apache.org/jira/browse/HADOOP-11104
>             Project: Hadoop Common
>          Issue Type: Bug
>            Reporter: Ray Chiang
>            Assignee: Ray Chiang
>            Priority: Minor
>              Labels: BB2015-05-RFC, newbie, supportability
>         Attachments: HADOOP-11104.001.patch, HADOOP-11104.002.patch
>
>
> Passing a negative value to the interval field of MetricsRegistry#newQuantiles should throw a MetricsException with a clear error message.  The current stack trace looks something like:
> java.lang.IllegalArgumentException: null
>         at java.util.concurrent.ScheduledThreadPoolExecutor.scheduleAtFixedRate(ScheduledThreadPoolExecutor.java:420)
>         at org.apache.hadoop.metrics2.lib.MutableQuantiles.<init>(MutableQuantiles.java:107)
>         at org.apache.hadoop.metrics2.lib.MetricsRegistry.newQuantiles(MetricsRegistry.java:200)
> Along similar lines, should the other methods like MetricsRegistry#newCounter() also have parameter checking for negative int/long values?



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)