hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "jiraposter@reviews.apache.org (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HIVE-2215) Add api for marking / querying set of partitions for events
Date Tue, 14 Jun 2011 01:02:49 GMT

    [ https://issues.apache.org/jira/browse/HIVE-2215?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13048925#comment-13048925
] 

jiraposter@reviews.apache.org commented on HIVE-2215:
-----------------------------------------------------


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



trunk/metastore/if/hive_metastore.thrift
<https://reviews.apache.org/r/883/#comment1784>

    I think this should be changed to "PartitionEventType" in order to make it clear that
this applies to partitions only. If in the future we need to introduce event types for tables,
indexes, etc, then we should add new enums for those event types as well.



trunk/metastore/if/hive_metastore.thrift
<https://reviews.apache.org/r/883/#comment1785>

    This should also throw UnknownDBException and UnknownTableException. The same goes for
isPartitionMarkedForEvent.



trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/883/#comment1786>

    Collections aren't required to satisfy an ordering property, so we have to assume the
output of this logging statement is ambiguous, e.g. "[a, b]" versus "[b, a]". We should disambiguate
this by passing in the part_vals map and logging the key/value pairs instead of just the values.



trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/883/#comment1789>

    Missing exceptions: UnknownDbException and UnknownTableException.



trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/883/#comment1787>

    Checking to see if the DB and Table exist should be done in the same database transaction
as the rest of the operation. If you do it here there's no guarantee that the db/table will
still exist when ms.markPartitionForEvent() is called.



trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/883/#comment1791>

    Should we add an InvalidPartitionException and UnknownPartitionException? Seems like those
are both valid exceptions in this situation.



trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/883/#comment1788>

    Same issue here as before. These checks need to get pushed into ms.isPartitionMarkedForEvent().



trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreEventListener.java
<https://reviews.apache.org/r/883/#comment1798>

    I think the name of this method is misleading. You're marking a single partition done,
not a set of partitions, right? 
    
    Also, in this context being "done" means that the load operation on that partition has
completed, so it would be good to include "load" in the name of the method and event class,
e.g. "LoadPartitionDoneEvent" and "onLoadPartitionDone".
    



trunk/metastore/src/model/org/apache/hadoop/hive/metastore/model/MPartitionEvent.java
<https://reviews.apache.org/r/883/#comment1800>

    Is it possible to use org.apache.hadoop.hive.metastore.api.EventType instead of int?
    
    Another approach is to create an MPartitionEvent baseclass, and then subclass that with
MPartitionLoadDoneEvent, etc, and use eventType as the internal type discriminator for JDO.



trunk/metastore/src/model/package.jdo
<https://reviews.apache.org/r/883/#comment1793>

    You need to supply schema upgrade scripts for Derby and MySQL. Please either do that in
this ticket or open a followup ticket and assign it to yourself.



trunk/metastore/src/model/package.jdo
<https://reviews.apache.org/r/883/#comment1799>

    It looks like it's possible for this table to hold more than one "MarkPartitionDone" event
for the same partition, but is that a legal state? If it is, how do you know when the load
operation for a partition is still in progress?



trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMarkPartitionSet.java
<https://reviews.apache.org/r/883/#comment1801>

    Can you subclass this with a remote and embedded version?



trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java
<https://reviews.apache.org/r/883/#comment1797>

    Any reason in particular why you switched to always running this test in local mode? If
we can only test one scenario, then I think there's more value in focusing on the standalone
client/server setup.


- Carl


On 2011-06-10 21:24:13, Ashutosh Chauhan wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/883/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-06-10 21:24:13)
bq.  
bq.  
bq.  Review request for hive and John Sichi.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  Follow-up for HIVE-2147.
bq.  
bq.  
bq.  This addresses bug HIVE-2215.
bq.      https://issues.apache.org/jira/browse/HIVE-2215
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    trunk/metastore/if/hive_metastore.thrift 1134443 
bq.    trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 1134443

bq.    trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
1134443 
bq.    trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 1134443

bq.    trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreEventListener.java
1134443 
bq.    trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 1134443

bq.    trunk/metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java 1134443 
bq.    trunk/metastore/src/java/org/apache/hadoop/hive/metastore/events/MarkPartitionEvent.java
PRE-CREATION 
bq.    trunk/metastore/src/model/org/apache/hadoop/hive/metastore/model/MPartitionEvent.java
PRE-CREATION 
bq.    trunk/metastore/src/model/package.jdo 1134443 
bq.    trunk/metastore/src/test/org/apache/hadoop/hive/metastore/DummyListener.java 1134443

bq.    trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMarkPartitionSet.java
PRE-CREATION 
bq.    trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java
1134443 
bq.  
bq.  Diff: https://reviews.apache.org/r/883/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  Added test cases for new api.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Ashutosh
bq.  
bq.



> Add api for marking / querying set of partitions for events
> -----------------------------------------------------------
>
>                 Key: HIVE-2215
>                 URL: https://issues.apache.org/jira/browse/HIVE-2215
>             Project: Hive
>          Issue Type: New Feature
>          Components: Metastore
>    Affects Versions: 0.8.0
>            Reporter: Ashutosh Chauhan
>            Assignee: Ashutosh Chauhan
>             Fix For: 0.8.0
>
>         Attachments: hive_2215.patch
>
>


--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Mime
View raw message