You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Shreepadma Venugopalan <sh...@cloudera.com> on 2012/08/16 04:33:11 UTC
Review Request: HIVE-3056: Ability to bulk update location field in
Db/Table/Partition records
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6650/
-----------------------------------------------------------
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.
Additionally this patch also upgrades DN to 3.0
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
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 7ab76f9
conf/hive-default.xml.template fa92bb1
ivy/libraries.properties f0b1918
metastore/ivy.xml 3011d2f
metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaTool.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java ccb0d7f
Diff: https://reviews.apache.org/r/6650/diff/
Testing
-------
Manual testing for various metatool options.
Thanks,
Shreepadma Venugopalan
Re: Review Request: HIVE-3056: Ability to bulk update location field in
Db/Table/Partition records
Posted by Carl Steinbach <ca...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6650/#review10520
-----------------------------------------------------------
A couple more requests:
* Can you please add a link to this review in HIVE-3013. Also, please remember to change the status of HIVE-3013 to "patch available" every time you submit a new version of this patch for review.
* Please add a comment to HIVE-2084 saying that you plan to upgrade DN as part of this ticket and provide a link back to HIVE-3013.
Thanks.
- Carl Steinbach
On Aug. 16, 2012, 2:33 a.m., Shreepadma Venugopalan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6650/
> -----------------------------------------------------------
>
> (Updated Aug. 16, 2012, 2:33 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.
>
> Additionally this patch also upgrades DN to 3.0
>
>
> 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
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 7ab76f9
> conf/hive-default.xml.template fa92bb1
> ivy/libraries.properties f0b1918
> metastore/ivy.xml 3011d2f
> metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaTool.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java ccb0d7f
>
> Diff: https://reviews.apache.org/r/6650/diff/
>
>
> Testing
> -------
>
> Manual testing for various metatool options.
>
>
> Thanks,
>
> Shreepadma Venugopalan
>
>
Re: Review Request: HIVE-3056: Ability to bulk update location field in
Db/Table/Partition records
Posted by Shreepadma Venugopalan <sh...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6650/#review10790
-----------------------------------------------------------
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23270>
Done
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23272>
Done
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23273>
Done
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23274>
Moved the printing to HiveMetaTool and the results are printed only if the transaction succeeds.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23276>
Done
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23275>
Done
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23277>
Done
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23278>
Done
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23280>
Changed method to return a set of strings. HiveMetaTool prints the root location. The current implementation uses a hashset to eliminate duplicates.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23279>
Done
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23281>
done
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23284>
Replaced iterator with forloop
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23285>
Changed iterator to use forloop
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23248>
Done
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23250>
done
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23249>
done
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23251>
done
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23253>
done
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23254>
I'm not sure what you are trying to say here. I've changes the variable "HAUpgrade" to "updateHDFSRootLoc". We need to add this option to the cmdLineOptions variable.
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23268>
Replaced exception with system.err..
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23255>
Moved to main().
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23256>
I've removed the run method. I'll keep this in this mind while coding :)
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23257>
I've removed the run method and have moved the logic to main. I'll keep this i.e., exception rather than error codes should be used, in mind while coding :)
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23269>
Moved the logic in run() to main().
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23259>
done
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23261>
done
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23265>
I removed the exception and replaced it with a system.err.. I also print the help option.
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23266>
Moved the try catch block. Changed to use getLocalizedMessage() instead of getMessage()
- Shreepadma Venugopalan
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
>
>
Re: Review Request: HIVE-3056: Ability to bulk update location field in
Db/Table/Partition records
Posted by Carl Steinbach <ca...@cloudera.com>.
-----------------------------------------------------------
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
>
>
Re: Review Request: HIVE-3056: Ability to bulk update location field in
Db/Table/Partition records
Posted by Carl Steinbach <ca...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6650/#review10836
-----------------------------------------------------------
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23341>
Please remove this blank line.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23340>
These methods don't know for a fact that they're getting called by HiveMetaTool, so I don't think the logging statements should reference HiveMetaTool.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23342>
blank line.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23343>
blank line.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23344>
I think this should be refactored one of two ways:
1) Create a method "Set<MDatabase> getDatabases()" that returns all MDatabase records, and then iterate over the result to extract the locationUris.
2) Execute a JDOQL query that only extracts the locationUri field (see ObjectStore.getDatabases(String pattern) for an example of how to do this).
Option (1) is more generic, but option (2) will be more efficient since it avoids extracting unnecessary information.
In either case the name and signature of this method should be changed to Set<URI> getDatabaseLocationURIs()
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23345>
This logic belongs in HiveMetaTool.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23339>
Please be consistent with blank lines. I don't think having one here improves readability.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23348>
This logic (or most of it) also needs to get moved up into HiveMetaTool. The tip-off that this is necessary is that we can't print to stdout/stderr here, and printing the dry-run info to the log stream isn't good enough because it requires folks to have logging enabled and redirected to the console in order to actually see it.
I also tend to doubt that we want to do the entire update in a single transaction. Most administrators will probably find a best effort approach more useful since it means they can use at least some of their tables after an HA failover even some updates fail to commit.
Also, one of the Java coding conventions is to give boolean variables names starting with "is", e.g. "isDryRun".
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23350>
Regardless of where this logic goes, I think the method is too long. It needs to get split up into smaller pieces. I recommend splitting it into separate methods for the DB, StorageDescriptor, and SerdeParam updates.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23346>
Formatting.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23347>
Formatting.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23337>
Looks like there's a TAB here.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23351>
It looks like if we encounter this exception we're not going to be able to skip over the bad field and continue updating those that follow. Utilities like this should generally try to make a best effort to execute the user's command, and log any errors that they encounter to stderr.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23349>
Info like this needs to get dumped to stderr, which reinforces the point that this logic belongs in HiveMetaTool.
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23352>
You're working around commons-cli instead of with it. This should get split into three separate arguments. If they aren't relevant to the specific action requested by the user (e.g. when a query string is specified) you can either abort with an error or ignore them.
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23353>
Prefixing everything with "HiveMetaTool" is unnecessary.
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23354>
Generally speaking when a class has a main() method the main() method should only be responsible for parsing the command line args and using these values to initialize and control an instance of the class via the public methods that the class exposes. In other words, main() should not refer directly to ObjectStore -- it should be handled instead in HiveMetaTool's constructor or in its init() method. The advantage of structuring the code this way is that you end up with a class that can also be used productively by other code.
The other reason why you need to structure the code this way is because Java's main method must have a void return type, which precludes you from returning non-zero values on errors. In order to do that you have to call System.exit(), and doing that makes it difficult for other code to cheat and invoke your main() method directly.
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23355>
blank line.
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23356>
We should print the help screen here and then exit immediately with a non-zero return value.
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23358>
Always assume that the user is going to use the command as part of a pipeline. Given that assumption it makes sense to only output the information they requested unless an error occurred.
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23357>
Under what circumstances would you expect this error to get triggered?
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23359>
This does not look right. Please handle this with commons-cli instead.
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23360>
This looks weird.
updateLocations = objStore.updateFSRootLocations(...)
if (isDryRun) {
printUpdateLocations(updateLocations);
}
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23361>
Putting shudown code at the end of a long method like this is risky because it assumes that you'll always make it here. Maybe that's the case today, but you have to assume the next person who modifies this code maintain that invariant. Code in a finally() block always gets executed (ok, there are some exceptions to that rule), so putting your cleanup code in a finally block (even if you don't have any catch blocks) is usually a good idea.
metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23362>
Wow this looks painful. We really need to make MetaStoreClient easier to use.
metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23363>
Does anything here actually throw a java.lang.Error? If not then you should catch an Exception instead of a Throwable.
metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23364>
The test methods that JUnit calls should be declare to throw exceptions. If one is thrown JUnit will automatically log the exception along with additional debugging info. Squelching it here means that your test will always pass.
metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23338>
Extra ';'
- 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
>
>
Re: Review Request: HIVE-3056: Ability to bulk update location field in
Db/Table/Partition records
Posted by Carl Steinbach <ca...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6650/#review10837
-----------------------------------------------------------
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23365>
I take back the last comment about returning a collection of URIs. Since there's always the possibility that one of the fields won't form a valid URI we should just return a collection of the raw Strings, and let HiveMetaTool do the conversion from String to URI. If one of those conversions fails HiveMetaTool will be a better position to handle the failure and alert the user. Doing the conversion at this level makes it hard to take a best effort approach.
- 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
>
>
Re: Review Request: HIVE-3056: Ability to bulk update location field in
Db/Table/Partition records
Posted by Shreepadma Venugopalan <sh...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6650/
-----------------------------------------------------------
(Updated Sept. 3, 2012, 4:29 p.m.)
Review request for hive and Carl Steinbach.
Changes
-------
This revision addresses comments from revision # 6.
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 (updated)
-----
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/TestHiveMetaStore.java 369eebe
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
Re: Review Request: HIVE-3056: Ability to bulk update location field in
Db/Table/Partition records
Posted by Shreepadma Venugopalan <sh...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6650/#review10848
-----------------------------------------------------------
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23414>
Will do.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23413>
I'll remove HiveMetaTool from the logging information.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23415>
Will do.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23416>
Will remove.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23411>
This logic checks if the location URI on disk matches the location URI that the user has specified for the old-nn-location. ObjectStore retrieves the locationURI from the metastore and needs to verify that the locationURI in metastore matches the URI that the user has passed in before updating the records in the metastore. I'm not sure why this logic should be in HiveMetaTool.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23417>
This logic checks if the URI in the metastore record matches the NN location URI that the user has passed in. ObjectStore retrieves the metastore records and decides to update a given record based on whether the record's current location URI matches the URI that the user has passed in. This logic is an integral and central part of the update logic which belongs in the ObjectStore. I don't see how this logic can belong elsewhere.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23388>
Will remove.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23418>
The output of the dryRun i.e., the current location and the new location are returned as a map of string values that are printed to stdout by HiveMetaTool. I think most of the logic in this function belongs in the ObjectStore because it uses the JDO and DN to access and write to the metastore records. Based on the previous revisions,
I understand that we want to encapsulate all JDO/DN logic within ObjectStore.
I'll rename dryRun to isDryRun.
I think we should do the update in a single transaction ie., the HA upgrade should be atomic. If for some reason the upgrade fails, I think its better to have all of the records in the same state. The admin may later choose to retry the update. If we allowed non-atomic or partial updates then we would end up in a state where some records point to the HA NN and some others don't. In such a scenario some tables and the underlying data would be inaccessible. On the other hand if we allowed only an atomic upgrade and if the upgrade of the metastore failed the admin may choose not to use a HA NN till the HA upgrade of the metastore can be successfully completed.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23419>
I'll split this method into updates for DB, SerdeParam and StorageDescriptor.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23420>
Will do.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23421>
Will remove.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23389>
This tab was added by eclipse when I split the line up. I'll try to remove this.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23391>
I think we should either update all the metastore records or none. Updating some of the records and failing in between leaves the metastore in an inconsistent state with some records updated and others not. I think the metastore update should be atomic. Please see my earlier comment on this.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23392>
While some of the logic such as some of the printing can be moved to the metatool, most of the logic in this method i.e., the logic that accesses the metastore through JDO should probably reside in objectstore. While executing some of that logic it becomes useful to output information e.g., the stage of processing we are in etc. I think its OK to use LOG.info to do some of that since the purpose of doing so is for debugging if there is a problem. Please not that some other ObjectStore methods do that as well.
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23380>
Unfortunately the arg name can't be split into three - I tried that and the only the last arg is printed. On googling I found out that this is a known problem with the commons-cli and helpformatter classes.
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23381>
I can move the ObjectStore constructor to the init method if you like. However, I'd like to explain why I've it in the main method. Initializing the object store is an expensive operation and I think we should not do it unless we want to use the options such as -updateLocation, -listFSRoot etc that actually need the ObjectStore. We don't have to initialize the ObjectStore for instance to print the help menu.
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23382>
Done
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23383>
I'll print the help menu here.
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23384>
I thought it was informative to print this information. If you like I'll remove all System.out.println that prints the stage of the pipeline that the tool is in. Please confirm that's what you want.
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23385>
listFSRoot() will return null if the commit of the transaction fails. So if an error occurs during the commit of the JDO transaction during execution of the listFSRoot() operation, this error will be triggered.
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23431>
I tried to handle this using commons-cli but there doesn't seem to be a good way to do this. I don't see a way to add an option within another option i.e, there doesn't seem to be a way to support nested options. dryRun is an option that is valid only in the context of the updateLocation option. So I can't create a new Options object for dryRun. If you have a suggestion on how to do this through commons-cli I'll be happy to hear.
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23386>
I'll make this change.
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23387>
This was originally in the finally block. In a previous revision you asked me to move the try catch as close as possible to the code that could potentially raise an exception. So I moved try .. catch block as close to parser.parse as possible.
- Shreepadma Venugopalan
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
>
>
Re: Review Request: HIVE-3056: Ability to bulk update location field in
Db/Table/Partition records
Posted by Shreepadma Venugopalan <sh...@cloudera.com>.
-----------------------------------------------------------
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.
Changes
-------
This revision to the original patch addresses almost all of the review comments from revision # 5. Additionally the following changes have been made to the -updateLocation option,
* A new option -dryRun has been added. When run with the -dryRun option persistent changes are not made. Instead the current location and the proposed new location are printed to stdout.
* Both new-loc and old-loc have to be valid URIs. This validation is performed in the code and an error is raised if either the new-loc or the old-loc is not a valid URI. Please note that both the host name and scheme fields of the URI are required, while the port is optional. However, if the old-loc contains a port, the new-loc should specify a port too. The motivation behind making the scheme a required field is to prevent an inadvertent update of the location field when the schemes doesn't match. Please note that the primary motivation for this tool at this point is to update the location field to ensure Hive survives a non-HA to HA upgrade and vice versa.
* The test case has been fixed to remove hard coded paths in the locationURI field of MDatabase.
Additionally the updateLocation option of the metatool has been manually tested on a real cluster. All of the locations in the metastore correctly point to the HDFS NN and running metatool -updateLocation <new-nn-loc> <old-nn-loc> correctly updates the location of all relevant records to point to the new HA NN.
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 (updated)
-----
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
Re: Review Request: HIVE-3056: Ability to bulk update location field in
Db/Table/Partition records
Posted by Carl Steinbach <ca...@cloudera.com>.
> On Aug. 26, 2012, 3:48 a.m., Carl Steinbach wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java, line 133
> > <https://reviews.apache.org/r/6650/diff/5/?file=146096#file146096line133>
> >
> > Please use e.getLocalizedMessage() instead. Also, it's generally a good idea to put the catch() block as close to the thrower as possible. Wrapping parser.parse() with it's own try/catch block would probably be cleaner.
s/it's/its/
- Carl
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6650/#review10764
-----------------------------------------------------------
On Aug. 25, 2012, 9 p.m., Shreepadma Venugopalan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6650/
> -----------------------------------------------------------
>
> (Updated Aug. 25, 2012, 9 p.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
> conf/hive-default.xml.template cc1565f
> 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
>
>
Re: Review Request: HIVE-3056: Ability to bulk update location field in
Db/Table/Partition records
Posted by Carl Steinbach <ca...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6650/#review10764
-----------------------------------------------------------
conf/hive-default.xml.template
<https://reviews.apache.org/r/6650/#comment23202>
This comment has fallen out of sync with the value given. Please back this change out since it is no longer relevant to this patch.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23204>
This comment should be repeated for each of the new methods added to ObjectStore.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23203>
Please replace the println calls with LOG.info() calls. Generally speaking the only code that should use System.*.println to print directly to stdout/stderr is stuff that's located in an application's main() method, or in classes that contain a main() method.
Also, please change the sentence to use the active tense, i.e. "Executing query: ..."
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23205>
Seems like this stuff should only get printed out if the transaction succeeded.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23207>
Formatting: please remove these blank lines.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23206>
Same deal here. I know that commitTransaction is probably kind of broken, but we should only print this out if committed == true. Also, each of these methods should probably either return a boolean or throw an exception in order to indicate that transaction succeeded or failed.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23209>
Doesn't look the 'committed' boolean variable is used. Also, this function should probably return a List<String> of hdfsRoot strings instead of printing them out.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23208>
Formatting: '{' goes on the same line as the method parameter list.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment23210>
Please use a foreach loop instead of an explicit iterator. Same comment applies to the other methods when possible.
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23211>
s/data nucleus/DataNucleus/
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23216>
s/location/locations/
Also, it would be good to eliminate duplicate entries from the output.
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23221>
"Update HDFS root location..." instead of "Updates HDFS root location..."
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23219>
Please change "HAUpgrade" to something that isn't HA specific. Also, all variable names (with the exception of final statics) should adhere to the camel-case naming convention, including starting with a lowercase letter (which I admit is a bit awkward in this case).
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23220>
I'm a little worried that this operation is too generic, and that as a consequence we're probably missing some errors. Every location value stored in MDatabase and MStorageDescriptor should be a valid o.a.h.fs.Path object (http://hadoop.apache.org/common/docs/current/api/org/apache/hadoop/fs/Path), which means that it should also be a valid URI (http://docs.oracle.com/javase/6/docs/api/java/net/URI.html?is-external=true). java.net.URI allows you to pick out individual components of the URI, and in this case I think we're mainly interested in updating the "host" (and possibly the "port") part of the URI. So I think metatool should give you option of specifying a scheme, host, and port to match (although only the host is required), as well as replacement values for the those three parts, although only the host is required. Another thing to point out is that all three of these things should be case-insensitive (which can't be said for "path" component of the URI), so everything should get converted to lowercase in ObjectStore before the matching logic is applied.
I think what we want is something like the following:
metatool --updateLocation [--oldScheme=hdfs] --oldHost=127.0.0.1 [--oldPort=xyz] --newHost=yoyodyne [--newPort=333] etc.
Also, can you please add a --dryRun option that dumps logs the changes that would be made to stdout but doesn't actually make any of the changes? This way people can first experiment with the options without making any big mistakes.
Finally, it's possible to run Hive on top of other filesystems besides HDFS (S3 is one example), so it's probably best to avoid using the term "HDFS" in this code.
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23218>
Please remove.
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23213>
main() should not throw any checked exceptions. Please catch them and print appropriate error messages.
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23212>
I don't think moving this logic to run() really accomplishes anything. Please move it back into main().
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23223>
It looks like run() has two ways of signaling an error, either by returning a non-zero value or by throwing an exception. Exceptions are semantically much richer than return codes (e.g. you can wrap return codes in exceptions if you really want to) and are the right way of handling error conditions in Java. Generally speaking the only method that should use an error return code is main().
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23217>
Options should start with a lowercase letter, e.g. "listHDFSRoots", "executeJDOQL", etc.
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23215>
Formatting: else should go on the same line as '}'
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23214>
A couple things:
* Throwing java.lang.Exception is never the right thing to do. If you have to throw an exception you need to throw a subclass of java.lang.Exception, or the appropriate runtime exception if it's not something that you expect another caller further down the stack will be able to handle.
* If the command line args are incorrect or unrecognizable it's considerably more polite to echo the unrecognizable part to stderr and then print the help screen. Right now when I run metatool without an arguments I get an exception. It should print the help info instead.
metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment23222>
Please use e.getLocalizedMessage() instead. Also, it's generally a good idea to put the catch() block as close to the thrower as possible. Wrapping parser.parse() with it's own try/catch block would probably be cleaner.
- Carl Steinbach
On Aug. 25, 2012, 9 p.m., Shreepadma Venugopalan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6650/
> -----------------------------------------------------------
>
> (Updated Aug. 25, 2012, 9 p.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
> conf/hive-default.xml.template cc1565f
> 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
>
>
Re: Review Request: HIVE-3056: Ability to bulk update location field in
Db/Table/Partition records
Posted by Shreepadma Venugopalan <sh...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6650/
-----------------------------------------------------------
(Updated Aug. 25, 2012, 9 p.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 (updated)
-----
bin/ext/metatool.sh PRE-CREATION
bin/metatool PRE-CREATION
build.xml 6712af9
conf/hive-default.xml.template cc1565f
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
Re: Review Request: HIVE-3056: Ability to bulk update location field in
Db/Table/Partition records
Posted by Shreepadma Venugopalan <sh...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6650/
-----------------------------------------------------------
(Updated Aug. 25, 2012, 8:56 p.m.)
Review request for hive and Carl Steinbach.
Changes
-------
Updated patch to change the name of HAUpgrade option to UpdateLocation.
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 (updated)
-----
bin/ext/metatool.sh PRE-CREATION
bin/metatool PRE-CREATION
build.xml 6712af9
conf/hive-default.xml.template cc1565f
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/TestHiveMetaStore.java 369eebe
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
Re: Review Request: HIVE-3056: Ability to bulk update location field in
Db/Table/Partition records
Posted by Shreepadma Venugopalan <sh...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6650/
-----------------------------------------------------------
(Updated Aug. 23, 2012, 11:49 p.m.)
Review request for hive and Carl Steinbach.
Description (updated)
-------
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
conf/hive-default.xml.template cc1565f
eclipse-templates/TestHiveMetaTool.launchtemplate PRE-CREATION
metastore/ivy.xml 3011d2f
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 6f4716c
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 (updated)
-------
A new JUnit test - TestHiveMetaTool - has been added to test the various metatool options.
Thanks,
Shreepadma Venugopalan
Re: Review Request: HIVE-3056: Ability to bulk update location field in
Db/Table/Partition records
Posted by Shreepadma Venugopalan <sh...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6650/
-----------------------------------------------------------
(Updated Aug. 23, 2012, 8:19 p.m.)
Review request for hive and Carl Steinbach.
Changes
-------
Updated patch to remove TestHiveMetaTool.launch file with hard coded paths. The new patch contains TestHiveMetaTool.launchtemplate file with relative paths.
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.
Additionally this patch also upgrades DN to 3.0
This addresses bug HIVE-3056.
https://issues.apache.org/jira/browse/HIVE-3056
Diffs (updated)
-----
bin/ext/metatool.sh PRE-CREATION
bin/metatool PRE-CREATION
build.xml 6712af9
conf/hive-default.xml.template cc1565f
eclipse-templates/TestHiveMetaTool.launchtemplate PRE-CREATION
metastore/ivy.xml 3011d2f
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 6f4716c
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
-------
Manual testing for various metatool options.
Thanks,
Shreepadma Venugopalan
Re: Review Request: HIVE-3056: Ability to bulk update location field in
Db/Table/Partition records
Posted by Shreepadma Venugopalan <sh...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6650/#review10555
-----------------------------------------------------------
metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment22627>
Moved to o.a.h.hive.metastore.tools
metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment22585>
Done
metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment22586>
done
metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment22629>
Moved to ObjectStore.java
metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment22587>
done
metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment22628>
Done
metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment22588>
done
- Shreepadma Venugopalan
On Aug. 22, 2012, 2:48 a.m., Shreepadma Venugopalan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6650/
> -----------------------------------------------------------
>
> (Updated Aug. 22, 2012, 2:48 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.
>
> Additionally this patch also upgrades DN to 3.0
>
>
> This addresses bug HIVE-3056.
> https://issues.apache.org/jira/browse/HIVE-3056
>
>
> Diffs
> -----
>
> TestHiveMetaTool.launch PRE-CREATION
> bin/ext/metatool.sh PRE-CREATION
> bin/metatool PRE-CREATION
> build.xml 6712af9
> conf/hive-default.xml.template cc1565f
> metastore/ivy.xml 3011d2f
> metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 6f4716c
> 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
> -------
>
> Manual testing for various metatool options.
>
>
> Thanks,
>
> Shreepadma Venugopalan
>
>
Re: Review Request: HIVE-3056: Ability to bulk update location field in
Db/Table/Partition records
Posted by Carl Steinbach <ca...@cloudera.com>.
> On Aug. 22, 2012, 10:51 a.m., namit jain wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java, line 3903
> > <https://reviews.apache.org/r/6650/diff/2/?file=143762#file143762line3903>
> >
> > Is this a good idea ?
> >
> > Adding some API's in ObjectStore that are not to be exposed by thrift, and assumes a JDO.
Seems like a good idea to me. ObjectStore became JDO specific a long time ago. Also, it's important to remember that ObjectStore is just one possible implementation of the RawStore interface. We could also have (and once did have) a FileStore, or a JPAStore, etc, etc, and we don't want to expose these implementation details through a public API like the Thrift interface.
- Carl
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6650/#review10616
-----------------------------------------------------------
On Aug. 25, 2012, 9 p.m., Shreepadma Venugopalan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6650/
> -----------------------------------------------------------
>
> (Updated Aug. 25, 2012, 9 p.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
> conf/hive-default.xml.template cc1565f
> 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
>
>
Re: Review Request: HIVE-3056: Ability to bulk update location field in
Db/Table/Partition records
Posted by namit jain <nj...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6650/#review10616
-----------------------------------------------------------
TestHiveMetaTool.launch
<https://reviews.apache.org/r/6650/#comment22716>
looks like a hard-coded path
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment22718>
Is this a good idea ?
Adding some API's in ObjectStore that are not to be exposed by thrift, and assumes a JDO.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment22717>
System.out.println() in hive code seems like a bad idea
- namit jain
On Aug. 22, 2012, 2:48 a.m., Shreepadma Venugopalan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6650/
> -----------------------------------------------------------
>
> (Updated Aug. 22, 2012, 2:48 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.
>
> Additionally this patch also upgrades DN to 3.0
>
>
> This addresses bug HIVE-3056.
> https://issues.apache.org/jira/browse/HIVE-3056
>
>
> Diffs
> -----
>
> TestHiveMetaTool.launch PRE-CREATION
> bin/ext/metatool.sh PRE-CREATION
> bin/metatool PRE-CREATION
> build.xml 6712af9
> conf/hive-default.xml.template cc1565f
> metastore/ivy.xml 3011d2f
> metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 6f4716c
> 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
> -------
>
> Manual testing for various metatool options.
>
>
> Thanks,
>
> Shreepadma Venugopalan
>
>
Re: Review Request: HIVE-3056: Ability to bulk update location field in
Db/Table/Partition records
Posted by Shreepadma Venugopalan <sh...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6650/#review10629
-----------------------------------------------------------
TestHiveMetaTool.launch
<https://reviews.apache.org/r/6650/#comment22730>
Thanks Namit. My bad. I'll fix this.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment22729>
The reason behind adding these methods to the ObjectStore class is as follows. HiveMetaTool accesses the metastore through JDO and DN. I think the JDO and the metastore access logic should reside in ObjectStore; it doesn't seem to make a lot of sense to duplicate this logic elsewhere, for instance in the HiveMetaTool. Hence we decided to add the methods to ObjectStore.
While the other ObjectStore methods that access the metastore are exposed via Thrift, HiveMetaTool is yet another way to expose them.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment22731>
The reason behind adding these methods to ObjectStore is as follows. HiveMetaTool accesses the metastore through JDO and DN. ObjectStore seems like the place for all JDO and metastore access logic to reside. To me it doesn't make a lot of sense to duplicate this code elsewhere say for instance in the HiveMetaTool. While most of the ObjectStore methods are exposed via Thrift, I think of HiveMetaTool as yet another way to expose some of the ObjectStore methods.
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/6650/#comment22749>
Please let me know what the alternative is.
- Shreepadma Venugopalan
On Aug. 22, 2012, 2:48 a.m., Shreepadma Venugopalan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6650/
> -----------------------------------------------------------
>
> (Updated Aug. 22, 2012, 2:48 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.
>
> Additionally this patch also upgrades DN to 3.0
>
>
> This addresses bug HIVE-3056.
> https://issues.apache.org/jira/browse/HIVE-3056
>
>
> Diffs
> -----
>
> TestHiveMetaTool.launch PRE-CREATION
> bin/ext/metatool.sh PRE-CREATION
> bin/metatool PRE-CREATION
> build.xml 6712af9
> conf/hive-default.xml.template cc1565f
> metastore/ivy.xml 3011d2f
> metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 6f4716c
> 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
> -------
>
> Manual testing for various metatool options.
>
>
> Thanks,
>
> Shreepadma Venugopalan
>
>
Re: Review Request: HIVE-3056: Ability to bulk update location field in
Db/Table/Partition records
Posted by Shreepadma Venugopalan <sh...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6650/
-----------------------------------------------------------
(Updated Aug. 22, 2012, 2:48 a.m.)
Review request for hive and Carl Steinbach.
Changes
-------
Updated patch with the following changes,
* Uses commons.cli for parsing command line arguements
* Methods accessing PM are in ObjectStore.java
* JUnit test - TestHiveMetaTool.java
* Other misc. review comments
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.
Additionally this patch also upgrades DN to 3.0
This addresses bug HIVE-3056.
https://issues.apache.org/jira/browse/HIVE-3056
Diffs (updated)
-----
TestHiveMetaTool.launch PRE-CREATION
bin/ext/metatool.sh PRE-CREATION
bin/metatool PRE-CREATION
build.xml 6712af9
conf/hive-default.xml.template cc1565f
metastore/ivy.xml 3011d2f
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 6f4716c
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
-------
Manual testing for various metatool options.
Thanks,
Shreepadma Venugopalan
Re: Review Request: HIVE-3056: Ability to bulk update location field in
Db/Table/Partition records
Posted by Carl Steinbach <ca...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6650/#review10517
-----------------------------------------------------------
The major thing missing at this point is a testcase. Since this a command line tool the JUnit testcase should invoke metatool by calling its main() method. I also think it would be a good idea to create a separate class for this (e.g. TestHiveMetaTool) instead of adding it to something that already exists like TestHiveMetaStore.
metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment22506>
I think we should change the package to either o.a.h.hive.metastore.tools or o.a.h.hive.metastore.utils
metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment22507>
Formatting.
metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment22508>
No point including an empty finally block.
metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment22509>
Formatting: '{' always go on the same line.
metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment22510>
Please move this to MetaStoreUtils and have this class and ObjectStore depend on the same version.
metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment22525>
So I realize this probably contradicts what I said earlier, but after looking more closely at this I think the methods that reference the pm (e.g. executeSelect, executeUpdate, etc), should be moved into ObjectStore, along with a note explaining that these are *NOT* to be exposed through the Metastore Thrift interface. If we take this route then it won't be necessary to duplicate code (or copy duplicated code over to a Utils class). Please also change the names of these methods to executeJDOQLUpdate, executeJDOQLSelect, etc.
metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment22511>
Please use 2 spaces for indents. I think this problem exists in a couple other places.
metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment22505>
Formatting: TAB
metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment22514>
Please use commons-cli for handing command line arguments. Take a look at cli/src/java/org/apache/hadoop/hive/cli/OptionsProcessor.java for an example of how to do this.
metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaTool.java
<https://reviews.apache.org/r/6650/#comment22516>
Formatting.
- Carl Steinbach
On Aug. 16, 2012, 2:33 a.m., Shreepadma Venugopalan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6650/
> -----------------------------------------------------------
>
> (Updated Aug. 16, 2012, 2:33 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.
>
> Additionally this patch also upgrades DN to 3.0
>
>
> 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
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 7ab76f9
> conf/hive-default.xml.template fa92bb1
> ivy/libraries.properties f0b1918
> metastore/ivy.xml 3011d2f
> metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaTool.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java ccb0d7f
>
> Diff: https://reviews.apache.org/r/6650/diff/
>
>
> Testing
> -------
>
> Manual testing for various metatool options.
>
>
> Thanks,
>
> Shreepadma Venugopalan
>
>
Re: Review Request: HIVE-3056: Ability to bulk update location field in
Db/Table/Partition records
Posted by Shreepadma Venugopalan <sh...@cloudera.com>.
> On Aug. 20, 2012, 1:20 a.m., Carl Steinbach wrote:
> > Also, is the DN upgrade still necessary now that you're no longer trying to use JDOQL's batch update feature?
No. DN upgrade is not needed. I'll revert back to 2.0.3 in this patch.
- Shreepadma
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6650/#review10521
-----------------------------------------------------------
On Aug. 22, 2012, 2:48 a.m., Shreepadma Venugopalan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6650/
> -----------------------------------------------------------
>
> (Updated Aug. 22, 2012, 2:48 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.
>
> Additionally this patch also upgrades DN to 3.0
>
>
> This addresses bug HIVE-3056.
> https://issues.apache.org/jira/browse/HIVE-3056
>
>
> Diffs
> -----
>
> TestHiveMetaTool.launch PRE-CREATION
> bin/ext/metatool.sh PRE-CREATION
> bin/metatool PRE-CREATION
> build.xml 6712af9
> conf/hive-default.xml.template cc1565f
> metastore/ivy.xml 3011d2f
> metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 6f4716c
> 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
> -------
>
> Manual testing for various metatool options.
>
>
> Thanks,
>
> Shreepadma Venugopalan
>
>
Re: Review Request: HIVE-3056: Ability to bulk update location field in
Db/Table/Partition records
Posted by Carl Steinbach <ca...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6650/#review10521
-----------------------------------------------------------
Also, is the DN upgrade still necessary now that you're no longer trying to use JDOQL's batch update feature?
- Carl Steinbach
On Aug. 16, 2012, 2:33 a.m., Shreepadma Venugopalan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6650/
> -----------------------------------------------------------
>
> (Updated Aug. 16, 2012, 2:33 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.
>
> Additionally this patch also upgrades DN to 3.0
>
>
> 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
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 7ab76f9
> conf/hive-default.xml.template fa92bb1
> ivy/libraries.properties f0b1918
> metastore/ivy.xml 3011d2f
> metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaTool.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java ccb0d7f
>
> Diff: https://reviews.apache.org/r/6650/diff/
>
>
> Testing
> -------
>
> Manual testing for various metatool options.
>
>
> Thanks,
>
> Shreepadma Venugopalan
>
>