You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Nachiket Vaidya <nv...@cloudera.com> on 2017/01/30 20:51:09 UTC

Review Request 56095: HIVE-15754 exchange partition is not generating notifications

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

Review request for hive and Mohit Sabharwal.


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


Repository: hive-git


Description
-------

HIVE-15754 exchange partition is not generating notifications

exchange partition event is not generating notifications in notification_log.
There should multiple events generated. one add_partition event and several drop_partition events.
for example:
ALTER TABLE tab1 EXCHANGE PARTITION (part=1) WITH TABLE tab2;

There should be the following events:
ADD_PARTITION on tab2 on partition (part=1)
DROP_PARTITION on tab1 on partition (part=1)


Diffs
-----

  itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java 640b567 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 13d0aab 

Diff: https://reviews.apache.org/r/56095/diff/


Testing
-------

o Added unit test cases.
o Ran exchange partition queries and saw entired in notification_log.


Thanks,

Nachiket Vaidya


Re: Review Request 56095: HIVE-15754 exchange partition is not generating notifications

Posted by Nachiket Vaidya <nv...@cloudera.com>.

> On Feb. 8, 2017, 5:39 p.m., Vihang Karajgaonkar wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, line 2981
> > <https://reviews.apache.org/r/56095/diff/1/?file=1619268#file1619268line2981>
> >
> >     Does it need to fire exchangePartitionEvent here when success is true as well? If you look at the add_partitions_core method it creates a addPartition event for transaction listeners and also for listeners in the finally clause when success is true. Is there a reason this is different?

actually if you check the code for , we call fireMetaStoreAddPartitionEvent() for success as well as failure case. 

        if (!success) {
          ms.rollbackTransaction();
          for (Map.Entry<PartValEqWrapper, Boolean> e : addedPartitions.entrySet()) {
            if (e.getValue()) {
              wh.deleteDir(new Path(e.getKey().partition.getSd().getLocation()), true);
              // we just created this directory - it's not a case of pre-creation, so we nuke
            }
          }
          fireMetaStoreAddPartitionEvent(tbl, parts, null, false);
        } else {
          fireMetaStoreAddPartitionEvent(tbl, newParts, null, true);
          if (existingParts != null) {
            // The request has succeeded but we failed to add these partitions.
            fireMetaStoreAddPartitionEvent(tbl, existingParts, null, false);
          }
        }
      }

In case of exchange partition, we do not check if partition is existing or not. The operation will fail if there are existing partitions. For add partition operation, operation is successful even if partitions exist. That's the reason why we need a single call with result as sucsess or !success.


> On Feb. 8, 2017, 5:39 p.m., Vihang Karajgaonkar wrote:
> > itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java, line 586
> > <https://reviews.apache.org/r/56095/diff/1/?file=1619267#file1619267line586>
> >
> >     Can you please refactor out the repeating statements into util methods to improve readability of this test case?

I just followed what is there in this file. Unfortunately, the code is already committed to master.


- Nachiket


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


On Feb. 7, 2017, 9:32 p.m., Nachiket Vaidya wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56095/
> -----------------------------------------------------------
> 
> (Updated Feb. 7, 2017, 9:32 p.m.)
> 
> 
> Review request for hive, Mohit Sabharwal and Sergio Pena.
> 
> 
> Bugs: HIVE-15754
>     https://issues.apache.org/jira/browse/HIVE-15754
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-15754 exchange partition is not generating notifications
> 
> exchange partition event is not generating notifications in notification_log.
> There should multiple events generated. one add_partition event and several drop_partition events.
> for example:
> ALTER TABLE tab1 EXCHANGE PARTITION (part=1) WITH TABLE tab2;
> 
> There should be the following events:
> ADD_PARTITION on tab2 on partition (part=1)
> DROP_PARTITION on tab1 on partition (part=1)
> 
> 
> Diffs
> -----
> 
>   itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java 640b567ebf9af1af5a3dd9fa1442eb5c5b8ef1a4 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 13d0aab40eaf344869b8230614b55115f64857e6 
> 
> Diff: https://reviews.apache.org/r/56095/diff/
> 
> 
> Testing
> -------
> 
> o Added unit test cases.
> o Ran exchange partition queries and saw entired in notification_log.
> 
> 
> Thanks,
> 
> Nachiket Vaidya
> 
>


Re: Review Request 56095: HIVE-15754 exchange partition is not generating notifications

Posted by Vihang Karajgaonkar <vi...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56095/#review164707
-----------------------------------------------------------




itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java (line 586)
<https://reviews.apache.org/r/56095/#comment236467>

    Can you please refactor out the repeating statements into util methods to improve readability of this test case?



metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java (line 2980)
<https://reviews.apache.org/r/56095/#comment236466>

    Does it need to fire exchangePartitionEvent here when success is true as well? If you look at the add_partitions_core method it creates a addPartition event for transaction listeners and also for listeners in the finally clause when success is true. Is there a reason this is different?


- Vihang Karajgaonkar


On Feb. 7, 2017, 9:32 p.m., Nachiket Vaidya wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56095/
> -----------------------------------------------------------
> 
> (Updated Feb. 7, 2017, 9:32 p.m.)
> 
> 
> Review request for hive, Mohit Sabharwal and Sergio Pena.
> 
> 
> Bugs: HIVE-15754
>     https://issues.apache.org/jira/browse/HIVE-15754
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-15754 exchange partition is not generating notifications
> 
> exchange partition event is not generating notifications in notification_log.
> There should multiple events generated. one add_partition event and several drop_partition events.
> for example:
> ALTER TABLE tab1 EXCHANGE PARTITION (part=1) WITH TABLE tab2;
> 
> There should be the following events:
> ADD_PARTITION on tab2 on partition (part=1)
> DROP_PARTITION on tab1 on partition (part=1)
> 
> 
> Diffs
> -----
> 
>   itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java 640b567ebf9af1af5a3dd9fa1442eb5c5b8ef1a4 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 13d0aab40eaf344869b8230614b55115f64857e6 
> 
> Diff: https://reviews.apache.org/r/56095/diff/
> 
> 
> Testing
> -------
> 
> o Added unit test cases.
> o Ran exchange partition queries and saw entired in notification_log.
> 
> 
> Thanks,
> 
> Nachiket Vaidya
> 
>


Re: Review Request 56095: HIVE-15754 exchange partition is not generating notifications

Posted by Sergio Pena <se...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56095/#review164564
-----------------------------------------------------------


Ship it!




Ship It!

- Sergio Pena


On Feb. 7, 2017, 9:32 p.m., Nachiket Vaidya wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56095/
> -----------------------------------------------------------
> 
> (Updated Feb. 7, 2017, 9:32 p.m.)
> 
> 
> Review request for hive, Mohit Sabharwal and Sergio Pena.
> 
> 
> Bugs: HIVE-15754
>     https://issues.apache.org/jira/browse/HIVE-15754
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-15754 exchange partition is not generating notifications
> 
> exchange partition event is not generating notifications in notification_log.
> There should multiple events generated. one add_partition event and several drop_partition events.
> for example:
> ALTER TABLE tab1 EXCHANGE PARTITION (part=1) WITH TABLE tab2;
> 
> There should be the following events:
> ADD_PARTITION on tab2 on partition (part=1)
> DROP_PARTITION on tab1 on partition (part=1)
> 
> 
> Diffs
> -----
> 
>   itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java 640b567ebf9af1af5a3dd9fa1442eb5c5b8ef1a4 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 13d0aab40eaf344869b8230614b55115f64857e6 
> 
> Diff: https://reviews.apache.org/r/56095/diff/
> 
> 
> Testing
> -------
> 
> o Added unit test cases.
> o Ran exchange partition queries and saw entired in notification_log.
> 
> 
> Thanks,
> 
> Nachiket Vaidya
> 
>


Re: Review Request 56095: HIVE-15754 exchange partition is not generating notifications

Posted by Nachiket Vaidya <nv...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56095/
-----------------------------------------------------------

(Updated Feb. 7, 2017, 9:32 p.m.)


Review request for hive, Mohit Sabharwal and Sergio Pena.


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


Repository: hive-git


Description
-------

HIVE-15754 exchange partition is not generating notifications

exchange partition event is not generating notifications in notification_log.
There should multiple events generated. one add_partition event and several drop_partition events.
for example:
ALTER TABLE tab1 EXCHANGE PARTITION (part=1) WITH TABLE tab2;

There should be the following events:
ADD_PARTITION on tab2 on partition (part=1)
DROP_PARTITION on tab1 on partition (part=1)


Diffs
-----

  itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java 640b567ebf9af1af5a3dd9fa1442eb5c5b8ef1a4 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 13d0aab40eaf344869b8230614b55115f64857e6 

Diff: https://reviews.apache.org/r/56095/diff/


Testing
-------

o Added unit test cases.
o Ran exchange partition queries and saw entired in notification_log.


Thanks,

Nachiket Vaidya


Re: Review Request 56095: HIVE-15754 exchange partition is not generating notifications

Posted by Nachiket Vaidya <nv...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56095/
-----------------------------------------------------------

(Updated Feb. 7, 2017, 9:32 p.m.)


Review request for sentry and Mohit Sabharwal.


Changes
-------

o + Sergio's comments.


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


Repository: hive-git


Description
-------

HIVE-15754 exchange partition is not generating notifications

exchange partition event is not generating notifications in notification_log.
There should multiple events generated. one add_partition event and several drop_partition events.
for example:
ALTER TABLE tab1 EXCHANGE PARTITION (part=1) WITH TABLE tab2;

There should be the following events:
ADD_PARTITION on tab2 on partition (part=1)
DROP_PARTITION on tab1 on partition (part=1)


Diffs (updated)
-----

  itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java 640b567ebf9af1af5a3dd9fa1442eb5c5b8ef1a4 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 13d0aab40eaf344869b8230614b55115f64857e6 

Diff: https://reviews.apache.org/r/56095/diff/


Testing
-------

o Added unit test cases.
o Ran exchange partition queries and saw entired in notification_log.


Thanks,

Nachiket Vaidya


Re: Review Request 56095: HIVE-15754 exchange partition is not generating notifications

Posted by Sergio Pena <se...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56095/#review164553
-----------------------------------------------------------



Thanks Nachiket, the patch looks good. Just a minior change to check for null and empty values and this would good to go.


metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java (line 2991)
<https://reviews.apache.org/r/56095/#comment236322>

    Should we check table and partitions for null and empty values? I see this condition in other methods that fire add partition events.


- Sergio Pena


On Jan. 30, 2017, 8:51 p.m., Nachiket Vaidya wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56095/
> -----------------------------------------------------------
> 
> (Updated Jan. 30, 2017, 8:51 p.m.)
> 
> 
> Review request for hive and Mohit Sabharwal.
> 
> 
> Bugs: HIVE-15754
>     https://issues.apache.org/jira/browse/HIVE-15754
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-15754 exchange partition is not generating notifications
> 
> exchange partition event is not generating notifications in notification_log.
> There should multiple events generated. one add_partition event and several drop_partition events.
> for example:
> ALTER TABLE tab1 EXCHANGE PARTITION (part=1) WITH TABLE tab2;
> 
> There should be the following events:
> ADD_PARTITION on tab2 on partition (part=1)
> DROP_PARTITION on tab1 on partition (part=1)
> 
> 
> Diffs
> -----
> 
>   itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java 640b567 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 13d0aab 
> 
> Diff: https://reviews.apache.org/r/56095/diff/
> 
> 
> Testing
> -------
> 
> o Added unit test cases.
> o Ran exchange partition queries and saw entired in notification_log.
> 
> 
> Thanks,
> 
> Nachiket Vaidya
> 
>