[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13421534#comment-13421534 ]
Zhihong Ted Yu commented on HBASE-6411: --------------------------------------- @Alex: I read your arguments @ 24/Jul/12 15:29 Since DynamicMetricsRegistry (in hbase-hadoop1-compat) is to be replaced by org.apache.hadoop.metrics2.lib.MetricsRegistry in the future, it should be annotated with @InterfaceStability.Evolving The same class in hbase-hadoop2-compat is correctly annotated. {code} + private final LinkedHashMap<String, MetricsTag> tagsMap = + new LinkedHashMap<String, MetricsTag>(); {code} Do we need to consider concurrency for the above map ? For incr(): {code} + } + else if (m instanceof MetricMutableCounter<?>) { {code} Style comment: the keyword else should be on the same line as the preceding } {code} + throw new MetricsException("Unsupported incr() for metric "+ name); {code} Including the String form of MetricMutable (m) would reveal more. {code} + else if (factory != null) { + metricsMap.put(name, factory.newMetric(name)); + incr(name, null); + } {code} The last line above results in a recursive call. I would suggest storing the new metric in MetricMutable m to remove the recursive call. Similar comment for the case of factory != null in decr(). {code} + else { + throw new MetricsException("Metric "+ name +" doesn't exist"); + } {code} In the above case name would be null. Exception message should be clearer. Please put future patches on review board. Thanks > Move Master Metrics to metrics 2 > -------------------------------- > > Key: HBASE-6411 > URL: https://issues.apache.org/jira/browse/HBASE-6411 > Project: HBase > Issue Type: Sub-task > Reporter: Elliott Clark > Assignee: Elliott Clark > Attachments: HBASE-6411-0.patch, HBASE-6411-1.patch, > HBASE-6411-2.patch, HBASE-6411_concept.patch > > > Move Master Metrics to metrics 2 -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira