[HBase-dev] [jira] Created: (HBASE-2068) MetricsRate is missing "registry" parameter

[ https://issues.apache.org/jira/browse/HBASE-2068?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Lars George updated HBASE-2068:
-------------------------------

Description:
I am trying to get the graphing going using Edward Capriolo's great JMX to Cacti [guide|http://www.jointhegrid.com/hadoop/]. I checked and I am missing the request rate in the JMX MBean:

{noformat}
# sh 0_20/regionserver/RegionServerStatistics.sh service:jmx:rmi:///jndi/rmi://foobar:10102/jmxrmi controlRole mypass hadoop:name=RegionServerStatistics,service=RegionServer
storefiles:493 blockCacheFree:139771296 storefileIndexSizeMB:102 memstoreSizeMB:0 stores:658 blockCacheCount:8400
regions:83 blockCacheHitRatio:0 blockCacheSize:717478944 atomicIncrementTimeNumOps:0 atomicIncrementTimeAvgTime:0
atomicIncrementTimeMinTime:-1 atomicIncrementTimeMaxTime:0
{noformat}

I checked the code and the difference between requests and the other attributes is that MetricsRate does not register itself in the MetricsRegistry used by the dynamic MBean like for example the MetricsLongValue does.



was:
I am trying to get the graphing going using Edward Capriolo's great JMX to Cacti [guide|http://www.jointhegrid.com/hadoop/]. I checked and I am missing the request rate in the JMX MBean:

{code}
# sh 0_20/regionserver/RegionServerStatistics.sh service:jmx:rmi:///jndi/rmi://foobar:10102/jmxrmi controlRole mypass hadoop:name=RegionServerStatistics,service=RegionServer
storefiles:493 blockCacheFree:139771296 storefileIndexSizeMB:102 memstoreSizeMB:0 stores:658 blockCacheCount:8400 regions:83 blockCacheHitRatio:0 blockCacheSize:717478944 atomicIncrementTimeNumOps:0 atomicIncrementTimeAvgTime:0 atomicIncrementTimeMinTime:-1 atomicIncrementTimeMaxTime:0
{code}

I checked the code and the difference between requests and the other attributes is that MetricsRate does not register itself in the MetricsRegistry used by the dynamic MBean like for example the MetricsLongValue does.

[ https://issues.apache.org/jira/browse/HBASE-2068?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12793630#action_12793630 ]

Lars George commented on HBASE-2068:
------------------------------------

For this to work MetricsRate has to implement MetricsBase, just like MetricsLongValue etc. does. During creation of the MBean the MetricsDynamicMBeanBase iterates over the MetricsRegistry and declares the attributes dynamically. That is where the "request" falls off the plate. This only concerns JMX btw. as using the file or Ganglia based context employs the MetricsRecord directly.

[ https://issues.apache.org/jira/browse/HBASE-2068?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12793774#action_12793774 ]

stack commented on HBASE-2068:
------------------------------

@Gary Any comment on above? Is it an oversight in the jmx hookup work?

[ https://issues.apache.org/jira/browse/HBASE-2068?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12795032#action_12795032 ]

Lars George commented on HBASE-2068:
------------------------------------

Another inconsistency is that MasterMetrics uses a MetricsIntValue

{code}
/*
* Count of requests to the cluster since last call to metrics update
*/
private final MetricsIntValue cluster_requests =
new MetricsIntValue("cluster_requests", registry);
{code}

and the RegionsServerMetrics uses a MetricsRate class

{code}
/*
* Count of requests to the regionservers since last call to metrics update
*/
private final MetricsRate requests = new MetricsRate("requests");
{code}

I suggest after fixing MetricsRate to replace the MasterMetrics one with it as well.

I am also wondering if it would have been better to call it MetricsIntRate to indicate the size of the internal counter.

[ https://issues.apache.org/jira/browse/HBASE-2068?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Lars George updated HBASE-2068:
-------------------------------

Attachment: HBASE-2068-0.20.patch
HBASE-2068.patch

Patch adds MetricsRate support for adding itself to the MetricsRegistry and also exchanged the MetricsIntValue in MasterMetrics for the MetricsRate class.

[ https://issues.apache.org/jira/browse/HBASE-2068?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Lars George updated HBASE-2068:
-------------------------------

Status: Patch Available (was: Open)

[ https://issues.apache.org/jira/browse/HBASE-2068?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jean-Daniel Cryans updated HBASE-2068:
--------------------------------------

Resolution: Fixed
Hadoop Flags: [Reviewed]
Status: Resolved (was: Patch Available)

Looks good, committed to trunk and branch.

[ https://issues.apache.org/jira/browse/HBASE-2068?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12795446#action_12795446 ]

Gary Helmling commented on HBASE-2068:
--------------------------------------

It looks like MetricsRate is not going to work with the Hadoop dynamic MBean support for JMX

Trying out this patch, I get this message in logs:
2009-12-30 14:43:45,010 ERROR org.apache.hadoop.metrics.MetricsUtil: unknown metrics type: org.apache.hadoop.hbase.metrics.MetricsRate

and the requests attributes do not show up in JConsole for the RS or master stats. Traced the log message back to org.apache.hadoop.metrics.util.MetricsDynamicMBeanBase:

{code}
for (MetricsBase o : metricsRegistry.getMetricsList()) {

if (MetricsTimeVaryingRate.class.isInstance(o)) {
...
} else if ( MetricsIntValue.class.isInstance(o) || MetricsTimeVaryingInt.class.isInstance(o) ) {
...
} else if ( MetricsLongValue.class.isInstance(o) || MetricsTimeVaryingLong.class.isInstance(o) ) {
...
} else {
MetricsUtil.LOG.error("unknown metrics type: " + o.getClass().getName());
}
{code}

So for this to work with JMX, we'll need to extend MetricsDynamicMBeanBase with another class that understands MetricsRate. I'll pull together a patch for that.

[ https://issues.apache.org/jira/browse/HBASE-2068?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12795457#action_12795457 ]

Lars George commented on HBASE-2068:
------------------------------------

Thanks Gary! Another option would be to simply use the same MetricsIntValue plus reset to zero as the MasterMetrics employed. That does work with what Hadoop has now.

[ https://issues.apache.org/jira/browse/HBASE-2068?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Lars George reopened HBASE-2068:
--------------------------------


Reopened to address issue found by Gary. As this can be fixed quite quickly one way or the other I reopened this issue.

[ https://issues.apache.org/jira/browse/HBASE-2068?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Gary Helmling updated HBASE-2068:
---------------------------------

Release Note: Converts MasterStatistics "cluster_requests" attribute to a MetricsRate value (computed as requests / sec). Also adds a "requests" attribute to RegionServerStatistics MBean, also as a MetricsRate value.
Status: Patch Available (was: Reopened)

This patch adds JMX support for MetricsRate instances, by adding a MetricsMBeanBase class which understands HBase metrics classes.

[ https://issues.apache.org/jira/browse/HBASE-2068?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Gary Helmling updated HBASE-2068:
---------------------------------

Attachment: HBASE-2068-2_0.20.patch

Patch adding MetricsMBeanBase to support exporting MetricsRate under JMX.

[ https://issues.apache.org/jira/browse/HBASE-2068?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Gary Helmling updated HBASE-2068:
---------------------------------

Attachment: HBASE-2068-2_0.20.patch

Previous patch was missing the new MetricsMBeanBase class and unit test

[ https://issues.apache.org/jira/browse/HBASE-2068?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Gary Helmling updated HBASE-2068:
---------------------------------

Attachment: (was: HBASE-2068-2_0.20.patch)

[ https://issues.apache.org/jira/browse/HBASE-2068?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Gary Helmling updated HBASE-2068:
---------------------------------

Attachment: HBASE-2068-2.patch

Patch against trunk adding MetricsMBeanBase and unit test

[ https://issues.apache.org/jira/browse/HBASE-2068?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12795926#action_12795926 ]

Lars George commented on HBASE-2068:
------------------------------------

Fantastic Gary, I give that a shot asap and report back.

[ https://issues.apache.org/jira/browse/HBASE-2068?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12796319#action_12796319 ]

Lars George commented on HBASE-2068:
------------------------------------

+1

Works great! There are few now obsolete imports, like

{code}
-import org.apache.hadoop.metrics.util.MetricsDynamicMBeanBase;
{code}

in the changed Statistics classes. Could remove on commit.

Otherwise please commit.

[ https://issues.apache.org/jira/browse/HBASE-2068?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

stack updated HBASE-2068:
-------------------------

Resolution: Fixed
Status: Resolved (was: Patch Available)

Committed. Thanks for the patches lads. I removed the unused import. Added a couple of licenses too. Excellent.