Re: Review Request 62228: HIVE-17495: CachedStore: prewarm improvements, refactoring and caching some aggregate stats

2017-12-22 Thread Vaibhav Gumashta 

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

(Updated Dec. 22, 2017, 11:13 p.m.)


Review request for hive, Ashutosh Chauhan, Daniel Dai, and Thejas Nair.


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


Repository: hive-git


Description
---

https://issues.apache.org/jira/browse/HIVE-17495


Diffs (updated)
-

  
itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
 6dc052db45 
  
itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
 0aa1d4e16a 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
 14653b4043 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
 b708fae7ec 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java
 8af96db0bc 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CacheUtils.java
 ab6b90fb6b 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
 9856f8a195 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java
 b606779709 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/BinaryColumnStatsAggregator.java
 45d5d8c984 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/BooleanColumnStatsAggregator.java
 8aac0fe33d 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/ColumnStatsAggregator.java
 cd0392d6c0 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DateColumnStatsAggregator.java
 7f2956152c 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DecimalColumnStatsAggregator.java
 05c0280262 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DoubleColumnStatsAggregator.java
 faf22dcd7c 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java
 d12cdc08ea 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/StringColumnStatsAggregator.java
 4539e6b026 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
 8bc4ce752e 
  
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
 e59e3496bf 
  
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
 8be099cbcb 


Diff: https://reviews.apache.org/r/62228/diff/6/

Changes: https://reviews.apache.org/r/62228/diff/5-6/


Testing
---


Thanks,

Vaibhav Gumashta

Re: Review Request 62228: HIVE-17495: CachedStore: prewarm improvements, refactoring and caching some aggregate stats

2017-12-07 Thread Vaibhav Gumashta 

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

(Updated Dec. 8, 2017, 12:06 a.m.)


Review request for hive, Ashutosh Chauhan, Daniel Dai, and Thejas Nair.


Changes
---

Rebased on master


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


Repository: hive-git


Description
---

https://issues.apache.org/jira/browse/HIVE-17495


Diffs (updated)
-

  
itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
 62c9172ef5 
  
itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
 f344c47443 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
 14653b4043 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
 2e80c9d3b1 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java
 75fbfa23d2 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CacheUtils.java
 ab6b90fb6b 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
 da518ab6e3 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java
 b606779709 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/BinaryColumnStatsAggregator.java
 45d5d8c984 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/BooleanColumnStatsAggregator.java
 8aac0fe33d 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/ColumnStatsAggregator.java
 cd0392d6c0 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DateColumnStatsAggregator.java
 7f2956152c 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DecimalColumnStatsAggregator.java
 05c0280262 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DoubleColumnStatsAggregator.java
 faf22dcd7c 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java
 d12cdc08ea 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/StringColumnStatsAggregator.java
 4539e6b026 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
 cde34bcf42 
  
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
 24c59f2f1b 
  
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
 1e4fe5d973 


Diff: https://reviews.apache.org/r/62228/diff/5/

Changes: https://reviews.apache.org/r/62228/diff/4-5/


Testing
---


Thanks,

Vaibhav Gumashta

Re: Review Request 62228: HIVE-17495: CachedStore: prewarm improvements, refactoring and caching some aggregate stats

2017-12-05 Thread Vaibhav Gumashta 

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

(Updated Dec. 5, 2017, 8:50 a.m.)


Review request for hive, Ashutosh Chauhan, Daniel Dai, and Thejas Nair.


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


Repository: hive-git


Description
---

https://issues.apache.org/jira/browse/HIVE-17495


Diffs (updated)
-

  
itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
 7965ca3e0f 
  
itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
 98dad7a093 
  
metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
 a0a6e18199 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
 14653b4043 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
 fcd06a8cd3 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java
 dcc626c4bb 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CacheUtils.java
 ab6b90fb6b 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
 1f6d900324 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java
 b606779709 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/BinaryColumnStatsAggregator.java
 45d5d8c984 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/BooleanColumnStatsAggregator.java
 8aac0fe33d 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/ColumnStatsAggregator.java
 cd0392d6c0 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DateColumnStatsAggregator.java
 7f2956152c 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DecimalColumnStatsAggregator.java
 05c0280262 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DoubleColumnStatsAggregator.java
 faf22dcd7c 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java
 d12cdc08ea 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/columnstats/aggr/StringColumnStatsAggregator.java
 4539e6b026 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
 beee86f572 
  
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
 6121c9bd64 


Diff: https://reviews.apache.org/r/62228/diff/4/

Changes: https://reviews.apache.org/r/62228/diff/3-4/


Testing
---


Thanks,

Vaibhav Gumashta

Re: Review Request 62228: HIVE-17495: CachedStore: prewarm improvements, refactoring and caching some aggregate stats

2017-09-22 Thread Vaibhav Gumashta 

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

(Updated Sept. 22, 2017, 10:30 p.m.)


Review request for hive, Ashutosh Chauhan and Thejas Nair.


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


Repository: hive-git


Description
---

https://issues.apache.org/jira/browse/HIVE-17495


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1ba5968 
  data/conf/hive-site.xml a205b8c 
  data/conf/llap/hive-site.xml 870b584 
  data/conf/perf-reg/hive-site.xml 012369f 
  data/conf/tez/hive-site.xml 35e8c99 
  
itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
 d94d920 
  
itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
 98dad7a 
  metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
c1a8efe 
  metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 
b51446d 
  metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java b5e4bf0 
  metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java 2bc4d99 
  metastore/src/java/org/apache/hadoop/hive/metastore/cache/CacheUtils.java 
280655d 
  metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java 
7939bfe 
  metastore/src/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java 
80b17e0 
  
metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/BinaryColumnStatsAggregator.java
 e6c836b 
  
metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/BooleanColumnStatsAggregator.java
 a34bc9f 
  
metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/ColumnStatsAggregator.java
 a52e5e5 
  
metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DateColumnStatsAggregator.java
 ee95396 
  
metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DecimalColumnStatsAggregator.java
 284c12c 
  
metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DoubleColumnStatsAggregator.java
 bb4a725 
  
metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java
 5b1145e 
  
metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/StringColumnStatsAggregator.java
 1b29f92 
  
metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
 a75dbb0 
  
metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
 bbb4bf1 


Diff: https://reviews.apache.org/r/62228/diff/3/

Changes: https://reviews.apache.org/r/62228/diff/2-3/


Testing
---


Thanks,

Vaibhav Gumashta

Re: Review Request 62228: HIVE-17495: CachedStore: prewarm improvements, refactoring and caching some aggregate stats

2017-09-14 Thread Vaibhav Gumashta 


> On Sept. 12, 2017, 6:18 a.m., Ashutosh Chauhan wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
> > Lines 275 (patched)
> > 
> >
> > Here we are listing all partitions for table and than we immediately 
> > aggr stats for all partitions. Another (better) way is to not retrieve 
> > partNames and do a sql query to aggr stats for partitions by 
> > partFilterExpr. Essentially get_aggr_stats_for(dbName, tblName, 
> > partFilterExpr).
> > Here, partFilterExpr = * 
> > That will allow many roundtrips to backend DB.

I'll create a follow-up jira for this (near term item) if you're ok with it.


> On Sept. 12, 2017, 6:18 a.m., Ashutosh Chauhan wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
> > Lines 295 (patched)
> > 
> >
> > And here it will be partFilterExpr = partNames not in (defaultPartition)

I'll create a follow-up jira for this (near term item) if you're ok with it.


> On Sept. 12, 2017, 6:18 a.m., Ashutosh Chauhan wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
> > Line 1536 (original), 1570 (patched)
> > 
> >
> > This if condition will always be true for cachedstore prewarm 
> > invocation.
> > can you please add comments for that.

Modified prewarm to use the mergeColStatsForPartitions call directly which 
would also avoid the extra cache lookup we were doing in prewarm.


- Vaibhav


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


On Sept. 11, 2017, 9:25 p.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62228/
> ---
> 
> (Updated Sept. 11, 2017, 9:25 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and Thejas Nair.
> 
> 
> Bugs: HIVE-17495
> https://issues.apache.org/jira/browse/HIVE-17495
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-17495
> 
> 
> Diffs
> -
> 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
>  8d861e4 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
> dc1245e 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 
> bbe13fd 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 
> 3053dcb 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java 71982a0 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java 
> 3ba81ce 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java 
> 80b17e0 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/BinaryColumnStatsAggregator.java
>  e6c836b 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/BooleanColumnStatsAggregator.java
>  a34bc9f 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/ColumnStatsAggregator.java
>  a52e5e5 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/ColumnStatsAggregatorFactory.java
>  dfae708 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DateColumnStatsAggregator.java
>  ee95396 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DecimalColumnStatsAggregator.java
>  284c12c 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DoubleColumnStatsAggregator.java
>  bb4a725 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java
>  5b1145e 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/StringColumnStatsAggregator.java
>  1b29f92 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
>  4db203d 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
>  fb16cfc 
> 
> 
> Diff: https://reviews.apache.org/r/62228/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Re: Review Request 62228: HIVE-17495: CachedStore: prewarm improvements, refactoring and caching some aggregate stats

2017-09-14 Thread Vaibhav Gumashta 

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

(Updated Sept. 14, 2017, 9:03 p.m.)


Review request for hive, Ashutosh Chauhan and Thejas Nair.


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


Repository: hive-git


Description
---

https://issues.apache.org/jira/browse/HIVE-17495


Diffs (updated)
-

  
itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
 8d861e4 
  metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
dc1245e 
  metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 
bbe13fd 
  metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 3053dcb 
  metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java 71982a0 
  metastore/src/java/org/apache/hadoop/hive/metastore/cache/CacheUtils.java 
280655d 
  metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java 
3ba81ce 
  metastore/src/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java 
80b17e0 
  
metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/BinaryColumnStatsAggregator.java
 e6c836b 
  
metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/BooleanColumnStatsAggregator.java
 a34bc9f 
  
metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/ColumnStatsAggregator.java
 a52e5e5 
  
metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DateColumnStatsAggregator.java
 ee95396 
  
metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DecimalColumnStatsAggregator.java
 284c12c 
  
metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DoubleColumnStatsAggregator.java
 bb4a725 
  
metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java
 5b1145e 
  
metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/StringColumnStatsAggregator.java
 1b29f92 
  
metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
 4db203d 
  
metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
 fb16cfc 


Diff: https://reviews.apache.org/r/62228/diff/2/

Changes: https://reviews.apache.org/r/62228/diff/1-2/


Testing
---


Thanks,

Vaibhav Gumashta

Re: Review Request 62228: HIVE-17495: CachedStore: prewarm improvements, refactoring and caching some aggregate stats

2017-09-14 Thread Vaibhav Gumashta 


> On Sept. 12, 2017, 7:46 p.m., Daniel Dai wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
> > Line 258 (original), 264 (patched)
> > 
> >
> > If we want to reduce the num of sql statement, shall we do the same 
> > single fetch to all table statistics for symmetry?

Will open a follow up ticket for this.


- Vaibhav


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


On Sept. 11, 2017, 9:25 p.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62228/
> ---
> 
> (Updated Sept. 11, 2017, 9:25 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and Thejas Nair.
> 
> 
> Bugs: HIVE-17495
> https://issues.apache.org/jira/browse/HIVE-17495
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-17495
> 
> 
> Diffs
> -
> 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
>  8d861e4 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
> dc1245e 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 
> bbe13fd 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 
> 3053dcb 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java 71982a0 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java 
> 3ba81ce 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java 
> 80b17e0 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/BinaryColumnStatsAggregator.java
>  e6c836b 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/BooleanColumnStatsAggregator.java
>  a34bc9f 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/ColumnStatsAggregator.java
>  a52e5e5 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/ColumnStatsAggregatorFactory.java
>  dfae708 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DateColumnStatsAggregator.java
>  ee95396 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DecimalColumnStatsAggregator.java
>  284c12c 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DoubleColumnStatsAggregator.java
>  bb4a725 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java
>  5b1145e 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/StringColumnStatsAggregator.java
>  1b29f92 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
>  4db203d 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
>  fb16cfc 
> 
> 
> Diff: https://reviews.apache.org/r/62228/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Re: Review Request 62228: HIVE-17495: CachedStore: prewarm improvements, refactoring and caching some aggregate stats

2017-09-13 Thread Ashutosh Chauhan 

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



Also, I don't see any logic for cache invalidation in case partition is added 
or deleted. Did I miss that?

- Ashutosh Chauhan


On Sept. 11, 2017, 9:25 p.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62228/
> ---
> 
> (Updated Sept. 11, 2017, 9:25 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and Thejas Nair.
> 
> 
> Bugs: HIVE-17495
> https://issues.apache.org/jira/browse/HIVE-17495
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-17495
> 
> 
> Diffs
> -
> 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
>  8d861e4 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
> dc1245e 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 
> bbe13fd 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 
> 3053dcb 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java 71982a0 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java 
> 3ba81ce 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java 
> 80b17e0 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/BinaryColumnStatsAggregator.java
>  e6c836b 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/BooleanColumnStatsAggregator.java
>  a34bc9f 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/ColumnStatsAggregator.java
>  a52e5e5 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/ColumnStatsAggregatorFactory.java
>  dfae708 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DateColumnStatsAggregator.java
>  ee95396 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DecimalColumnStatsAggregator.java
>  284c12c 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DoubleColumnStatsAggregator.java
>  bb4a725 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java
>  5b1145e 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/StringColumnStatsAggregator.java
>  1b29f92 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
>  4db203d 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
>  fb16cfc 
> 
> 
> Diff: https://reviews.apache.org/r/62228/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Re: Review Request 62228: HIVE-17495: CachedStore: prewarm improvements, refactoring and caching some aggregate stats

2017-09-12 Thread Daniel Dai 

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




metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
Line 258 (original), 264 (patched)


If we want to reduce the num of sql statement, shall we do the same single 
fetch to all table statistics for symmetry?



metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
Lines 295 (patched)


It should be better to get both all/allbutdefault statistics from 
mergeColStatsForPartitions, rather than invoke get_aggr_stats_for twice, that 
would be colstly.


- Daniel Dai


On Sept. 11, 2017, 9:25 p.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62228/
> ---
> 
> (Updated Sept. 11, 2017, 9:25 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and Thejas Nair.
> 
> 
> Bugs: HIVE-17495
> https://issues.apache.org/jira/browse/HIVE-17495
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-17495
> 
> 
> Diffs
> -
> 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
>  8d861e4 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
> dc1245e 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 
> bbe13fd 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 
> 3053dcb 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java 71982a0 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java 
> 3ba81ce 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java 
> 80b17e0 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/BinaryColumnStatsAggregator.java
>  e6c836b 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/BooleanColumnStatsAggregator.java
>  a34bc9f 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/ColumnStatsAggregator.java
>  a52e5e5 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/ColumnStatsAggregatorFactory.java
>  dfae708 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DateColumnStatsAggregator.java
>  ee95396 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DecimalColumnStatsAggregator.java
>  284c12c 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DoubleColumnStatsAggregator.java
>  bb4a725 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java
>  5b1145e 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/StringColumnStatsAggregator.java
>  1b29f92 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
>  4db203d 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
>  fb16cfc 
> 
> 
> Diff: https://reviews.apache.org/r/62228/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Re: Review Request 62228: HIVE-17495: CachedStore: prewarm improvements, refactoring and caching some aggregate stats

2017-09-12 Thread Ashutosh Chauhan 

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




metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
Lines 1432-1433 (original)


It will be useful to retain (improved) comment.



metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java
Line 1977 (original), 1977 (patched)


Better comment: Group stats by colName for each partition



metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java
Lines 1993 (patched)


LOG.debug



metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java
Line 1999 (original), 2007 (patched)


for number of threads, better logic could be
Math.min(colStatsMap.size(), Runtime.getRuntime().availableProcessors())



metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java
Line 2008 (original), 2010 (patched)


LOG.debug



metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java
Lines 2024 (patched)


Can remove this



metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java
Lines 2025 (patched)


LOG.debug(e.getMessage())



metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java
Line 2025 (original), 2035 (patched)


future will never be null



metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java
Line 2027 (original), 2039 (patched)


Better to keep pool.shutdownNow()
and remove e.printsTacktrace()



metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java
Lines 2045 (patched)


LOG.debug



metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
Lines 275 (patched)


Here we are listing all partitions for table and than we immediately aggr 
stats for all partitions. Another (better) way is to not retrieve partNames and 
do a sql query to aggr stats for partitions by partFilterExpr. Essentially 
get_aggr_stats_for(dbName, tblName, partFilterExpr).
Here, partFilterExpr = * 
That will allow many roundtrips to backend DB.



metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
Lines 295 (patched)


And here it will be partFilterExpr = partNames not in (defaultPartition)



metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
Line 267 (original), 301 (patched)


Useful to log time taken to prewarm:
 LOG.info("Time taken to prewarm: " + 
(System.currentTimeMillis()-start)/1000);



metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
Line 1535 (original), 1569 (patched)


Caller from CachedStore made this call before calling this method. Might as 
well pass from there.



metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
Line 1536 (original), 1570 (patched)


This if condition will always be true for cachedstore prewarm invocation.
can you please add comments for that.



metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
Lines 1588 (patched)


LOG.debug



metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
Line 1563 (original), 1623 (patched)


LOG.debug and
Previous construction of msg was better.



metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
Lines 1630 (patched)


should this be < 1 instead of <=1 ?



metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
Lines 1631 (patched)


LOG.debug with {} instead of +



metastore/src/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java
Lines 461 (patched)


{} instead of +



metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/ColumnStatsAggregatorFactory.java
Lines 44 (patched)


LOG.trace

Re: Review Request 62228: HIVE-17495: CachedStore: prewarm improvements, refactoring and caching some aggregate stats

2017-09-11 Thread Sergey Shelukhin 

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




metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
Lines 1513 (patched)


try... finally for closeAll?



metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java
Lines 2137 (patched)


nit: final?


- Sergey Shelukhin


On Sept. 11, 2017, 9:25 p.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62228/
> ---
> 
> (Updated Sept. 11, 2017, 9:25 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and Thejas Nair.
> 
> 
> Bugs: HIVE-17495
> https://issues.apache.org/jira/browse/HIVE-17495
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-17495
> 
> 
> Diffs
> -
> 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
>  8d861e4 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
> dc1245e 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 
> bbe13fd 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 
> 3053dcb 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java 71982a0 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java 
> 3ba81ce 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java 
> 80b17e0 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/BinaryColumnStatsAggregator.java
>  e6c836b 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/BooleanColumnStatsAggregator.java
>  a34bc9f 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/ColumnStatsAggregator.java
>  a52e5e5 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/ColumnStatsAggregatorFactory.java
>  dfae708 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DateColumnStatsAggregator.java
>  ee95396 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DecimalColumnStatsAggregator.java
>  284c12c 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DoubleColumnStatsAggregator.java
>  bb4a725 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java
>  5b1145e 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/StringColumnStatsAggregator.java
>  1b29f92 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
>  4db203d 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
>  fb16cfc 
> 
> 
> Diff: https://reviews.apache.org/r/62228/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Review Request 62228: HIVE-17495: CachedStore: prewarm improvements, refactoring and caching some aggregate stats

2017-09-11 Thread Vaibhav Gumashta 

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

Review request for hive, Ashutosh Chauhan and Thejas Nair.


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


Repository: hive-git


Description
---

https://issues.apache.org/jira/browse/HIVE-17495


Diffs
-

  
itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
 8d861e4 
  metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
dc1245e 
  metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 
bbe13fd 
  metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 3053dcb 
  metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java 71982a0 
  metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java 
3ba81ce 
  metastore/src/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java 
80b17e0 
  
metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/BinaryColumnStatsAggregator.java
 e6c836b 
  
metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/BooleanColumnStatsAggregator.java
 a34bc9f 
  
metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/ColumnStatsAggregator.java
 a52e5e5 
  
metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/ColumnStatsAggregatorFactory.java
 dfae708 
  
metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DateColumnStatsAggregator.java
 ee95396 
  
metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DecimalColumnStatsAggregator.java
 284c12c 
  
metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/DoubleColumnStatsAggregator.java
 bb4a725 
  
metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/LongColumnStatsAggregator.java
 5b1145e 
  
metastore/src/java/org/apache/hadoop/hive/metastore/columnstats/aggr/StringColumnStatsAggregator.java
 1b29f92 
  
metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
 4db203d 
  
metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
 fb16cfc 


Diff: https://reviews.apache.org/r/62228/diff/1/


Testing
---


Thanks,

Vaibhav Gumashta