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 20:54:51 GMT

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

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



bq.  On 2011-06-14 01:02:20, Carl Steinbach wrote:
bq.  > trunk/metastore/if/hive_metastore.thrift, line 46
bq.  > <https://reviews.apache.org/r/883/diff/1/?file=20969#file20969line46>
bq.  >
bq.  >     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.

Done.


bq.  On 2011-06-14 01:02:20, Carl Steinbach wrote:
bq.  > trunk/metastore/if/hive_metastore.thrift, line 338
bq.  > <https://reviews.apache.org/r/883/diff/1/?file=20969#file20969line338>
bq.  >
bq.  >     This should also throw UnknownDBException and UnknownTableException. The same
goes for isPartitionMarkedForEvent.

Done.


bq.  On 2011-06-14 01:02:20, Carl Steinbach wrote:
bq.  > trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, line
528
bq.  > <https://reviews.apache.org/r/883/diff/1/?file=20970#file20970line528>
bq.  >
bq.  >     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.

Done.


bq.  On 2011-06-14 01:02:20, Carl Steinbach wrote:
bq.  > trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, line
3182
bq.  > <https://reviews.apache.org/r/883/diff/1/?file=20970#file20970line3182>
bq.  >
bq.  >     Missing exceptions: UnknownDbException and UnknownTableException.

Done.


bq.  On 2011-06-14 01:02:20, Carl Steinbach wrote:
bq.  > trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, line
3186
bq.  > <https://reviews.apache.org/r/883/diff/1/?file=20970#file20970line3186>
bq.  >
bq.  >     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.

Done.


bq.  On 2011-06-14 01:02:20, Carl Steinbach wrote:
bq.  > trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, line
3188
bq.  > <https://reviews.apache.org/r/883/diff/1/?file=20970#file20970line3188>
bq.  >
bq.  >     Should we add an InvalidPartitionException and UnknownPartitionException? Seems
like those are both valid exceptions in this situation.

Done.


bq.  On 2011-06-14 01:02:20, Carl Steinbach wrote:
bq.  > trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, line
3224
bq.  > <https://reviews.apache.org/r/883/diff/1/?file=20970#file20970line3224>
bq.  >
bq.  >     Same issue here as before. These checks need to get pushed into ms.isPartitionMarkedForEvent().

Done.


bq.  On 2011-06-14 01:02:20, Carl Steinbach wrote:
bq.  > trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreEventListener.java,
line 82
bq.  > <https://reviews.apache.org/r/883/diff/1/?file=20973#file20973line82>
bq.  >
bq.  >     I think the name of this method is misleading. You're marking a single partition
done, not a set of partitions, right? 
bq.  >     
bq.  >     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".
bq.  >

Done.


bq.  On 2011-06-14 01:02:20, Carl Steinbach wrote:
bq.  > trunk/metastore/src/model/org/apache/hadoop/hive/metastore/model/MPartitionEvent.java,
line 34
bq.  > <https://reviews.apache.org/r/883/diff/1/?file=20977#file20977line34>
bq.  >
bq.  >     Is it possible to use org.apache.hadoop.hive.metastore.api.EventType instead
of int?
bq.  >     
bq.  >     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.

No.

I don't see any benefit of it. Nor I cant see how will this work.


bq.  On 2011-06-14 01:02:20, Carl Steinbach wrote:
bq.  > trunk/metastore/src/model/package.jdo, line 668
bq.  > <https://reviews.apache.org/r/883/diff/1/?file=20978#file20978line668>
bq.  >
bq.  >     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.

Will do.


bq.  On 2011-06-14 01:02:20, Carl Steinbach wrote:
bq.  > trunk/metastore/src/model/package.jdo, line 683
bq.  > <https://reviews.apache.org/r/883/diff/1/?file=20978#file20978line683>
bq.  >
bq.  >     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?

This is not for when load operation is in progress. As suggested from name its "LoadPartitionDone".
So, marking partition load done is idempotent. Client can mark it multiple times. So, metastore
will return true if it finds one or more such partitioned marked in the table.


bq.  On 2011-06-14 01:02:20, Carl Steinbach wrote:
bq.  > trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMarkPartitionSet.java,
line 36
bq.  > <https://reviews.apache.org/r/883/diff/1/?file=20980#file20980line36>
bq.  >
bq.  >     Can you subclass this with a remote and embedded version?

Done.


bq.  On 2011-06-14 01:02:20, Carl Steinbach wrote:
bq.  > trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java,
line 80
bq.  > <https://reviews.apache.org/r/883/diff/1/?file=20981#file20981line80>
bq.  >
bq.  >     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.

I reverted those changes.


- Ashutosh


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


On 2011-06-14 20:51:53, 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-14 20:51:53)
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 1135779 
bq.    trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 1135779

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

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

bq.    trunk/metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java 1135779 
bq.    trunk/metastore/src/java/org/apache/hadoop/hive/metastore/events/LoadPartitionDoneEvent.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 1135779 
bq.    trunk/metastore/src/test/org/apache/hadoop/hive/metastore/DummyListener.java 1135779

bq.    trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMarkPartition.java PRE-CREATION

bq.    trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMarkPartitionRemote.java
PRE-CREATION 
bq.    trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java
1135779 
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