You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "Elliott Clark (JIRA)" <ji...@apache.org> on 2012/07/17 23:59:35 UTC
[jira] [Created] (HBASE-6411) Move Master Metrics to metrics 2
Elliott Clark created HBASE-6411:
------------------------------------
Summary: 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: Alex Baranau
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
[jira] [Updated] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Alex Baranau (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Alex Baranau updated HBASE-6411:
--------------------------------
Attachment: HBASE-6411-5.patch
Attached patch without integration with unit-testing tools of hadoop's metrics2
> 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: Alex Baranau
> Attachments: HBASE-6411-0.patch, HBASE-6411-1.patch, HBASE-6411-2.patch, HBASE-6411-3.patch, HBASE-6411-4.patch, HBASE-6411-4_2.patch, HBASE-6411-5.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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Zhihong Ted Yu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13426742#comment-13426742 ]
Zhihong Ted Yu commented on HBASE-6411:
---------------------------------------
For findbugs:
{code}
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-remote-resources-plugin:1.1:process (default) on project hbase-hadoop1-compat: Failed to resolve dependencies for one or more projects in the reactor. Reason: Missing:
[ERROR] ----------
[ERROR] 1) org.apache.hbase:hbase-hadoop-compat:test-jar:tests:0.95-SNAPSHOT
[ERROR]
[ERROR] Try downloading the file manually from the project website.
[ERROR]
[ERROR] Then, install it using the command:
[ERROR] mvn install:install-file -DgroupId=org.apache.hbase -DartifactId=hbase-hadoop-compat -Dversion=0.95-SNAPSHOT -Dclassifier=tests -Dpackaging=test-jar -Dfile=/path/to/file
[ERROR]
[ERROR] Alternatively, if you host your own repository you can deploy the file there:
[ERROR] mvn deploy:deploy-file -DgroupId=org.apache.hbase -DartifactId=hbase-hadoop-compat -Dversion=0.95-SNAPSHOT -Dclassifier=tests -Dpackaging=test-jar -Dfile=/path/to/file -Durl=[url] -DrepositoryId=[id]
[ERROR]
[ERROR] Path to dependency:
[ERROR] 1) org.apache.hbase:hbase-hadoop1-compat:jar:0.95-SNAPSHOT
[ERROR] 2) org.apache.hbase:hbase-hadoop-compat:test-jar:tests:0.95-SNAPSHOT
[ERROR]
[ERROR] ----------
[ERROR] 1 required artifact is missing.
[ERROR]
{code}
> 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: Alex Baranau
> Attachments: HBASE-6411-0.patch, HBASE-6411-1.patch, HBASE-6411-2.patch, HBASE-6411-3.patch, HBASE-6411-4.patch, HBASE-6411-4_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
[jira] [Updated] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Alex Baranau (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Alex Baranau updated HBASE-6411:
--------------------------------
Attachment: HBASE-6411-3.patch
Tried to fix things we mentioned about. Created review request: https://reviews.apache.org/r/6161
Removed those incr() methods we were willing to (it turned out that they were only in hadoop 1.0 metrics2 lib, and were removed in hadoop 2.0, so we are ok removing it and no need to ask hadoop guys about it).
Switched to using concurrent collections inside metric registries. It turned out that "synchronize-guarded" methods where only in hadoop 2.0, not in hadoop 1.0. In both cases replaced them with working with concurrent collections.
Tried to remove throwing MetricsExceptions, but there are still cases where I throw them. Not sure what is the best way to avoid this in those particular cases. Advice is much appreciated.
Two Qs remained:
1. what about old MasterMetrics? Are we sure we want to replace it? I though we want to leave it for some time
2. my Q from the comment above related to metricsSources interfaces - not critical, can be addressed later, I guess.
> 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-3.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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Alex Baranau (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13421478#comment-13421478 ]
Alex Baranau commented on HBASE-6411:
-------------------------------------
bq. Normally such method is annotated with @Before
Right. I actually got this idea not to place it in @Before from one of the existing tests. The benefit is that you can place in the test class other tests that does not require cluster startup. Otherwise (if cluster startup is in @Before) it will be always started. Does it make sense to you?
bq. If TestMasterMetrics is placed in master.metrics package, the following method can be package-private:
Right. Again, I think by example, placed it in master package. It also accesses package-private getter from HMaster. So in any case we'll need to make this or that one you mentioned public. As we are likely to manipulate HMaster a lot in this unit-test, I think it makes more sense to leave test class in master package.
Fixed other things.
Also extracted MetricsRegistry implementation. And this made me thinking about the following. It might makes sense to have different registries: StaticMetricsRegistry and DynamicMetricsRegistry. The latter one allows dynamically adding/removing metrics. This way where it is not needed StaticMetricsRegistry can be used and its implementation will be more efficient (no need for it to be thread-safe). Thoughts?
Had to give different name than MetricsRegistry to the one I added, since I had to put it into hadoop.metrics2.lib package.
Btw, Q to Elliott about the initial patch: I see you changed the existing "old" MasterMetrics, but I think we were discussing somewhere that we need to keep it for some time. Am I missing smth?
> 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_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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Elliott Clark (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13425943#comment-13425943 ]
Elliott Clark commented on HBASE-6411:
--------------------------------------
{quote}
Hm, I guess we now have two pairs of classes: MXBean and MXBeanImpl in org.apache.hadoop.hbase.master and in org.apache.hadoop.hbase.master.metrics. Not sure what was intended by Elliott here. I assume that he forgot to remove one of them (in org.apache.hadoop.hbase.master? why to move it in metrics package then?)
Elliott, could you provide some insight here please?
{quote}
Yea, I must have just missed deleting them. The move was just because those classes are only about metrics and not used anywhere else. So might as well clean up as we go. They were interface private so moving shouldn't be an issue.
{quote}
I believe Elliott is working on new such metrics (different issue) and this is why he removed it. Elliott?
{quote}
Correct. One of the sub-tasks of 4050 is creating a metrics2 histogram(Actually there will be two but that's out of scope) and using histograms where ever it's useful.
> 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-3.patch, HBASE-6411-4.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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Zhihong Ted Yu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13421566#comment-13421566 ]
Zhihong Ted Yu commented on HBASE-6411:
---------------------------------------
bq. increment shouldn't be on the registry. The registry is there to create and hold metrics.
Looking at MetricsRegistry source code in metrics2.lib of hadoop 1.0, I do see the following method:
{code}
public void incr(String name)
{code}
It would be nice to voice concern in a HADOOP JIRA so that it is heard by more developers.
> 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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Elliott Clark (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13418923#comment-13418923 ]
Elliott Clark commented on HBASE-6411:
--------------------------------------
Sorry didn't mean to re-assign. I must have done that when submitting to hadoop qa. Sorry I didn't mean to step on any toes.
I agree that a metrics factory or something like it could be very useful. However like I said above I was hoping to take a crack using guice to do most of the factory stuff. However maybe until I get that up it would be useful.
On #2 I don't think removing them interface completely is really the way to go since both the replication metrics and the region server metrics are mostly dynamic metrics; ie they aren't pre-created like the master metrics. I think it still makes sense to have a source that's mostly focused on those map based metrics.
> 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_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
[jira] [Updated] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Elliott Clark (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Elliott Clark updated HBASE-6411:
---------------------------------
Assignee: Elliott Clark (was: Alex Baranau)
Status: Patch Available (was: Open)
> 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_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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Alex Baranau (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13418505#comment-13418505 ]
Alex Baranau commented on HBASE-6411:
-------------------------------------
bq. Looking at MasterMetrics2, should it live in org.apache.hadoop.hbase.master.metrics2 package ?
Right
bq. Should we introduce config param such that HMaster.metrics / HMaster.myMetrics can be switched off at runtime ?
Do you think we might be OK with exposing two nodes in JMX when old metrics are not removed yet?
> 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: Alex Baranau
> Attachments: 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
[jira] [Updated] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Alex Baranau (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Alex Baranau updated HBASE-6411:
--------------------------------
Attachment: HBASE-6411-4_2.patch
Cleaned up redundant MXBean and MXBeanImpl. Looks like all comments and Qs are resolved.
> 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-3.patch, HBASE-6411-4.patch, HBASE-6411-4_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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Alex Baranau (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13418578#comment-13418578 ]
Alex Baranau commented on HBASE-6411:
-------------------------------------
@Elliott
Abstracting over third-party implementations makes perfect sense to me. With what you said in ming I guess it leads us to defining a good, more or less generic metrics-api in compat module, so that one can implement it with any tools they want (not just with metrics2 of hadoop) and at the same time it is usable in hbase code. This API might consist of basic interfaces/abstractions e.g. like in hadoop metrics2 framework: MetricsSystem, MetricsSource, MetricsSink, MetricInfo (Metric & MetricTag), metric types (counter/gauge), etc. If we want to define such an API, it will look closer to this patch though... Otherwise specific classes like: ReplicationMetricsSource doesn't look like generic API.
Or if we don't need it to be generic, this doesn't sound like *extracting* API for metrics system to implement, but rather *separating* "hbase-metrics" module/component, i.e. just putting all classes which implement metrics (incl. specific for HBase) into separate module (with an api in compat jar). I.e. *splitting* hbase codebase as opposed to reducing dependency on third party implementation of a tool (like metrics) _used_ by hbase.
Perhaps, *splitting* hbase codebase (not abstracting from third-party dependencies) is what really you intend?
> 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: Alex Baranau
> Attachments: 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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Elliott Clark (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13421552#comment-13421552 ]
Elliott Clark commented on HBASE-6411:
--------------------------------------
bq.Plan to refactor BaseMetricsSource & Impl, if there are no objections
We shouldn't care about dynamic/static metrics registry. They both do the same things. Right now we have that dichotomy and using it is a giant pain. I would vote strongly against that. Stack and I discussed long ago how having both makes it very hard to add dynamic metrics after the fact and how users should not need to know when somethings are dynamic metrics and others are static. So it's best to have them together.
bq.. not everyone probably should use JvmMetrics
It's only done once. We do always want to have jvm metrics, and it's better to have them there than to put that init into the base classes. So far HBase has put way too much knowledge of metrics into the base HRegion/HRegionServer/HMaster classes. Putting it in an init of the sources separates this
{code}
throw new MetricsException("Unsupported incr() for metric "+ name);
{code}
{code}
throw new MetricsException("Unsupported add(value) for metric "+ name);
{code}
Throwing code in metrics code is a big part of the stuff I was trying to avoid. Since we don't consider metrics exceptions fatal it leads to having to wrap things in giant try catches. These exceptions are not something that a user would ever need to know about so we shouldn't throw them.
{code}
+ /**
+ * Increment a metric by name.
+ * @param name of the metric
+ */
+ public void incr(String name) {
+ incr(name, mf);
+ }
{code}
increment shouldn't be on the registry. The registry is there to create and hold metrics. Others are there to manipulate them. 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.
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.
> 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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Alex Baranau (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13418443#comment-13418443 ]
Alex Baranau commented on HBASE-6411:
-------------------------------------
Oh, and I ran the build with:
{noformat}
mvn clean install -Dtest=TestMasterMetrics
{noformat}
and
{noformat}
MAVEN_OPTS=-Xmx2048m mvn clean test help:active-profiles -X -Dtest=TestMasterMetrics -Dhadoop.profile=2.0
{noformat}
> 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: Alex Baranau
> Attachments: 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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Zhihong Ted Yu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13425954#comment-13425954 ]
Zhihong Ted Yu commented on HBASE-6411:
---------------------------------------
Since MXBean.java is in master.metrics, should TestMXBean.java be in the same package ?
> 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-3.patch, HBASE-6411-4.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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Alex Baranau (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13418518#comment-13418518 ]
Alex Baranau commented on HBASE-6411:
-------------------------------------
Aha, sorry, misread.
Can we do that with hadoop-metrics(2).conf? Not sure. Will look at it
> 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: Alex Baranau
> Attachments: 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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Elliott Clark (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13421703#comment-13421703 ]
Elliott Clark commented on HBASE-6411:
--------------------------------------
Sorry for all the comment spam.
Looks like I left ReplicationMetricsSourceFactory in by accident. Can you remove it whenever you post your next version.
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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Elliott Clark (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13421659#comment-13421659 ]
Elliott Clark commented on HBASE-6411:
--------------------------------------
bq.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.
The only usage before was through a guaranteed singleton that was only used on a single thread at a time (The master rpc thread or the replication thread). Once there's the possibility that a registry can be shared other places we need to guard against concurrent access. I would vote using concurrent hash maps. That's usually my preferred way beacuse it's faster if there is a lot of contention.
bq.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:
Awesome. we're on the same page then. Too bad it's already in shipped hadoop code. Seems like a design flaw on their part. Oh well.
bq. That is why I suggested we replace this class with the registry in the first place.
When I started part of my purpose was to do the clean up so that the interfaces, used by classes in hbase-server, are not in any way tied to metrics2. I want to modularize things and make most of hbase's use of hadoop actually pluggable.
> 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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13427744#comment-13427744 ]
Hudson commented on HBASE-6411:
-------------------------------
Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #119 (See [https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/119/])
HBASE-6411 Move Master Metrics to metrics 2 (Alex Baranau) (Revision 1368598)
Result = FAILURE
tedyu :
Files :
* /hbase/trunk/conf/hadoop-metrics2.properties
* /hbase/trunk/hbase-hadoop-compat/pom.xml
* /hbase/trunk/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/CompatibilitySingletonFactory.java
* /hbase/trunk/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/master
* /hbase/trunk/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/master/metrics
* /hbase/trunk/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/master/metrics/MasterMetricsSource.java
* /hbase/trunk/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/metrics/MBeanSource.java
* /hbase/trunk/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/replication/regionserver/metrics/ReplicationMetricsSource.java
* /hbase/trunk/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/replication/regionserver/metrics/ReplicationMetricsSourceFactory.java
* /hbase/trunk/hbase-hadoop-compat/src/test/java/org/apache/hadoop/hbase/master
* /hbase/trunk/hbase-hadoop-compat/src/test/java/org/apache/hadoop/hbase/master/metrics
* /hbase/trunk/hbase-hadoop-compat/src/test/java/org/apache/hadoop/hbase/master/metrics/MasterMetricsSourceFactoryTest.java
* /hbase/trunk/hbase-hadoop-compat/src/test/java/org/apache/hadoop/hbase/replication/regionserver/metrics/ReplicationMetricsSourceFactoryTest.java
* /hbase/trunk/hbase-hadoop1-compat/src/main/java/org/apache/hadoop/hbase/master
* /hbase/trunk/hbase-hadoop1-compat/src/main/java/org/apache/hadoop/hbase/master/metrics
* /hbase/trunk/hbase-hadoop1-compat/src/main/java/org/apache/hadoop/hbase/master/metrics/MasterMetricsSourceImpl.java
* /hbase/trunk/hbase-hadoop1-compat/src/main/java/org/apache/hadoop/hbase/metrics/BaseMetricsSourceImpl.java
* /hbase/trunk/hbase-hadoop1-compat/src/main/java/org/apache/hadoop/hbase/metrics/MBeanSourceImpl.java
* /hbase/trunk/hbase-hadoop1-compat/src/main/java/org/apache/hadoop/hbase/replication/regionserver/metrics/ReplicationMetricsSourceImpl.java
* /hbase/trunk/hbase-hadoop1-compat/src/main/java/org/apache/hadoop/metrics2
* /hbase/trunk/hbase-hadoop1-compat/src/main/java/org/apache/hadoop/metrics2/lib
* /hbase/trunk/hbase-hadoop1-compat/src/main/java/org/apache/hadoop/metrics2/lib/DynamicMetricsRegistry.java
* /hbase/trunk/hbase-hadoop1-compat/src/main/resources/META-INF/services/org.apache.hadoop.hbase.master.metrics.MasterMetricsSource
* /hbase/trunk/hbase-hadoop1-compat/src/main/resources/META-INF/services/org.apache.hadoop.hbase.metrics.MBeanSource
* /hbase/trunk/hbase-hadoop1-compat/src/test/java/org/apache/hadoop/hbase/master
* /hbase/trunk/hbase-hadoop1-compat/src/test/java/org/apache/hadoop/hbase/master/metrics
* /hbase/trunk/hbase-hadoop1-compat/src/test/java/org/apache/hadoop/hbase/master/metrics/MasterMetricsSourceImplTest.java
* /hbase/trunk/hbase-hadoop1-compat/src/test/java/org/apache/hadoop/hbase/metrics/BaseMetricsSourceImplTest.java
* /hbase/trunk/hbase-hadoop1-compat/src/test/java/org/apache/hadoop/hbase/replication/regionserver/metrics/ReplicationMetricsSourceImplTest.java
* /hbase/trunk/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/master
* /hbase/trunk/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/master/metrics
* /hbase/trunk/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/master/metrics/MasterMetricsSourceImpl.java
* /hbase/trunk/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/metrics/BaseMetricsSourceImpl.java
* /hbase/trunk/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/metrics/MBeanSourceImpl.java
* /hbase/trunk/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/replication/regionserver/metrics/ReplicationMetricsSourceImpl.java
* /hbase/trunk/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/metrics2/lib/DynamicMetricsRegistry.java
* /hbase/trunk/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/metrics2/lib/HBaseMetricsFactory.java
* /hbase/trunk/hbase-hadoop2-compat/src/main/resources/META-INF/services/org.apache.hadoop.hbase.master.metrics.MasterMetricsSource
* /hbase/trunk/hbase-hadoop2-compat/src/main/resources/META-INF/services/org.apache.hadoop.hbase.metrics.MBeanSource
* /hbase/trunk/hbase-hadoop2-compat/src/test/java/org/apache/hadoop/hbase/master
* /hbase/trunk/hbase-hadoop2-compat/src/test/java/org/apache/hadoop/hbase/master/metrics
* /hbase/trunk/hbase-hadoop2-compat/src/test/java/org/apache/hadoop/hbase/master/metrics/MasterMetricsSourceImplTest.java
* /hbase/trunk/hbase-hadoop2-compat/src/test/java/org/apache/hadoop/hbase/metrics/BaseMetricsSourceImplTest.java
* /hbase/trunk/hbase-hadoop2-compat/src/test/java/org/apache/hadoop/hbase/replication/regionserver/metrics/ReplicationMetricsSourceImplTest.java
* /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
* /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MXBean.java
* /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MXBeanImpl.java
* /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/metrics/MXBean.java
* /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/metrics/MXBeanImpl.java
* /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/metrics/MasterMetrics.java
* /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/metrics/ReplicationSinkMetrics.java
* /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/metrics/ReplicationSourceMetrics.java
* /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMXBean.java
> 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: Alex Baranau
> Attachments: HBASE-6411-0.patch, HBASE-6411-1.patch, HBASE-6411-2.patch, HBASE-6411-3.patch, HBASE-6411-4.patch, HBASE-6411-4_2.patch, HBASE-6411-5.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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13418968#comment-13418968 ]
Hadoop QA commented on HBASE-6411:
----------------------------------
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12537268/HBASE-6411-0.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 18 new or modified tests.
+1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.
+1 javadoc. The javadoc tool did not generate any warning messages.
-1 javac. The applied patch generated 5 javac compiler warnings (more than the trunk's current 4 warnings).
-1 findbugs. The patch appears to introduce 16 new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
-1 core tests. The patch failed these unit tests:
org.apache.hadoop.hbase.coprocessor.TestRowProcessorEndpoint
org.apache.hadoop.hbase.security.access.TestZKPermissionsWatcher
Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/2418//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/2418//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/2418//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/2418//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/2418//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/2418//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/2418//console
This message is automatically generated.
> 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_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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Alex Baranau (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13418493#comment-13418493 ]
Alex Baranau commented on HBASE-6411:
-------------------------------------
bq. For my opinion this removes most abstraction of the actual metrics2 implementation, which was something I liked about the first approach. It would be possible for someone to implement their own metrics1 implementation with the earlier approach.
Right. I guess the Q is do we really need this extra abstraction?
At the same time, by adding extra abstraction hadoop-compat with hadoop1/2-compat modules in this case turn into smth like metrics-api and metrics-impl modules. I.e. not just "shim over hadoop classes". Which may be OK, I don't know.
> 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: Alex Baranau
> Attachments: 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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Alex Baranau (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13423373#comment-13423373 ]
Alex Baranau commented on HBASE-6411:
-------------------------------------
Here's why I'm concerned about rewriting existing MasterMetrics class: as a part of "revising metrics" we are likely to change existing metrics (incl. names that we expose, esp. dependent on old stuff like MetricsTimeVaryingRate). Though we now just copied in the initial patch all metrics from old master metrics, they may change. That's why in my first version I just added "empty placeholder" class with single "example" metric (cluster_requests), so that after we revise existing we can fill in more (and they are likely to be different from the old Master/RegionServer metrics). I guess this was discussed a bit in HBASE-4050.
May be that is more true for RegionServer metrics though where we have a lot of such old metrics types we don't want to support anymore. But I guess the approach should be the same w.r.t. "overwrite or add new metrics class".
Thanx for your answers!
> 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-3.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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Zhihong Ted Yu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13418513#comment-13418513 ]
Zhihong Ted Yu commented on HBASE-6411:
---------------------------------------
bq. OK with exposing two nodes in JMX
Yes.
I was thinking either of the two metrics systems can be switched off.
> 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: Alex Baranau
> Attachments: 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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Elliott Clark (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13418648#comment-13418648 ]
Elliott Clark commented on HBASE-6411:
--------------------------------------
The way I'm approaching it is hbase-hadoop1-compat and hbase-hadoop2-compat will be the implementation of things that are backed by hadoop 1 and 2 respectively. By putting things backed by the different versions into different modules we'll see where things are tied together; and get to untangle them.
Perhaps MasterMetricsSource and ReplicationMetricsSource was the wrong naming as it ties them too closely to the o.a.h.metrics2.MetricsSource. Maybe something like {Master|Replication|Base}MetricsExporter? would give the correct impression that it exposes metrics to the underlying metrics system.
> 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: Alex Baranau
> Attachments: 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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Alex Baranau (JIRA)" <ji...@apache.org>.
[ 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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Elliott Clark (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13423348#comment-13423348 ]
Elliott Clark commented on HBASE-6411:
--------------------------------------
bq.what about old MasterMetrics? Are we sure we want to replace it? I though we want to leave it for some time
The class is still there so we're not breaking compat. All that we're doing is changing which metrics system exports to file/jmx/ganglia. From the conversations I've had it seems like people are fine with changing to the new metrics system as the backing as long as we don't remove classes that could be used externally. eg. we'll have to leave the current implementation of MetricsHistorgram as it's possible that someone is using it outside of hbase internal code.
(That's all just my understanding and if someone disagree's that's fine too.)
bq.Should we probably unify that? E.g. push managing "sink.ageOfLastAppliedOp" metrics and such to xxxMetricsSource?
Stuffing dealing with the replication source uses dynamic metric names so it relies on the more general interface and since we wanted those metrics under the same node in jmx, I think the way we have it is fine for now; or at least good enough to leave it as is until after your patch gets applied.
> 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-3.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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Alex Baranau (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13418417#comment-13418417 ]
Alex Baranau commented on HBASE-6411:
-------------------------------------
To sum up:
To me, with approach in this patch, we *really* create *shim for hadoop classes* as opposed to moving HBase classes (metric sources implementations and such) into the shim layer. This way every class/interface from hadoop-compat acts as a shim for hadoop class, which makes it easier to switch to actual implementation later down the road. I'd say we may be should follow this logic when adding things into the shim layer.
But there might be a con to that: we have to provide shim for more classes with this approach. Though still it looks like there shouldn't be many of them. For now the difference boils down to two extra Mutable metrics classes.
> 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: Alex Baranau
> Attachments: 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
[jira] [Assigned] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Alex Baranau (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Alex Baranau reassigned HBASE-6411:
-----------------------------------
Assignee: Alex Baranau (was: Elliott Clark)
> 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: Alex Baranau
> Attachments: HBASE-6411-0.patch, HBASE-6411-1.patch, HBASE-6411-2.patch, HBASE-6411-3.patch, HBASE-6411-4.patch, HBASE-6411-4_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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Elliott Clark (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13418486#comment-13418486 ]
Elliott Clark commented on HBASE-6411:
--------------------------------------
For my opinion this removes most abstraction of the actual metrics2 implementation, which was something I liked about the first approach. It would be possible for someone to implement their own metrics1 implementation with the earlier approach. This approach metrics2 code and classes are seeping into the hbase-server code. In addition there are a lot more moving parts and a lot more code than is needed if more code is moved into the compat modules. I'll post a patch that shows what I'm thinking in a little bit.
> 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: Alex Baranau
> Attachments: 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
[jira] [Updated] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Alex Baranau (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Alex Baranau updated HBASE-6411:
--------------------------------
Attachment: HBASE-6411-1.patch
Adjusted Elliott's patch.
Added example unit-test for MasterMetrics that verifies metrics value change. Had to create MetricsAsserts shim in test sources in compat modules.
Please let me know what you think.
Will try to extract maps in BaseMetricsSourceImpl(s) into separate class and add support for MetricTags. I guess we agreed on that previously.
> 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_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
[jira] [Updated] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "liang xie (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
liang xie updated HBASE-6411:
-----------------------------
Fix Version/s: 0.96.0
> 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: Alex Baranau
> Fix For: 0.96.0
>
> Attachments: HBASE-6411-0.patch, HBASE-6411-1.patch, HBASE-6411-2.patch, HBASE-6411-3.patch, HBASE-6411-4_2.patch, HBASE-6411-4.patch, HBASE-6411-5.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
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13423512#comment-13423512 ]
Hadoop QA commented on HBASE-6411:
----------------------------------
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12538040/HBASE-6411-3.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 45 new or modified tests.
+1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.
+1 javadoc. The javadoc tool did not generate any warning messages.
-1 javac. The patch appears to cause mvn compile goal to fail.
-1 findbugs. The patch appears to cause Findbugs (version 1.3.9) to fail.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
-1 core tests. The patch failed these unit tests:
org.apache.hadoop.hbase.regionserver.TestServerCustomProtocol
Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/2442//testReport/
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/2442//console
This message is automatically generated.
> 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-3.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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13421515#comment-13421515 ]
Hadoop QA commented on HBASE-6411:
----------------------------------
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12537701/HBASE-6411-2.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 45 new or modified tests.
+1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.
+1 javadoc. The javadoc tool did not generate any warning messages.
-1 javac. The patch appears to cause mvn compile goal to fail.
-1 findbugs. The patch appears to cause Findbugs (version 1.3.9) to fail.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
-1 core tests. The patch failed these unit tests:
org.apache.hadoop.hbase.master.TestSplitLogManager
Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/2432//testReport/
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/2432//console
This message is automatically generated.
> 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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Alex Baranau (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13421600#comment-13421600 ]
Alex Baranau commented on HBASE-6411:
-------------------------------------
Some more words on incr() in registry. This may actually be OK. Or rather "not that big a deal". I.e. to me this interface that we have:
{noformat}
public interface BaseMetricsSource {
public void setGauge(String gaugeName, long value);
public void incGauge(String gaugeName, long delta);
public void decGauge(String gaugeName, long delta);
public void removeGauge(String key);
public void incCounters(String counterName, long delta);
public void removeCounter(String key);
}
{noformat}
looks like "registry description": smth which holds metrics (allows to add/remove them by name) and provides read/write access to them _by name_ (if one doesn't want to deal with the specific classes/implementations of those metrics). I.e. seems not that bad.
That is why I suggested we replace this class with the registry in the first place. But OK to leave it.
> 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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13427498#comment-13427498 ]
Hudson commented on HBASE-6411:
-------------------------------
Integrated in HBase-TRUNK #3190 (See [https://builds.apache.org/job/HBase-TRUNK/3190/])
HBASE-6411 Move Master Metrics to metrics 2 (Alex Baranau) (Revision 1368598)
Result = SUCCESS
tedyu :
Files :
* /hbase/trunk/conf/hadoop-metrics2.properties
* /hbase/trunk/hbase-hadoop-compat/pom.xml
* /hbase/trunk/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/CompatibilitySingletonFactory.java
* /hbase/trunk/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/master
* /hbase/trunk/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/master/metrics
* /hbase/trunk/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/master/metrics/MasterMetricsSource.java
* /hbase/trunk/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/metrics/MBeanSource.java
* /hbase/trunk/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/replication/regionserver/metrics/ReplicationMetricsSource.java
* /hbase/trunk/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/replication/regionserver/metrics/ReplicationMetricsSourceFactory.java
* /hbase/trunk/hbase-hadoop-compat/src/test/java/org/apache/hadoop/hbase/master
* /hbase/trunk/hbase-hadoop-compat/src/test/java/org/apache/hadoop/hbase/master/metrics
* /hbase/trunk/hbase-hadoop-compat/src/test/java/org/apache/hadoop/hbase/master/metrics/MasterMetricsSourceFactoryTest.java
* /hbase/trunk/hbase-hadoop-compat/src/test/java/org/apache/hadoop/hbase/replication/regionserver/metrics/ReplicationMetricsSourceFactoryTest.java
* /hbase/trunk/hbase-hadoop1-compat/src/main/java/org/apache/hadoop/hbase/master
* /hbase/trunk/hbase-hadoop1-compat/src/main/java/org/apache/hadoop/hbase/master/metrics
* /hbase/trunk/hbase-hadoop1-compat/src/main/java/org/apache/hadoop/hbase/master/metrics/MasterMetricsSourceImpl.java
* /hbase/trunk/hbase-hadoop1-compat/src/main/java/org/apache/hadoop/hbase/metrics/BaseMetricsSourceImpl.java
* /hbase/trunk/hbase-hadoop1-compat/src/main/java/org/apache/hadoop/hbase/metrics/MBeanSourceImpl.java
* /hbase/trunk/hbase-hadoop1-compat/src/main/java/org/apache/hadoop/hbase/replication/regionserver/metrics/ReplicationMetricsSourceImpl.java
* /hbase/trunk/hbase-hadoop1-compat/src/main/java/org/apache/hadoop/metrics2
* /hbase/trunk/hbase-hadoop1-compat/src/main/java/org/apache/hadoop/metrics2/lib
* /hbase/trunk/hbase-hadoop1-compat/src/main/java/org/apache/hadoop/metrics2/lib/DynamicMetricsRegistry.java
* /hbase/trunk/hbase-hadoop1-compat/src/main/resources/META-INF/services/org.apache.hadoop.hbase.master.metrics.MasterMetricsSource
* /hbase/trunk/hbase-hadoop1-compat/src/main/resources/META-INF/services/org.apache.hadoop.hbase.metrics.MBeanSource
* /hbase/trunk/hbase-hadoop1-compat/src/test/java/org/apache/hadoop/hbase/master
* /hbase/trunk/hbase-hadoop1-compat/src/test/java/org/apache/hadoop/hbase/master/metrics
* /hbase/trunk/hbase-hadoop1-compat/src/test/java/org/apache/hadoop/hbase/master/metrics/MasterMetricsSourceImplTest.java
* /hbase/trunk/hbase-hadoop1-compat/src/test/java/org/apache/hadoop/hbase/metrics/BaseMetricsSourceImplTest.java
* /hbase/trunk/hbase-hadoop1-compat/src/test/java/org/apache/hadoop/hbase/replication/regionserver/metrics/ReplicationMetricsSourceImplTest.java
* /hbase/trunk/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/master
* /hbase/trunk/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/master/metrics
* /hbase/trunk/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/master/metrics/MasterMetricsSourceImpl.java
* /hbase/trunk/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/metrics/BaseMetricsSourceImpl.java
* /hbase/trunk/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/metrics/MBeanSourceImpl.java
* /hbase/trunk/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/replication/regionserver/metrics/ReplicationMetricsSourceImpl.java
* /hbase/trunk/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/metrics2/lib/DynamicMetricsRegistry.java
* /hbase/trunk/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/metrics2/lib/HBaseMetricsFactory.java
* /hbase/trunk/hbase-hadoop2-compat/src/main/resources/META-INF/services/org.apache.hadoop.hbase.master.metrics.MasterMetricsSource
* /hbase/trunk/hbase-hadoop2-compat/src/main/resources/META-INF/services/org.apache.hadoop.hbase.metrics.MBeanSource
* /hbase/trunk/hbase-hadoop2-compat/src/test/java/org/apache/hadoop/hbase/master
* /hbase/trunk/hbase-hadoop2-compat/src/test/java/org/apache/hadoop/hbase/master/metrics
* /hbase/trunk/hbase-hadoop2-compat/src/test/java/org/apache/hadoop/hbase/master/metrics/MasterMetricsSourceImplTest.java
* /hbase/trunk/hbase-hadoop2-compat/src/test/java/org/apache/hadoop/hbase/metrics/BaseMetricsSourceImplTest.java
* /hbase/trunk/hbase-hadoop2-compat/src/test/java/org/apache/hadoop/hbase/replication/regionserver/metrics/ReplicationMetricsSourceImplTest.java
* /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
* /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MXBean.java
* /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MXBeanImpl.java
* /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/metrics/MXBean.java
* /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/metrics/MXBeanImpl.java
* /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/metrics/MasterMetrics.java
* /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/metrics/ReplicationSinkMetrics.java
* /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/metrics/ReplicationSourceMetrics.java
* /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMXBean.java
> 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: Alex Baranau
> Attachments: HBASE-6411-0.patch, HBASE-6411-1.patch, HBASE-6411-2.patch, HBASE-6411-3.patch, HBASE-6411-4.patch, HBASE-6411-4_2.patch, HBASE-6411-5.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
[jira] [Updated] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Alex Baranau (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Alex Baranau updated HBASE-6411:
--------------------------------
Resolution: Fixed
Release Note: Master metrics now use Hadoop's metrics2 framework
Status: Resolved (was: Patch Available)
Marking as fixed since patch was committed
> 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: Alex Baranau
> Attachments: HBASE-6411-0.patch, HBASE-6411-1.patch, HBASE-6411-2.patch, HBASE-6411-3.patch, HBASE-6411-4.patch, HBASE-6411-4_2.patch, HBASE-6411-5.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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Alex Baranau (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13418891#comment-13418891 ]
Alex Baranau commented on HBASE-6411:
-------------------------------------
Glanced over your patch. I like this way better (over initial patch at 4050): exposing the real interface of MetricsSource (in this case master metrics). I.e. with methods defines, not empty + hashmap.
1. What do you think about having MasterMetricsFactory available through compat module (created by CompatibilitySingletonFactory?) which is creating MetricsSource, like this:
interface MasterMetricsFactory {
MasterMetricsSource create(final String name, final String sessionId);
}
This way we could pass parameters and control creation of metrics source.
2. Independent on the above: how about removing BaseMetricsSource interface from compat as we don't really need it with explicit definition of metrics in sources? This way current BaseMetricsSourceImpl could be renamed to MetricsRegistry and used via composition (as a field) in metrics sources instead of realization. Thus, creating & initializing of the sources which might be different for each could stay in metrics source implementation itself. Including deciding on using JvmMetricsSource (I assume not every source should create it), etc.
This way they would look as normal metricsSources from hadoop codebase, just that they will use hbase's MetricsRegistry which allows metrics removals.
Thoughts?
> 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_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
[jira] [Updated] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Alex Baranau (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Alex Baranau updated HBASE-6411:
--------------------------------
Attachment: HBASE-6411-2.patch
Attached patch as per comment above.
Plan to refactor BaseMetricsSource & Impl, if there are no objections:
* metrics source initialization should be extracted as it is unique for every source (e.g. not everyone probably should use JvmMetrics, plus one (MasterMetrics) may want to pass extra params, like servername/sessionId)
> 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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Zhihong Ted Yu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13421145#comment-13421145 ]
Zhihong Ted Yu commented on HBASE-6411:
---------------------------------------
For TestMasterMetrics:
{code}
+ private void startCluster() throws Exception{
{code}
Normally such method is annotated with @Before
{code}
+ mxBean = CompatibilitySingletonFactory.getInstance(MBeanSource.class).register("hbase", "HMaster,sub=MXBean", mxBeanInfo);
{code}
The above line is longer than 100 characters.
If TestMasterMetrics is placed in master.metrics package, the following method can be package-private:
{code}
+ // for unit-test usage
+ public MasterMetricsSource getMetricsSource() {
{code}
{code}
+public class MetricsAssertsImpl extends MetricsAsserts {
{code}
Please add javadoc for the above class.
> 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_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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Alex Baranau (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13426070#comment-13426070 ]
Alex Baranau commented on HBASE-6411:
-------------------------------------
Not sure if I should do smth about these:
bq. -1 javac. The patch appears to cause mvn compile goal to fail.
bq. -1 findbugs. The patch appears to cause Findbugs (version 1.3.9) to fail.
Please let me know.
> 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: Alex Baranau
> Attachments: HBASE-6411-0.patch, HBASE-6411-1.patch, HBASE-6411-2.patch, HBASE-6411-3.patch, HBASE-6411-4.patch, HBASE-6411-4_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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Zhihong Ted Yu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13423567#comment-13423567 ]
Zhihong Ted Yu commented on HBASE-6411:
---------------------------------------
Put up some comments on review board.
> 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-3.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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Alex Baranau (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13421732#comment-13421732 ]
Alex Baranau commented on HBASE-6411:
-------------------------------------
bq. The only usage before was through a guaranteed singleton that was only used on a single thread at a time (The master rpc thread or the replication thread)
I think this remained the same. MetricsRegistry is used in same MetricSources as private field, so it is shared only if the source is shared. Anyhow, using concurrent hash map makes more sense to me too, if we are ok with a bit more of modifying "copied" MetricsRegistry code.
bq. Maybe we should file a new hbase jira to make the BaseMetricsSource have a generic increment and decrement only
I think it existing methods are OK. We need also at list remove() method as well.
I think we are mostly on the same page! :)
One thing that catches my eye:
* in MasterMetrics we have metrics values (counters & gauges) managed by the class in hadoop-compat module, which exposes interface for working with them (MasterMetricsSource)
* while with ReplicationSink(Source)Metrics we have different pattern: metrics values managed in the classes created in hbase-server code and ReplicationSourceMetrics interface is empty (provides just generic methods from BaseMetricsSource)
by "managed" I mean defines interface for creating/changing values.
Should we probably unify that? E.g. push managing "sink.ageOfLastAppliedOp" metrics and such to xxxMetricsSource?
> 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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13426066#comment-13426066 ]
Hadoop QA commented on HBASE-6411:
----------------------------------
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12538593/HBASE-6411-4_2.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 45 new or modified tests.
+1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.
+1 javadoc. The javadoc tool did not generate any warning messages.
-1 javac. The patch appears to cause mvn compile goal to fail.
-1 findbugs. The patch appears to cause Findbugs (version 1.3.9) to fail.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 core tests. The patch passed unit tests in .
Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/2463//testReport/
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/2463//console
This message is automatically generated.
> 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-3.patch, HBASE-6411-4.patch, HBASE-6411-4_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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Zhihong Ted Yu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13418496#comment-13418496 ]
Zhihong Ted Yu commented on HBASE-6411:
---------------------------------------
Looking at MasterMetrics2, should it live in org.apache.hadoop.hbase.master.metrics2 package ?
Should we introduce config param such that HMaster.metrics / HMaster.myMetrics can be switched off at runtime ?
> 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: Alex Baranau
> Attachments: 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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Alex Baranau (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13426005#comment-13426005 ]
Alex Baranau commented on HBASE-6411:
-------------------------------------
bq. Since MXBean.java is in master.metrics, should TestMXBean.java be in the same package ?
I'd say it may be the same situation as with TestMasterMetrics. It's just easier to place it here as test relies on access to master internal state heavily.
> 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-3.patch, HBASE-6411-4.patch, HBASE-6411-4_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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Elliott Clark (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13421683#comment-13421683 ]
Elliott Clark commented on HBASE-6411:
--------------------------------------
I like the generic increment even if we both think it's in the wrong location. Maybe we should file a new hbase jira to make the BaseMetricsSource have a generic increment and decrement only. Then if we try and do a set on something that was created as a counter it can be "promoted" to a gauge. Thoughts ?
> 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
[jira] [Updated] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Elliott Clark (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Elliott Clark updated HBASE-6411:
---------------------------------
Status: Patch Available (was: Open)
> 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_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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Alex Baranau (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13418898#comment-13418898 ]
Alex Baranau commented on HBASE-6411:
-------------------------------------
Looks like you reassigned the task, so I should probably not touch the patch to avoid intersection, right?
Was going to add actual metrics tests (which test metrics values changes in addition to testing factories/classes loading) and perhaps apply the 2nd point above, if it makes sense to you.
> 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_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
[jira] [Updated] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Elliott Clark (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Elliott Clark updated HBASE-6411:
---------------------------------
Attachment: HBASE-6411-0.patch
Here's a working implementation of master with metrics2. It includes some tests but not a whole lot. I plan to include a lot more once I am able to inject test metricsources (HBASE-6407).
It doesn't include histograms of the split size (HBASE-6409).
> 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: Alex Baranau
> Attachments: HBASE-6411-0.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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Zhihong Ted Yu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13427451#comment-13427451 ]
Zhihong Ted Yu commented on HBASE-6411:
---------------------------------------
I ran TestFromClientSideWithCoprocessor#testPoolBehavior and it passed locally.
Integrated to trunk.
Thanks for the patch, Alex.
Thanks for the review, Elliot.
> 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: Alex Baranau
> Attachments: HBASE-6411-0.patch, HBASE-6411-1.patch, HBASE-6411-2.patch, HBASE-6411-3.patch, HBASE-6411-4.patch, HBASE-6411-4_2.patch, HBASE-6411-5.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
[jira] [Updated] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Alex Baranau (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Alex Baranau updated HBASE-6411:
--------------------------------
Attachment: HBASE-6411-4.patch
Uploaded newer diff on review board. Fixed nits.
Note: after trunk update had to remove
{noformat}
public RegionsInTransitionInfo[] getRegionsInTransition()
{noformat}
method from master.metrics.MBean. I believe that this metric is now exposed via metrics, not via this bean. Please correct me if I'm wrong.
There are two Qs for Elliott inside, pasting here for convenience:
1.
{noformat}
/hbase-server/src/main/java/org/apache/hadoop/hbase/master/metrics/MXBean.java (Diff revision 1)
18
package org.apache.hadoop.hbase.master.metrics;
{noformat}
Ted:
Should this class be in org.apache.hadoop.hbase.master namespace ?
Alex Baranau:
Hm, I guess we now have two pairs of classes: MXBean and MXBeanImpl in org.apache.hadoop.hbase.master and in org.apache.hadoop.hbase.master.metrics. Not sure what was intended by Elliott here. I assume that he forgot to remove one of them (in org.apache.hadoop.hbase.master? why to move it in metrics package then?)
Elliott, could you provide some insight here please?
2.
{noformat}
/hbase-server/src/main/java/org/apache/hadoop/hbase/master/metrics/MasterMetrics.java (Diff revision 1)
63 Deleted:
final MetricsHistogram splitTime = new MetricsHistogram("splitTime", registry);
{noformat}
We don't maintain such metrics now ?
Alex Baranau
I believe Elliott is working on new such metrics (different issue) and this is why he removed it. Elliott?
> 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-3.patch, HBASE-6411-4.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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Elliott Clark (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13418517#comment-13418517 ]
Elliott Clark commented on HBASE-6411:
--------------------------------------
bq.At the same time, by adding extra abstraction hadoop-compat with hadoop1/2-compat modules in this case turn into smth like metrics-api and metrics-impl modules. I.e. not just "shim over hadoop classes". Which may be OK, I don't know.
Yes. That's what I was thinking. That's also why one of the work items is to start using guice to inject dependencies. However I am hoping that more than just metrics will be abstracted over. Right now we have issues with compiling for hadoop1 and running with hadoop2. I'm hoping that can be solved in the same way that we've solved metrics2 issues.
As things are modularized it gives us an opportunity make things more abstracted and maybe make it so that hbase's usage of hadoop is fully pluggable. This would make testing things easier and it could allow us to have other storage engines. (obviously that's a long way away and might not ever be realized, however just trying means that things become more testable.)
> 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: Alex Baranau
> Attachments: 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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13427419#comment-13427419 ]
Hadoop QA commented on HBASE-6411:
----------------------------------
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12538917/HBASE-6411-5.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 27 new or modified tests.
+1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.
+1 javadoc. The javadoc tool did not generate any warning messages.
-1 javac. The applied patch generated 5 javac compiler warnings (more than the trunk's current 4 warnings).
-1 findbugs. The patch appears to introduce 10 new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
-1 core tests. The patch failed these unit tests:
org.apache.hadoop.hbase.client.TestFromClientSideWithCoprocessor
Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/2482//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/2482//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/2482//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/2482//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/2482//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/2482//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/2482//console
This message is automatically generated.
> 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: Alex Baranau
> Attachments: HBASE-6411-0.patch, HBASE-6411-1.patch, HBASE-6411-2.patch, HBASE-6411-3.patch, HBASE-6411-4.patch, HBASE-6411-4_2.patch, HBASE-6411-5.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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Alex Baranau (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13427386#comment-13427386 ]
Alex Baranau commented on HBASE-6411:
-------------------------------------
As we discussed a bit with Ted, to move forward, let's exclude integration with hadoop's metrics2 unit-testing tools form this jira issue as it is somewhat problematic. Briefly we ran into this bug with maven: http://jira.codehaus.org/browse/MRRESOURCES-53.
I created separate task for that: HBASE-6501.
> 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: Alex Baranau
> Attachments: HBASE-6411-0.patch, HBASE-6411-1.patch, HBASE-6411-2.patch, HBASE-6411-3.patch, HBASE-6411-4.patch, HBASE-6411-4_2.patch, HBASE-6411-5.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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Elliott Clark (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13419399#comment-13419399 ]
Elliott Clark commented on HBASE-6411:
--------------------------------------
bq.Will try to extract maps in BaseMetricsSourceImpl(s) into separate class and add support for MetricTags. I guess we agreed on that previously.
Thanks. That sounds great.
> 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_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
[jira] [Updated] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Alex Baranau (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Alex Baranau updated HBASE-6411:
--------------------------------
Attachment: HBASE-6411_concept.patch
Attached patch which is based on my suggestion at HBASE-4050.
NOTE: Haven't changed added by HBASE-4050 classes.
NOTE: Added ability to create unit-tests for MetricsSources and added simplest unit-test for MasterMetrics.
NOTE: I know that this patch changes committed approach from Elliott's patch at HBASE-4050. Please refer to my last comments at HBASE-4050 for reasoning.
Again: I will be completely OK if you turn down my suggestion. Just wanted to share thoughts in which I saw some benefits.
Please, let me know what you think.
> 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: Alex Baranau
> Attachments: 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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Elliott Clark (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13423406#comment-13423406 ]
Elliott Clark commented on HBASE-6411:
--------------------------------------
Thanks so much for all your work on the patch. Functionally it looks great. I'm +1
> 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-3.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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Zhihong Ted Yu (JIRA)" <ji...@apache.org>.
[ 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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Elliott Clark (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13423399#comment-13423399 ]
Elliott Clark commented on HBASE-6411:
--------------------------------------
You're 100% correct we are for sure going to add, remove, and rename metrics; these changes are potentially a big deal. However, 0.96 is the singularity release so now's the time. Lars wanted a big metrics clean up so we might as well get started on that while moving over to metrics 2.
> 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-3.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
[jira] [Commented] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13419411#comment-13419411 ]
Hadoop QA commented on HBASE-6411:
----------------------------------
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12537375/HBASE-6411-1.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 39 new or modified tests.
+1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.
+1 javadoc. The javadoc tool did not generate any warning messages.
-1 javac. The patch appears to cause mvn compile goal to fail.
-1 findbugs. The patch appears to cause Findbugs (version 1.3.9) to fail.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
-1 core tests. The patch failed these unit tests:
org.apache.hadoop.hbase.client.TestFromClientSide
org.apache.hadoop.hbase.master.TestAssignmentManager
Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/2422//testReport/
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/2422//console
This message is automatically generated.
> 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_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
[jira] [Updated] (HBASE-6411) Move Master Metrics to metrics 2
Posted by "Elliott Clark (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-6411?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Elliott Clark updated HBASE-6411:
---------------------------------
Status: Open (was: Patch Available)
> 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_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