You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Ashutosh Chauhan <ha...@apache.org> on 2011/04/12 03:29:41 UTC
Review Request: Review request for HIVE-2038
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/581/
-----------------------------------------------------------
Review request for hive, Carl Steinbach, John Sichi, and Paul Yang.
Summary
-------
Review request for HIVE-2038
This addresses bug HIVE-2038.
https://issues.apache.org/jira/browse/HIVE-2038
Diffs
-----
trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1079575
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 1079575
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreListener.java PRE-CREATION
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/NoOpListener.java PRE-CREATION
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 1079575
trunk/metastore/src/test/org/apache/hadoop/hive/metastore/DummyListener.java PRE-CREATION
trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMetaStoreListener.java PRE-CREATION
Diff: https://reviews.apache.org/r/581/diff
Testing
-------
Unit test with dummy listener included. Also tested with an alternate listener which sends a message on a message bus.
Thanks,
Ashutosh
Re: Review Request: Review request for HIVE-2038
Posted by Ashutosh Chauhan <ha...@apache.org>.
> On 2011-04-12 03:07:54, Carl Steinbach wrote:
> > trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java, line 999
> > <https://reviews.apache.org/r/581/diff/1/?file=15625#file15625line999>
> >
> > Unrelated bugfix?
Related bugfix, I will say : ) Without it, when drop partition returns from object store, partition object doesn't contain partition values.
> On 2011-04-12 03:07:54, Carl Steinbach wrote:
> > trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java, line 180
> > <https://reviews.apache.org/r/581/diff/1/?file=15621#file15621line180>
> >
> > Please add this property to hive-default.xml along with a description of what it does.
> >
Will add it in hive-default.xml.
> On 2011-04-12 03:07:54, Carl Steinbach wrote:
> > trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, line 955
> > <https://reviews.apache.org/r/581/diff/1/?file=15622#file15622line955>
> >
> > Please run checkstyle and correct any violations included in your patch.
Will run checkstyle to check for any style violations.
> On 2011-04-12 03:07:54, Carl Steinbach wrote:
> > trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreListener.java, line 27
> > <https://reviews.apache.org/r/581/diff/1/?file=15623#file15623line27>
> >
> > Please add some javadoc explaining the intended use of this interface.
> >
> > * Are the methods called before or after an action completes? What happens if a metastore operation fails?
> >
> > * Are the methods allowed to block? Are they run in a separate thread?
> >
> > * Are the methods allowed to modify the catalog objects that are passed in as parameters?
Will also add in javadoc.
* Methods are called after action completes. Only if action succeeds. They are not called if operation fails since in that case nothing has actually changed in metastore.
* This is upto implementation. They can run in same thread, or they can schedule there work in separate thread and return immediately.
* I don't see a reason to disallow modification of passed in parameter objects. But, its mostly irrelevant here since methods are called after change has already been persisted on metastore. So, modifying these objects can't change any state on metastore.
> On 2011-04-12 03:07:54, Carl Steinbach wrote:
> > trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreListener.java, line 29
> > <https://reviews.apache.org/r/581/diff/1/?file=15623#file15623line29>
> >
> > Instead of passing in raw Table/Partition/Database objects it may be better to instead wrap these objects in containers, e.g. CreateTableEvent, DropTableEvent, etc. Eventually this interface will probably include onAlterTable() and onAlterPartition(), and programmers will probably want to access both the before and after versions of a Table/Partition, etc.
Whats the advantage of wrapper container objects?
> On 2011-04-12 03:07:54, Carl Steinbach wrote:
> > trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, line 1446
> > <https://reviews.apache.org/r/581/diff/1/?file=15622#file15622line1446>
> >
> > No need to reference "this", right?
Right. Though, I think using "this" in such cases improves code readability.
> On 2011-04-12 03:07:54, Carl Steinbach wrote:
> > trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreListener.java, line 26
> > <https://reviews.apache.org/r/581/diff/1/?file=15623#file15623line26>
> >
> > What do you think about changing the name to MetaStoreEventListener or CatalogEventListener?
MetaStoreEventListener is fine too.
- Ashutosh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/581/#review428
-----------------------------------------------------------
On 2011-04-12 01:29:41, Ashutosh Chauhan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/581/
> -----------------------------------------------------------
>
> (Updated 2011-04-12 01:29:41)
>
>
> Review request for hive, Carl Steinbach, John Sichi, and Paul Yang.
>
>
> Summary
> -------
>
> Review request for HIVE-2038
>
>
> This addresses bug HIVE-2038.
> https://issues.apache.org/jira/browse/HIVE-2038
>
>
> Diffs
> -----
>
> trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1079575
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 1079575
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreListener.java PRE-CREATION
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/NoOpListener.java PRE-CREATION
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 1079575
> trunk/metastore/src/test/org/apache/hadoop/hive/metastore/DummyListener.java PRE-CREATION
> trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMetaStoreListener.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/581/diff
>
>
> Testing
> -------
>
> Unit test with dummy listener included. Also tested with an alternate listener which sends a message on a message bus.
>
>
> Thanks,
>
> Ashutosh
>
>
Re: Review Request: Review request for HIVE-2038
Posted by Carl Steinbach <ca...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/581/#review428
-----------------------------------------------------------
trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
<https://reviews.apache.org/r/581/#comment808>
Please add this property to hive-default.xml along with a description of what it does.
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/581/#comment809>
Please run checkstyle and correct any violations included in your patch.
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/581/#comment810>
No need to reference "this", right?
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreListener.java
<https://reviews.apache.org/r/581/#comment815>
What do you think about changing the name to MetaStoreEventListener or CatalogEventListener?
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreListener.java
<https://reviews.apache.org/r/581/#comment811>
Please add some javadoc explaining the intended use of this interface.
* Are the methods called before or after an action completes? What happens if a metastore operation fails?
* Are the methods allowed to block? Are they run in a separate thread?
* Are the methods allowed to modify the catalog objects that are passed in as parameters?
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreListener.java
<https://reviews.apache.org/r/581/#comment812>
Checkstyle.
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreListener.java
<https://reviews.apache.org/r/581/#comment816>
Instead of passing in raw Table/Partition/Database objects it may be better to instead wrap these objects in containers, e.g. CreateTableEvent, DropTableEvent, etc. Eventually this interface will probably include onAlterTable() and onAlterPartition(), and programmers will probably want to access both the before and after versions of a Table/Partition, etc.
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/581/#comment813>
Unrelated bugfix?
trunk/metastore/src/test/org/apache/hadoop/hive/metastore/DummyListener.java
<https://reviews.apache.org/r/581/#comment814>
Checkstyle.
- Carl
On 2011-04-12 01:29:41, Ashutosh Chauhan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/581/
> -----------------------------------------------------------
>
> (Updated 2011-04-12 01:29:41)
>
>
> Review request for hive, Carl Steinbach, John Sichi, and Paul Yang.
>
>
> Summary
> -------
>
> Review request for HIVE-2038
>
>
> This addresses bug HIVE-2038.
> https://issues.apache.org/jira/browse/HIVE-2038
>
>
> Diffs
> -----
>
> trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1079575
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 1079575
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreListener.java PRE-CREATION
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/NoOpListener.java PRE-CREATION
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 1079575
> trunk/metastore/src/test/org/apache/hadoop/hive/metastore/DummyListener.java PRE-CREATION
> trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMetaStoreListener.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/581/diff
>
>
> Testing
> -------
>
> Unit test with dummy listener included. Also tested with an alternate listener which sends a message on a message bus.
>
>
> Thanks,
>
> Ashutosh
>
>