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




standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Line 2417 (original), 2428 (patched)
<https://reviews.apache.org/r/67351/#comment286787>

    Do we need a version of this that can be called when we already have the 
MTable object?



standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Line 2428 (original), 2434 (patched)
<https://reviews.apache.org/r/67351/#comment286783>

    Note that this may throw an exception which you need to catch and rollback 
your transaction. SO immediately after openTransaction you need a try-finally 
block.



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

    missing commitTransaction()



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

    Strictly speaking we do not need openTransaction() inside try block



standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Line 2446 (original), 2477 (patched)
<https://reviews.apache.org/r/67351/#comment286791>

    The rest can be done outside of try block - we already committed the 
transaction.



standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Line 2449 (original), 2480 (patched)
<https://reviews.apache.org/r/67351/#comment286794>

    Can we do this with setUnique(true) instead?



standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Line 2456 (original), 2487 (patched)
<https://reviews.apache.org/r/67351/#comment286796>

    Can we just return mpart here?



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

    Can you document this behavior?



standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Line 2487 (original), 2538 (patched)
<https://reviews.apache.org/r/67351/#comment286799>

    Can you document this behavior?



standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Line 9120 (original), 9171 (patched)
<https://reviews.apache.org/r/67351/#comment286800>

    I'm wondering whether we can make things faster a bit by using the fact 
that we only need to know whether partition exists and don't care about 
fetching any actual values from partitions - we can have a potentially faster 
query that just checks for partition existence.



standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java
Line 331 (original), 331 (patched)
<https://reviews.apache.org/r/67351/#comment286779>

    Please update Javadoc as well


- Alexander Kolbasov


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
> 
>

Reply via email to