----------------------------------------------------------- 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 > >
