hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jimmy Xiang" <jxi...@cloudera.com>
Subject Re: Review Request 36241: HIVE-10927 : Add number of HMS connection metrics
Date Tue, 07 Jul 2015 19:17:20 GMT

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


Looks good to me. Just couple nits.


common/src/java/org/apache/hadoop/hive/common/metrics/LegacyMetrics.java (line 188)
<https://reviews.apache.org/r/36241/#comment143887>

    Nit. variable name: decrement?



common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java (line 81)
<https://reviews.apache.org/r/36241/#comment143888>

    Same here, and javadoc. decrement?



metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java (line 6075)
<https://reviews.apache.org/r/36241/#comment143889>

    Should we use metrics instead of calling MetricsFactory.getInstance() again?
    
    Same below.



metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java (line 270)
<https://reviews.apache.org/r/36241/#comment143892>

    Do we need to make openTransactionCalls volatile? This variable is accessed from multiple
threads, right?


- Jimmy Xiang


On July 7, 2015, 3 a.m., Szehon Ho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36241/
> -----------------------------------------------------------
> 
> (Updated July 7, 2015, 3 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-10927
>     https://issues.apache.org/jira/browse/HIVE-10927
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Adds following new metrics to HMS, can be renamed if necessary.
> 
> "open_connections";  //#HMS clients connecting to HMS
> 
> "active_jdo_transactions";  //#active JDO transactions
> "rollbacked_jdo_transactions";  //#failed JDO transactions
> "committed_jdo_transactions";  //#successful JDO transactions
> "opened_jdo_transactions";  //#attempted JDO transactions
> 
> Also to HS2:
> "open_connections";  //#HMS clients connecting to HMS
> 
> 
> This also fixes some minor issues:
> 1.  For the metrics JSON-file-reporter, the file system was not chosen right.  Fixing
that, and also making the default the local file system.
> 2.  The metrics was getting closed in the metastore in the wrong place, fixing it.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/common/JvmPauseMonitor.java ec5ac4a 
>   common/src/java/org/apache/hadoop/hive/common/metrics/LegacyMetrics.java e811339 
>   common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java 27b69cc 
>   common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsConstant.java PRE-CREATION

>   common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsVariable.java PRE-CREATION

>   common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java
ae353d0 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 6d0cf15 
>   common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestCodahaleMetrics.java
954b388 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreMetrics.java
25f34d1 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 0bcd053 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 4273c0b 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java dfb7faa 
> 
> Diff: https://reviews.apache.org/r/36241/diff/
> 
> 
> Testing
> -------
> 
> Adding some unit tests, and tested manually for HS2 which cannot be unit-tested.
> 
> 
> Thanks,
> 
> Szehon Ho
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message