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

Alex Baranau commented on HBASE-6411:
-------------------------------------

Thanks for the comments guys!

@Ted:

Re concurrency, see below. Other nits: this is the code copied from hadoop 
metrics2. The only lines added are removeMetric() method and after it (3 
methods total). Other code is unchanged (as we probably don't want to change 
it, see below also).

@Elliott:

RE having one for dynamic and static: understood. Makes sense.

By refactoring I didn't mean to create two registries, but only extract 
MetricsSource initialization. Ok, let's leave it as is.

Re exceptions: OK, agreed.

bq. increment shouldn't be on the registry. The registry is there to create and 
hold metrics.

I agree, I haven't added it. It is from hadoop's code, as Ted Yu mentioned. 
I.e. I have same thoughts about doing exactly this:

bq. We want the registry to be functionally close to what's in 
org.apache.hadoop.metrics2 so that if they ever add a remove function then we 
can use that registry wholesale.

On:

bq. Since you've moved the metrics creation out of the source, it's possible 
that more then one thread could be using it and everything will need to be done 
using concurrent hash maps.

There's no access to the registry from outside the BaseMetricsSourceImpl class, 
so I don't see how extracting changes things. In the MetricsRegistry impl that 
I added you can see that concurrent maps are used to hold metrics. Didn't think 
that tags will be created a lot, but rather in initialization methods once, so 
haven't changed to using concurrent map for it.
At the same time I see that access to maps in hadoop code (and hence the one I 
copied) is synchronized (didn't notice that, sorry). So it looks like we either 
need to make added methods to be synchronized, or use concurrent collections. 
If we do want to change _minimally_ existing hadoop code (for the reasons we 
said above) it makes sense to just make these methods  use those sync internal 
methods when adding metrics and such.

Thanx again for the comments!
                
> 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

        

Reply via email to