You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Vihang Karajgaonkar via Review Board <no...@reviews.apache.org> on 2018/06/04 18:21:51 UTC

Re: Review Request 67351: HIVE-19718 Adding partitions in bulk also fetches table for each partition

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




standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Lines 2411-2416 (original)
<https://reviews.apache.org/r/67351/#comment286661>

    Why do we need to remove these lines?



standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Line 2422 (original)
<https://reviews.apache.org/r/67351/#comment286662>

    Why do we need to remove this line?



standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Lines 2426 (patched)
<https://reviews.apache.org/r/67351/#comment286663>

    Adding a javadoc would be great. esp. mentioning that the advantage of using this method and when its better to use it.


- Vihang Karajgaonkar


On May 29, 2018, 10:53 a.m., Peter Vary wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67351/
> -----------------------------------------------------------
> 
> (Updated May 29, 2018, 10:53 a.m.)
> 
> 
> Review request for hive, Alexander Kolbasov and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-19718
>     https://issues.apache.org/jira/browse/HIVE-19718
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Various optimization for addPartitions call:
> - Push down table object to convertToMPart
> - Push down partitionKeys to startAddPartition -> doesPartitionExist -> getMPartition, so it does not have to query the table object for every time if we add multiple partitions for the same table
> - The original getMPartition used to query the table every time. Created a new version of getMPartition, which can use the provided partitionKeys instead of querying it again.
> 
> 
> Diffs
> -----
> 
>   itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java 3d6fda6 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java c1d25db 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java 13ccdb1 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java ce7d286 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java b223920 
>   standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java f6899be 
>   standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java 98a85cc 
> 
> 
> Diff: https://reviews.apache.org/r/67351/diff/1/
> 
> 
> Testing
> -------
> 
> Run several performance tests with Sasha's performance tool. These optimisations shave of ~10% of the runtime
> 
> 
> Thanks,
> 
> Peter Vary
> 
>


Re: Review Request 67351: HIVE-19718 Adding partitions in bulk also fetches table for each partition

Posted by Peter Vary via Review Board <no...@reviews.apache.org>.

> On June 4, 2018, 6:21 p.m., Vihang Karajgaonkar wrote:
> > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
> > Lines 2411-2416 (original)
> > <https://reviews.apache.org/r/67351/diff/1/?file=2031431#file2031431line2412>
> >
> >     Why do we need to remove these lines?

Good point.
Added openTransaction back.
Thanks for noticing


> On June 4, 2018, 6:21 p.m., Vihang Karajgaonkar wrote:
> > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
> > Line 2422 (original)
> > <https://reviews.apache.org/r/67351/diff/1/?file=2031431#file2031431line2423>
> >
> >     Why do we need to remove this line?

Good point.
Added commitTransaction back.
Thanks for noticing


> On June 4, 2018, 6:21 p.m., Vihang Karajgaonkar wrote:
> > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
> > Lines 2426 (patched)
> > <https://reviews.apache.org/r/67351/diff/1/?file=2031431#file2031431line2433>
> >
> >     Adding a javadoc would be great. esp. mentioning that the advantage of using this method and when its better to use it.

Added javadoc


- Peter


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


On June 5, 2018, 7:54 a.m., Peter Vary wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67351/
> -----------------------------------------------------------
> 
> (Updated June 5, 2018, 7:54 a.m.)
> 
> 
> Review request for hive, Alexander Kolbasov and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-19718
>     https://issues.apache.org/jira/browse/HIVE-19718
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Various optimization for addPartitions call:
> - Push down table object to convertToMPart
> - Push down partitionKeys to startAddPartition -> doesPartitionExist -> getMPartition, so it does not have to query the table object for every time if we add multiple partitions for the same table
> - The original getMPartition used to query the table every time. Created a new version of getMPartition, which can use the provided partitionKeys instead of querying it again.
> 
> 
> Diffs
> -----
> 
>   itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java 0cc0ae5 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java d8b8414 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java b15d89d 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java 283798c 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java 9da8d72 
>   standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java 0461c4e 
>   standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java b71eda4 
> 
> 
> Diff: https://reviews.apache.org/r/67351/diff/2/
> 
> 
> Testing
> -------
> 
> Run several performance tests with Sasha's performance tool. These optimisations shave of ~10% of the runtime
> 
> 
> Thanks,
> 
> Peter Vary
> 
>