You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Szehon Ho <sz...@cloudera.com> on 2015/05/20 01:58:10 UTC

Review Request 34447: HIVE-10761 : Create codahale-based metrics system for Hive

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34447/
-----------------------------------------------------------

Review request for hive.


Repository: hive-git


Description
-------

See JIRA for the motivation.  Summary: There is an existing metric system that uses some custom model and hooked up to JMX reporting, codahale-based metrics system will be desirable for standard model and reporting.

This adds a codahale-based metrics system to HiveServer2 and HiveMetastore.  Metrics implementation is now internally pluggable, and the existing Metrics system can be re-enabled by configuration if desired for backward-compatibility.

Following metrics are supported by Metrics system:
1.  JVMPauseMonitor (used to call Hadoop's internal implementation, now forked off to integrate with Metrics system)
2.  HMS API calls
3.  Standard JVM metrics (only for new implementation, as its free with codahale).

The following metrics reporting are supported by new system (configuration exposed)
1.  JMX
2.  CONSOLE
3.  JSON_FILE (periodic file of metrics that gets overwritten).

A goal is to add a webserver that exposes the JSON metrics, but this will defer to a later implementation.


Diffs
-----

  common/pom.xml a615c1e 
  common/src/java/org/apache/hadoop/hive/common/JvmPauseMonitor.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/Metrics.java 01c9d1d 
  common/src/java/org/apache/hadoop/hive/common/metrics/MetricsLegacy.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsFactory.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/Metrics.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/MetricsReporting.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java eff4d30 
  common/src/test/org/apache/hadoop/hive/common/metrics/TestMetrics.java e85d3f8 
  common/src/test/org/apache/hadoop/hive/common/metrics/TestMetricsLegacy.java PRE-CREATION 
  common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestMetrics.java PRE-CREATION 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java e796048 
  pom.xml 920f7a5 
  service/src/java/org/apache/hive/service/server/HiveServer2.java 58e8e49 
  shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java 6d8166c 
  shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java d349068 
  shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java 5a6bc44 

Diff: https://reviews.apache.org/r/34447/diff/


Testing
-------

New unit test added.  Manually tested.


Thanks,

Szehon Ho


Re: Review Request 34447: HIVE-10761 : Create codahale-based metrics system for Hive

Posted by Xuefu Zhang <xz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34447/#review86228
-----------------------------------------------------------

Ship it!



common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java
<https://reviews.apache.org/r/34447/#comment138147>

    Nit: It's good to close bw. One problem with this is that if bw=null, while new OutputStreamWriter() is successful, then this new OSW will not get closed. Very unlikely though.


- Xuefu Zhang


On May 31, 2015, 5:19 a.m., Szehon Ho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34447/
> -----------------------------------------------------------
> 
> (Updated May 31, 2015, 5:19 a.m.)
> 
> 
> Review request for hive, Chao Sun, Jimmy Xiang, and Xuefu Zhang.
> 
> 
> Bugs: HIVE-10761
>     https://issues.apache.org/jira/browse/HIVE-10761
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> See JIRA for the motivation.  Summary: There is an existing metric system that uses some custom model and hooked up to JMX reporting, codahale-based metrics system will be desirable for standard model and reporting.
> 
> This adds a codahale-based metrics system to HiveServer2 and HiveMetastore.  Metrics implementation is now internally pluggable, and the existing Metrics system can be re-enabled by configuration if desired for backward-compatibility.
> 
> Following metrics are supported by Metrics system:
> 1.  JVMPauseMonitor (used to call Hadoop's internal implementation, now forked off to integrate with Metrics system)
> 2.  HMS API calls
> 3.  Standard JVM metrics (only for new implementation, as its free with codahale).
> 
> The following metrics reporting are supported by new system (configuration exposed)
> 1.  JMX
> 2.  CONSOLE
> 3.  JSON_FILE (periodic file of metrics that gets overwritten).
> 
> A goal is to add a webserver that exposes the JSON metrics, but this will defer to a later implementation.
> 
> 
> Diffs
> -----
> 
>   common/pom.xml a615c1e 
>   common/src/java/org/apache/hadoop/hive/common/JvmPauseMonitor.java PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/common/metrics/LegacyMetrics.java PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/common/metrics/Metrics.java 01c9d1d 
>   common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsFactory.java PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/MetricsReporting.java PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 49b8f97 
>   common/src/test/org/apache/hadoop/hive/common/metrics/TestLegacyMetrics.java PRE-CREATION 
>   common/src/test/org/apache/hadoop/hive/common/metrics/TestMetrics.java e85d3f8 
>   common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestCodahaleMetrics.java PRE-CREATION 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreMetrics.java PRE-CREATION 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java d81c856 
>   pom.xml b21d894 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 58e8e49 
>   shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java 6d8166c 
>   shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java 19324b8 
>   shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java 5a6bc44 
> 
> Diff: https://reviews.apache.org/r/34447/diff/
> 
> 
> Testing
> -------
> 
> New unit test added.  Manually tested.
> 
> 
> Thanks,
> 
> Szehon Ho
> 
>


Re: Review Request 34447: HIVE-10761 : Create codahale-based metrics system for Hive

Posted by Szehon Ho <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34447/
-----------------------------------------------------------

(Updated May 31, 2015, 5:19 a.m.)


Review request for hive, Chao Sun, Jimmy Xiang, and Xuefu Zhang.


Changes
-------

One more minor change to the metrics file-reporter, where we are writing the file first then mv it, so reader won't see a partial file.  Mv should be atomic if same file-system.  Updating tests for the same.


Bugs: HIVE-10761
    https://issues.apache.org/jira/browse/HIVE-10761


Repository: hive-git


Description
-------

See JIRA for the motivation.  Summary: There is an existing metric system that uses some custom model and hooked up to JMX reporting, codahale-based metrics system will be desirable for standard model and reporting.

This adds a codahale-based metrics system to HiveServer2 and HiveMetastore.  Metrics implementation is now internally pluggable, and the existing Metrics system can be re-enabled by configuration if desired for backward-compatibility.

Following metrics are supported by Metrics system:
1.  JVMPauseMonitor (used to call Hadoop's internal implementation, now forked off to integrate with Metrics system)
2.  HMS API calls
3.  Standard JVM metrics (only for new implementation, as its free with codahale).

The following metrics reporting are supported by new system (configuration exposed)
1.  JMX
2.  CONSOLE
3.  JSON_FILE (periodic file of metrics that gets overwritten).

A goal is to add a webserver that exposes the JSON metrics, but this will defer to a later implementation.


Diffs (updated)
-----

  common/pom.xml a615c1e 
  common/src/java/org/apache/hadoop/hive/common/JvmPauseMonitor.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/LegacyMetrics.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/Metrics.java 01c9d1d 
  common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsFactory.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/MetricsReporting.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 49b8f97 
  common/src/test/org/apache/hadoop/hive/common/metrics/TestLegacyMetrics.java PRE-CREATION 
  common/src/test/org/apache/hadoop/hive/common/metrics/TestMetrics.java e85d3f8 
  common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestCodahaleMetrics.java PRE-CREATION 
  itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreMetrics.java PRE-CREATION 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java d81c856 
  pom.xml b21d894 
  service/src/java/org/apache/hive/service/server/HiveServer2.java 58e8e49 
  shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java 6d8166c 
  shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java 19324b8 
  shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java 5a6bc44 

Diff: https://reviews.apache.org/r/34447/diff/


Testing
-------

New unit test added.  Manually tested.


Thanks,

Szehon Ho


Re: Review Request 34447: HIVE-10761 : Create codahale-based metrics system for Hive

Posted by Szehon Ho <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34447/
-----------------------------------------------------------

(Updated May 29, 2015, 12:01 a.m.)


Review request for hive, Chao Sun, Jimmy Xiang, and Xuefu Zhang.


Changes
-------

1.  Address review comments
2.  Minor change to new config names due to some other feedback.
3.  Adding code to properly de-initialize the CodahaleMetrics
4.  Testing this, and also making the test more resilient in general (in the rare case that file is read as its being written).


Bugs: HIVE-10761
    https://issues.apache.org/jira/browse/HIVE-10761


Repository: hive-git


Description
-------

See JIRA for the motivation.  Summary: There is an existing metric system that uses some custom model and hooked up to JMX reporting, codahale-based metrics system will be desirable for standard model and reporting.

This adds a codahale-based metrics system to HiveServer2 and HiveMetastore.  Metrics implementation is now internally pluggable, and the existing Metrics system can be re-enabled by configuration if desired for backward-compatibility.

Following metrics are supported by Metrics system:
1.  JVMPauseMonitor (used to call Hadoop's internal implementation, now forked off to integrate with Metrics system)
2.  HMS API calls
3.  Standard JVM metrics (only for new implementation, as its free with codahale).

The following metrics reporting are supported by new system (configuration exposed)
1.  JMX
2.  CONSOLE
3.  JSON_FILE (periodic file of metrics that gets overwritten).

A goal is to add a webserver that exposes the JSON metrics, but this will defer to a later implementation.


Diffs (updated)
-----

  common/pom.xml a615c1e 
  common/src/java/org/apache/hadoop/hive/common/JvmPauseMonitor.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/LegacyMetrics.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/Metrics.java 01c9d1d 
  common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsFactory.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/MetricsReporting.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 49b8f97 
  common/src/test/org/apache/hadoop/hive/common/metrics/TestLegacyMetrics.java PRE-CREATION 
  common/src/test/org/apache/hadoop/hive/common/metrics/TestMetrics.java e85d3f8 
  common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestCodahaleMetrics.java PRE-CREATION 
  itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreMetrics.java PRE-CREATION 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java d81c856 
  pom.xml b21d894 
  service/src/java/org/apache/hive/service/server/HiveServer2.java 58e8e49 
  shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java 6d8166c 
  shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java 19324b8 
  shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java 5a6bc44 

Diff: https://reviews.apache.org/r/34447/diff/


Testing
-------

New unit test added.  Manually tested.


Thanks,

Szehon Ho


Re: Review Request 34447: HIVE-10761 : Create codahale-based metrics system for Hive

Posted by Szehon Ho <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34447/
-----------------------------------------------------------

(Updated May 28, 2015, 2:11 a.m.)


Review request for hive, Chao Sun, Jimmy Xiang, and Xuefu Zhang.


Changes
-------

Address review comments.


Bugs: HIVE-10761
    https://issues.apache.org/jira/browse/HIVE-10761


Repository: hive-git


Description
-------

See JIRA for the motivation.  Summary: There is an existing metric system that uses some custom model and hooked up to JMX reporting, codahale-based metrics system will be desirable for standard model and reporting.

This adds a codahale-based metrics system to HiveServer2 and HiveMetastore.  Metrics implementation is now internally pluggable, and the existing Metrics system can be re-enabled by configuration if desired for backward-compatibility.

Following metrics are supported by Metrics system:
1.  JVMPauseMonitor (used to call Hadoop's internal implementation, now forked off to integrate with Metrics system)
2.  HMS API calls
3.  Standard JVM metrics (only for new implementation, as its free with codahale).

The following metrics reporting are supported by new system (configuration exposed)
1.  JMX
2.  CONSOLE
3.  JSON_FILE (periodic file of metrics that gets overwritten).

A goal is to add a webserver that exposes the JSON metrics, but this will defer to a later implementation.


Diffs (updated)
-----

  common/pom.xml a615c1e 
  common/src/java/org/apache/hadoop/hive/common/JvmPauseMonitor.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/LegacyMetrics.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/Metrics.java 01c9d1d 
  common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsFactory.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/MetricsReporting.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 49b8f97 
  common/src/test/org/apache/hadoop/hive/common/metrics/TestLegacyMetrics.java PRE-CREATION 
  common/src/test/org/apache/hadoop/hive/common/metrics/TestMetrics.java e85d3f8 
  common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestCodahaleMetrics.java PRE-CREATION 
  itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreMetrics.java PRE-CREATION 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java d81c856 
  pom.xml b21d894 
  service/src/java/org/apache/hive/service/server/HiveServer2.java 58e8e49 
  shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java 6d8166c 
  shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java 19324b8 
  shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java 5a6bc44 

Diff: https://reviews.apache.org/r/34447/diff/


Testing
-------

New unit test added.  Manually tested.


Thanks,

Szehon Ho


Re: Review Request 34447: HIVE-10761 : Create codahale-based metrics system for Hive

Posted by Xuefu Zhang <xz...@cloudera.com>.

> On May 27, 2015, 9:29 p.m., Xuefu Zhang wrote:
> > common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/Metrics.java, line 141
> > <https://reviews.apache.org/r/34447/diff/3/?file=972974#file972974line141>
> >
> >     If the synchronized block is for the whole method, we might just as well declare the whole method as synchronized.
> 
> Szehon Ho wrote:
>     In this context, I think a object synchronization makes more sense than synchronizing on the class (sycnrhonized method).

I think they are equivalent. A synchronized method is synchronizing on "this". It will be on the class if the method is static.


- Xuefu


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34447/#review85418
-----------------------------------------------------------


On May 28, 2015, 2:11 a.m., Szehon Ho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34447/
> -----------------------------------------------------------
> 
> (Updated May 28, 2015, 2:11 a.m.)
> 
> 
> Review request for hive, Chao Sun, Jimmy Xiang, and Xuefu Zhang.
> 
> 
> Bugs: HIVE-10761
>     https://issues.apache.org/jira/browse/HIVE-10761
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> See JIRA for the motivation.  Summary: There is an existing metric system that uses some custom model and hooked up to JMX reporting, codahale-based metrics system will be desirable for standard model and reporting.
> 
> This adds a codahale-based metrics system to HiveServer2 and HiveMetastore.  Metrics implementation is now internally pluggable, and the existing Metrics system can be re-enabled by configuration if desired for backward-compatibility.
> 
> Following metrics are supported by Metrics system:
> 1.  JVMPauseMonitor (used to call Hadoop's internal implementation, now forked off to integrate with Metrics system)
> 2.  HMS API calls
> 3.  Standard JVM metrics (only for new implementation, as its free with codahale).
> 
> The following metrics reporting are supported by new system (configuration exposed)
> 1.  JMX
> 2.  CONSOLE
> 3.  JSON_FILE (periodic file of metrics that gets overwritten).
> 
> A goal is to add a webserver that exposes the JSON metrics, but this will defer to a later implementation.
> 
> 
> Diffs
> -----
> 
>   common/pom.xml a615c1e 
>   common/src/java/org/apache/hadoop/hive/common/JvmPauseMonitor.java PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/common/metrics/LegacyMetrics.java PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/common/metrics/Metrics.java 01c9d1d 
>   common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsFactory.java PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/MetricsReporting.java PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 49b8f97 
>   common/src/test/org/apache/hadoop/hive/common/metrics/TestLegacyMetrics.java PRE-CREATION 
>   common/src/test/org/apache/hadoop/hive/common/metrics/TestMetrics.java e85d3f8 
>   common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestCodahaleMetrics.java PRE-CREATION 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreMetrics.java PRE-CREATION 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java d81c856 
>   pom.xml b21d894 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 58e8e49 
>   shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java 6d8166c 
>   shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java 19324b8 
>   shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java 5a6bc44 
> 
> Diff: https://reviews.apache.org/r/34447/diff/
> 
> 
> Testing
> -------
> 
> New unit test added.  Manually tested.
> 
> 
> Thanks,
> 
> Szehon Ho
> 
>


Re: Review Request 34447: HIVE-10761 : Create codahale-based metrics system for Hive

Posted by Szehon Ho <sz...@cloudera.com>.

> On May 27, 2015, 9:29 p.m., Xuefu Zhang wrote:
> > common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/Metrics.java, line 141
> > <https://reviews.apache.org/r/34447/diff/3/?file=972974#file972974line141>
> >
> >     If the synchronized block is for the whole method, we might just as well declare the whole method as synchronized.

In this context, I think a object synchronization makes more sense than synchronizing on the class (sycnrhonized method).


- Szehon


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34447/#review85418
-----------------------------------------------------------


On May 28, 2015, 2:11 a.m., Szehon Ho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34447/
> -----------------------------------------------------------
> 
> (Updated May 28, 2015, 2:11 a.m.)
> 
> 
> Review request for hive, Chao Sun, Jimmy Xiang, and Xuefu Zhang.
> 
> 
> Bugs: HIVE-10761
>     https://issues.apache.org/jira/browse/HIVE-10761
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> See JIRA for the motivation.  Summary: There is an existing metric system that uses some custom model and hooked up to JMX reporting, codahale-based metrics system will be desirable for standard model and reporting.
> 
> This adds a codahale-based metrics system to HiveServer2 and HiveMetastore.  Metrics implementation is now internally pluggable, and the existing Metrics system can be re-enabled by configuration if desired for backward-compatibility.
> 
> Following metrics are supported by Metrics system:
> 1.  JVMPauseMonitor (used to call Hadoop's internal implementation, now forked off to integrate with Metrics system)
> 2.  HMS API calls
> 3.  Standard JVM metrics (only for new implementation, as its free with codahale).
> 
> The following metrics reporting are supported by new system (configuration exposed)
> 1.  JMX
> 2.  CONSOLE
> 3.  JSON_FILE (periodic file of metrics that gets overwritten).
> 
> A goal is to add a webserver that exposes the JSON metrics, but this will defer to a later implementation.
> 
> 
> Diffs
> -----
> 
>   common/pom.xml a615c1e 
>   common/src/java/org/apache/hadoop/hive/common/JvmPauseMonitor.java PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/common/metrics/LegacyMetrics.java PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/common/metrics/Metrics.java 01c9d1d 
>   common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsFactory.java PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/MetricsReporting.java PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 49b8f97 
>   common/src/test/org/apache/hadoop/hive/common/metrics/TestLegacyMetrics.java PRE-CREATION 
>   common/src/test/org/apache/hadoop/hive/common/metrics/TestMetrics.java e85d3f8 
>   common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestCodahaleMetrics.java PRE-CREATION 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreMetrics.java PRE-CREATION 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java d81c856 
>   pom.xml b21d894 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 58e8e49 
>   shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java 6d8166c 
>   shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java 19324b8 
>   shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java 5a6bc44 
> 
> Diff: https://reviews.apache.org/r/34447/diff/
> 
> 
> Testing
> -------
> 
> New unit test added.  Manually tested.
> 
> 
> Thanks,
> 
> Szehon Ho
> 
>


Re: Review Request 34447: HIVE-10761 : Create codahale-based metrics system for Hive

Posted by Xuefu Zhang <xz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34447/#review85418
-----------------------------------------------------------



common/src/java/org/apache/hadoop/hive/common/JvmPauseMonitor.java
<https://reviews.apache.org/r/34447/#comment136981>

    Maybe a more informational message



common/src/java/org/apache/hadoop/hive/common/JvmPauseMonitor.java
<https://reviews.apache.org/r/34447/#comment136983>

    should we check isStarted()?



common/src/java/org/apache/hadoop/hive/common/metrics/MetricsLegacy.java
<https://reviews.apache.org/r/34447/#comment137016>

    LegacyMetrics?



common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsFactory.java
<https://reviews.apache.org/r/34447/#comment136986>

    This should be also synchronized.



common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsFactory.java
<https://reviews.apache.org/r/34447/#comment137006>

    Should we call it deinit()?



common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/Metrics.java
<https://reviews.apache.org/r/34447/#comment137008>

    Could we rename the class so that we don't have to handle the duplicated class/interface names?



common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/Metrics.java
<https://reviews.apache.org/r/34447/#comment137010>

    Could we rename the class so that we don't have to handle the duplicated class/interface names?



common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/Metrics.java
<https://reviews.apache.org/r/34447/#comment137009>

    If the synchronized block is for the whole method, we might just as well declare the whole method as synchronized.



common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/Metrics.java
<https://reviews.apache.org/r/34447/#comment137011>

    Same as above.



common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/Metrics.java
<https://reviews.apache.org/r/34447/#comment137013>

    Shouldn't this be private?



common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/Metrics.java
<https://reviews.apache.org/r/34447/#comment137012>

    I think fd needs to be closed properly in final block.



common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/Metrics.java
<https://reviews.apache.org/r/34447/#comment137014>

    I think checking initialized needs to be synchronized.



common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/Metrics.java
<https://reviews.apache.org/r/34447/#comment137015>

    Same as above.



metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/34447/#comment137007>

    Where do we call uninit() or it doesn't matter? Same for HS2.


- Xuefu Zhang


On May 27, 2015, 6:25 p.m., Szehon Ho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34447/
> -----------------------------------------------------------
> 
> (Updated May 27, 2015, 6:25 p.m.)
> 
> 
> Review request for hive, Chao Sun, Jimmy Xiang, and Xuefu Zhang.
> 
> 
> Bugs: HIVE-10761
>     https://issues.apache.org/jira/browse/HIVE-10761
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> See JIRA for the motivation.  Summary: There is an existing metric system that uses some custom model and hooked up to JMX reporting, codahale-based metrics system will be desirable for standard model and reporting.
> 
> This adds a codahale-based metrics system to HiveServer2 and HiveMetastore.  Metrics implementation is now internally pluggable, and the existing Metrics system can be re-enabled by configuration if desired for backward-compatibility.
> 
> Following metrics are supported by Metrics system:
> 1.  JVMPauseMonitor (used to call Hadoop's internal implementation, now forked off to integrate with Metrics system)
> 2.  HMS API calls
> 3.  Standard JVM metrics (only for new implementation, as its free with codahale).
> 
> The following metrics reporting are supported by new system (configuration exposed)
> 1.  JMX
> 2.  CONSOLE
> 3.  JSON_FILE (periodic file of metrics that gets overwritten).
> 
> A goal is to add a webserver that exposes the JSON metrics, but this will defer to a later implementation.
> 
> 
> Diffs
> -----
> 
>   common/pom.xml a615c1e 
>   common/src/java/org/apache/hadoop/hive/common/JvmPauseMonitor.java PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/common/metrics/Metrics.java 01c9d1d 
>   common/src/java/org/apache/hadoop/hive/common/metrics/MetricsLegacy.java PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsFactory.java PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/Metrics.java PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/MetricsReporting.java PRE-CREATION 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 49b8f97 
>   common/src/test/org/apache/hadoop/hive/common/metrics/TestMetrics.java e85d3f8 
>   common/src/test/org/apache/hadoop/hive/common/metrics/TestMetricsLegacy.java PRE-CREATION 
>   common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestMetrics.java PRE-CREATION 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreMetrics.java PRE-CREATION 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java d81c856 
>   pom.xml b21d894 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 58e8e49 
>   shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java 6d8166c 
>   shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java 19324b8 
>   shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java 5a6bc44 
> 
> Diff: https://reviews.apache.org/r/34447/diff/
> 
> 
> Testing
> -------
> 
> New unit test added.  Manually tested.
> 
> 
> Thanks,
> 
> Szehon Ho
> 
>


Re: Review Request 34447: HIVE-10761 : Create codahale-based metrics system for Hive

Posted by Szehon Ho <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34447/
-----------------------------------------------------------

(Updated May 27, 2015, 6:25 p.m.)


Review request for hive, Chao Sun, Jimmy Xiang, and Xuefu Zhang.


Changes
-------

Rebase the patch.


Bugs: HIVE-10761
    https://issues.apache.org/jira/browse/HIVE-10761


Repository: hive-git


Description
-------

See JIRA for the motivation.  Summary: There is an existing metric system that uses some custom model and hooked up to JMX reporting, codahale-based metrics system will be desirable for standard model and reporting.

This adds a codahale-based metrics system to HiveServer2 and HiveMetastore.  Metrics implementation is now internally pluggable, and the existing Metrics system can be re-enabled by configuration if desired for backward-compatibility.

Following metrics are supported by Metrics system:
1.  JVMPauseMonitor (used to call Hadoop's internal implementation, now forked off to integrate with Metrics system)
2.  HMS API calls
3.  Standard JVM metrics (only for new implementation, as its free with codahale).

The following metrics reporting are supported by new system (configuration exposed)
1.  JMX
2.  CONSOLE
3.  JSON_FILE (periodic file of metrics that gets overwritten).

A goal is to add a webserver that exposes the JSON metrics, but this will defer to a later implementation.


Diffs (updated)
-----

  common/pom.xml a615c1e 
  common/src/java/org/apache/hadoop/hive/common/JvmPauseMonitor.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/Metrics.java 01c9d1d 
  common/src/java/org/apache/hadoop/hive/common/metrics/MetricsLegacy.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsFactory.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/Metrics.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/MetricsReporting.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 49b8f97 
  common/src/test/org/apache/hadoop/hive/common/metrics/TestMetrics.java e85d3f8 
  common/src/test/org/apache/hadoop/hive/common/metrics/TestMetricsLegacy.java PRE-CREATION 
  common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestMetrics.java PRE-CREATION 
  itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreMetrics.java PRE-CREATION 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java d81c856 
  pom.xml b21d894 
  service/src/java/org/apache/hive/service/server/HiveServer2.java 58e8e49 
  shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java 6d8166c 
  shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java 19324b8 
  shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java 5a6bc44 

Diff: https://reviews.apache.org/r/34447/diff/


Testing
-------

New unit test added.  Manually tested.


Thanks,

Szehon Ho


Re: Review Request 34447: HIVE-10761 : Create codahale-based metrics system for Hive

Posted by Szehon Ho <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34447/
-----------------------------------------------------------

(Updated May 27, 2015, 2:04 a.m.)


Review request for hive, Chao Sun, Jimmy Xiang, and Xuefu Zhang.


Changes
-------

Some changes: Make the metrics take in a list of configured reporters, add a new E2E test for the basic Metastore metrics.


Bugs: HIVE-10761
    https://issues.apache.org/jira/browse/HIVE-10761


Repository: hive-git


Description
-------

See JIRA for the motivation.  Summary: There is an existing metric system that uses some custom model and hooked up to JMX reporting, codahale-based metrics system will be desirable for standard model and reporting.

This adds a codahale-based metrics system to HiveServer2 and HiveMetastore.  Metrics implementation is now internally pluggable, and the existing Metrics system can be re-enabled by configuration if desired for backward-compatibility.

Following metrics are supported by Metrics system:
1.  JVMPauseMonitor (used to call Hadoop's internal implementation, now forked off to integrate with Metrics system)
2.  HMS API calls
3.  Standard JVM metrics (only for new implementation, as its free with codahale).

The following metrics reporting are supported by new system (configuration exposed)
1.  JMX
2.  CONSOLE
3.  JSON_FILE (periodic file of metrics that gets overwritten).

A goal is to add a webserver that exposes the JSON metrics, but this will defer to a later implementation.


Diffs (updated)
-----

  common/pom.xml a615c1e 
  common/src/java/org/apache/hadoop/hive/common/JvmPauseMonitor.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/Metrics.java 01c9d1d 
  common/src/java/org/apache/hadoop/hive/common/metrics/MetricsLegacy.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsFactory.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/Metrics.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/MetricsReporting.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java eff4d30 
  common/src/test/org/apache/hadoop/hive/common/metrics/TestMetrics.java e85d3f8 
  common/src/test/org/apache/hadoop/hive/common/metrics/TestMetricsLegacy.java PRE-CREATION 
  common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestMetrics.java PRE-CREATION 
  itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreMetrics.java PRE-CREATION 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java e796048 
  pom.xml 920f7a5 
  service/src/java/org/apache/hive/service/server/HiveServer2.java 58e8e49 
  shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java 6d8166c 
  shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java d349068 
  shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java 5a6bc44 

Diff: https://reviews.apache.org/r/34447/diff/


Testing
-------

New unit test added.  Manually tested.


Thanks,

Szehon Ho


Re: Review Request 34447: HIVE-10761 : Create codahale-based metrics system for Hive

Posted by Szehon Ho <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34447/
-----------------------------------------------------------

(Updated May 19, 2015, 11:58 p.m.)


Review request for hive.


Bugs: HIVE-10761
    https://issues.apache.org/jira/browse/HIVE-10761


Repository: hive-git


Description
-------

See JIRA for the motivation.  Summary: There is an existing metric system that uses some custom model and hooked up to JMX reporting, codahale-based metrics system will be desirable for standard model and reporting.

This adds a codahale-based metrics system to HiveServer2 and HiveMetastore.  Metrics implementation is now internally pluggable, and the existing Metrics system can be re-enabled by configuration if desired for backward-compatibility.

Following metrics are supported by Metrics system:
1.  JVMPauseMonitor (used to call Hadoop's internal implementation, now forked off to integrate with Metrics system)
2.  HMS API calls
3.  Standard JVM metrics (only for new implementation, as its free with codahale).

The following metrics reporting are supported by new system (configuration exposed)
1.  JMX
2.  CONSOLE
3.  JSON_FILE (periodic file of metrics that gets overwritten).

A goal is to add a webserver that exposes the JSON metrics, but this will defer to a later implementation.


Diffs
-----

  common/pom.xml a615c1e 
  common/src/java/org/apache/hadoop/hive/common/JvmPauseMonitor.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/Metrics.java 01c9d1d 
  common/src/java/org/apache/hadoop/hive/common/metrics/MetricsLegacy.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsFactory.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/Metrics.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/MetricsReporting.java PRE-CREATION 
  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java eff4d30 
  common/src/test/org/apache/hadoop/hive/common/metrics/TestMetrics.java e85d3f8 
  common/src/test/org/apache/hadoop/hive/common/metrics/TestMetricsLegacy.java PRE-CREATION 
  common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestMetrics.java PRE-CREATION 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java e796048 
  pom.xml 920f7a5 
  service/src/java/org/apache/hive/service/server/HiveServer2.java 58e8e49 
  shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java 6d8166c 
  shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java d349068 
  shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java 5a6bc44 

Diff: https://reviews.apache.org/r/34447/diff/


Testing
-------

New unit test added.  Manually tested.


Thanks,

Szehon Ho