hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "stack (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-4050) Update HBase metrics framework to metrics2 framework
Date Fri, 13 Jul 2012 07:15:36 GMT

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

stack commented on HBASE-4050:
------------------------------

I suppose you don't need to set test size annotation on below because annotations are not
a dependency when this is built:

{code}
+public class ReplicationMetricsSourceFactoryTest {
{code}

Does BaseMetricsSource not implement MetricsSource?

{code}
+public class BaseMetricsSourceImpl implements BaseMetricsSource, MetricsSource {
{code}

These need to be this accessible:

{code}
+  public ConcurrentMap<String, MetricMutableGaugeLong>
+      gauges = new ConcurrentHashMap<String, MetricMutableGaugeLong>();
+  public ConcurrentMap<String, MetricMutableCounterLong> counters =
+      new ConcurrentHashMap<String, MetricMutableCounterLong>();
+
+  protected String metricsContext;
+  protected String metricsName;
+  protected String metricsDescription;
{code}

(I see above twice)

The stuff below where we have a static boolean and in constructor we test something already
created could be a PITA in minihbase setups?  Does it have to be static?  Aren't we slinging
singletons here anyways?  (The singletons are ok in minihbasecontext too?):

{code}
+    if (!hasInited) {
+      //Not too worried about mutli-threaded here as all it does is spam the logs.
+      hasInited = true;
+      DefaultMetricsSystem.initialize(HBASE_METRICS_SYSTEM_NAME);
+    }
{code}

'hasInited' is name of a method that tests 'inited' variable... suggest changing its name.

What about that jmx mess registering metrics in tests?  The exception saying metrics already
registered because we have more than one daemon in the one jvm.  We still have that issue
here?

You wanted to complete this: "+/** BaseClass for */"

Another class has no class comments though has the comment delimiters.

Do we have to have metrics2 package?  Can this new stuff be in the metrics package?

I thought I saw a patch where you'd renamed the properties file to what LarsG suggested?

You seem to have made it so we do not need to have a metrics2 in hbase... thats great... but
in the properties file I see:

{code}
+# See package.html for org.apache.hadoop.metrics2 for details
+
+*.sink.file.class=org.apache.hadoop.metrics2.sink.FileSink
{code}

Is that just old stuff?

Good stuff Elliott.  I'd be up for committing this and then doing other stuff in other issues.


                
> Update HBase metrics framework to metrics2 framework
> ----------------------------------------------------
>
>                 Key: HBASE-4050
>                 URL: https://issues.apache.org/jira/browse/HBASE-4050
>             Project: HBase
>          Issue Type: New Feature
>          Components: metrics
>    Affects Versions: 0.90.4
>         Environment: Java 6
>            Reporter: Eric Yang
>            Assignee: Alex Baranau
>            Priority: Critical
>             Fix For: 0.96.0
>
>         Attachments: 4050-metrics-v2.patch, 4050-metrics-v3.patch, HBASE-4050-0.patch,
HBASE-4050-1.patch, HBASE-4050-2.patch, HBASE-4050-3.patch, HBASE-4050-5.patch, HBASE-4050-6.patch,
HBASE-4050-7.patch, HBASE-4050.patch
>
>
> Metrics Framework has been marked deprecated in Hadoop 0.20.203+ and 0.22+, and it might
get removed in future Hadoop release.  Hence, HBase needs to revise the dependency of MetricsContext
to use Metrics2 framework.

--
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

        

Mime
View raw message