hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Carl Steinbach" <c...@cloudera.com>
Subject Re: Review Request: HIVE-3056: Ability to bulk update location field in Db/Table/Partition records
Date Fri, 31 Aug 2012 07:51:27 GMT

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



metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23480>

    Ok, point taken. Do you think it would make sense to change the name? The current name
makes it seem like this method does the actual update, but what it's really doing is checking
to see if the two URIs match. Maybe compareURI() or shouldUpdateURI() would make more sense?



metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23481>

    I disagree with your take on this. If you're an admin and your NN just failed over and
your Hive warehouse goes offline because all of the tables point to the wrong NN, you'll be
glad if you can get at least some of them back online ASAP. The other issue worth considering
is that anyone can change the schema.url serde parameter using the ALTER TABLE SET SERDEPROPERTIES
command, and I don't think any validation logic is applied to the new value supplied by the
user. As a result I would assume that many Hive warehouses are going to contain tables with
schema.url values that will cause this method to abort and rollback without completing. 



metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23482>

    M-x untabify if all else fails :)



metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23483>

    If the update fails because the location field is not a valid URI, doesn't that mean that
the metastore record in question was already in an inconsistent state?



metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23484>

    Ok, sounds good.



metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23485>

    We discussed this offline and reached an agreement. 



metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23486>

    Ok, good point. Putting the objectstore initialization code in the constructor doesn't
make sense, but I don't think it should get called directly from main() either. 



metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23487>

    Ok, feel free to ignore my comment.



metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23488>

    In that case I think it would be good for the error message to include more information
about the actual cause of the error.



metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23490>

    Discussed offline.



metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23489>

    Discussed offline.


- Carl Steinbach


On Aug. 29, 2012, 3:24 a.m., Shreepadma Venugopalan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6650/
> -----------------------------------------------------------
> 
> (Updated Aug. 29, 2012, 3:24 a.m.)
> 
> 
> Review request for hive and Carl Steinbach.
> 
> 
> Description
> -------
> 
> This patch implement hive metatool which,
> 
> * lets admins perform a HA upgrade by patching the location of the NN in Hive's metastore
> * allows JDOQL to be executed against the metastore.
> 
> 
> This addresses bug HIVE-3056.
>     https://issues.apache.org/jira/browse/HIVE-3056
> 
> 
> Diffs
> -----
> 
>   bin/ext/metatool.sh PRE-CREATION 
>   bin/metatool PRE-CREATION 
>   build.xml 6712af9 
>   eclipse-templates/TestHiveMetaTool.launchtemplate PRE-CREATION 
>   metastore/ivy.xml 3011d2f 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 045b550 
>   metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java PRE-CREATION

>   metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaTool.java PRE-CREATION

> 
> Diff: https://reviews.apache.org/r/6650/diff/
> 
> 
> Testing
> -------
> 
> A new JUnit test - TestHiveMetaTool - has been added to test the various metatool options.
> 
> 
> Thanks,
> 
> Shreepadma Venugopalan
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message