Re: Review Request 69672: HIVE-21045: Add total API timing stats and connection pool stats to metrics

2019-01-11 Thread Yongzhi Chen via Review Board 

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


Ship it!




Ship It!

- Yongzhi Chen


On Jan. 5, 2019, 12:41 a.m., Karthik Manamcheri wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69672/
> ---
> 
> (Updated Jan. 5, 2019, 12:41 a.m.)
> 
> 
> Review request for hive, Adam Holley, Morio Ramdenbourg, Naveen Gangam, and 
> Vihang Karajgaonkar.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-21045: Add total API timing stats and connection pool stats to metrics
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/PersistenceManagerProvider.java
>  dfd7abff85 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/datasource/BoneCPDataSourceProvider.java
>  7e33c519a8 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/datasource/DataSourceProvider.java
>  6dc63fb3bc 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/datasource/DataSourceProviderFactory.java
>  5a92e104be 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/datasource/DbCPDataSourceProvider.java
>  7fe487b184 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/datasource/HikariCPDataSourceProvider.java
>  8f6ae57e36 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metrics/MetricsConstants.java
>  3b188f83af 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metrics/PerfLogger.java
>  a2def26fc5 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  2a6290315a 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/datasource/TestDataSourceProviderFactory.java
>  6ae7f50471 
> 
> 
> Diff: https://reviews.apache.org/r/69672/diff/1/
> 
> 
> Testing
> ---
> 
> Manual testing to verify that the new metrics show up for hikaricp, bonecp, 
> and also the total stats. Here are samples of
> 1. [HikariCP json metrics 
> sample](https://gist.github.com/kmanamcheri/48ff2a680e85c7e925a6f95a9384dcef)
> 2. [BoneCP json metrics 
> sample](https://gist.github.com/kmanamcheri/b005f68263a1a1be06b25156a159d975)
> 
> In both the reports note that there are pool gauges (for tracking the 
> connection pool info) and also a timer for total api calls.
> 
> 
> Thanks,
> 
> Karthik Manamcheri
> 
>

Re: Review Request 69672: HIVE-21045: Add total API timing stats and connection pool stats to metrics

2019-01-09 Thread Yongzhi Chen via Review Board 

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




standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/PersistenceManagerProvider.java
Line 228 (original), 227 (patched)


This is a little bit different from the old impl funtionally : The old impl 
will return null if there is no custom properties, where new impl will still 
return the provider. The old impl has a kind of sanity check. But if the custom 
properties are not required here, it should be fine.



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/datasource/BoneCPDataSourceProvider.java
Lines 101 (patched)


If registry is null, should we give a warning in the log?


- Yongzhi Chen


On Jan. 5, 2019, 12:41 a.m., Karthik Manamcheri wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69672/
> ---
> 
> (Updated Jan. 5, 2019, 12:41 a.m.)
> 
> 
> Review request for hive, Adam Holley, Morio Ramdenbourg, Naveen Gangam, and 
> Vihang Karajgaonkar.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-21045: Add total API timing stats and connection pool stats to metrics
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/PersistenceManagerProvider.java
>  dfd7abff85 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/datasource/BoneCPDataSourceProvider.java
>  7e33c519a8 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/datasource/DataSourceProvider.java
>  6dc63fb3bc 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/datasource/DataSourceProviderFactory.java
>  5a92e104be 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/datasource/DbCPDataSourceProvider.java
>  7fe487b184 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/datasource/HikariCPDataSourceProvider.java
>  8f6ae57e36 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metrics/MetricsConstants.java
>  3b188f83af 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metrics/PerfLogger.java
>  a2def26fc5 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
>  2a6290315a 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/datasource/TestDataSourceProviderFactory.java
>  6ae7f50471 
> 
> 
> Diff: https://reviews.apache.org/r/69672/diff/1/
> 
> 
> Testing
> ---
> 
> Manual testing to verify that the new metrics show up for hikaricp, bonecp, 
> and also the total stats. Here are samples of
> 1. [HikariCP json metrics 
> sample](https://gist.github.com/kmanamcheri/48ff2a680e85c7e925a6f95a9384dcef)
> 2. [BoneCP json metrics 
> sample](https://gist.github.com/kmanamcheri/b005f68263a1a1be06b25156a159d975)
> 
> In both the reports note that there are pool gauges (for tracking the 
> connection pool info) and also a timer for total api calls.
> 
> 
> Thanks,
> 
> Karthik Manamcheri
> 
>

Re: Review Request 66188: HIVE-18986 Table rename will run java.lang.StackOverflowError in dataNucleus if the table contains large number of columns

2018-04-17 Thread Yongzhi Chen via Review Board 

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




standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Lines 7730 (patched)


Should you call addQueryAfterUse and closeAllQueries ? That's how do you 
release the resources held by the batch queries?


- Yongzhi Chen


On March 21, 2018, 6:57 p.m., Aihua Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66188/
> ---
> 
> (Updated March 21, 2018, 6:57 p.m.)
> 
> 
> Review request for hive, Alexander Kolbasov and Yongzhi Chen.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> If the table contains a lot of columns e.g, 5k, simple table rename would 
> fail with the following stack trace. The issue is datanucleus can't handle 
> the query with lots of colName='c1' && colName='c2' && ... .
> 
> I'm breaking the query into multiple smaller queries and then we aggregate 
> the result together.
> 
> 
> Diffs
> -
> 
>   ql/src/test/queries/clientpositive/alter_rename_table.q 2061850540 
>   ql/src/test/results/clientpositive/alter_rename_table.q.out 732d8a28d8 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/Batchable.java
>  PRE-CREATION 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
>  6ead20aeaf 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
>  88d88ed4df 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  9f822564bd 
> 
> 
> Diff: https://reviews.apache.org/r/66188/diff/2/
> 
> 
> Testing
> ---
> 
> Manual test has been done for large column of tables.
> 
> 
> Thanks,
> 
> Aihua Xu
> 
>