Return-Path: X-Original-To: apmail-hive-dev-archive@www.apache.org Delivered-To: apmail-hive-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id EAEFFDF2E for ; Fri, 31 Aug 2012 07:51:29 +0000 (UTC) Received: (qmail 36863 invoked by uid 500); 31 Aug 2012 07:51:29 -0000 Delivered-To: apmail-hive-dev-archive@hive.apache.org Received: (qmail 36763 invoked by uid 500); 31 Aug 2012 07:51:29 -0000 Mailing-List: contact dev-help@hive.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@hive.apache.org Delivered-To: mailing list dev@hive.apache.org Received: (qmail 36730 invoked by uid 99); 31 Aug 2012 07:51:28 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 31 Aug 2012 07:51:28 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id DF11D1C1D51; Fri, 31 Aug 2012 07:51:27 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============8341325313154023130==" MIME-Version: 1.0 Subject: Re: Review Request: HIVE-3056: Ability to bulk update location field in Db/Table/Partition records From: "Carl Steinbach" To: "Carl Steinbach" Cc: "hive" , "Shreepadma Venugopalan" Date: Fri, 31 Aug 2012 07:51:27 -0000 Message-ID: <20120831075127.27227.29814@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org Auto-Submitted: auto-generated Sender: "Carl Steinbach" X-ReviewGroup: hive X-ReviewRequest-URL: https://reviews.apache.org/r/6650/ X-Sender: "Carl Steinbach" References: <20120829032439.27228.92394@reviews.apache.org> In-Reply-To: <20120829032439.27228.92394@reviews.apache.org> Reply-To: "Carl Steinbach" --===============8341325313154023130== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable ----------------------------------------------------------- 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 Ok, point taken. Do you think it would make sense to change the name? T= he 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 comp= areURI() or shouldUpdateURI() would make more sense? metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 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 chan= ge 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 valu= e supplied by the user. As a result I would assume that many Hive warehouse= s are going to contain tables with schema.url values that will cause this m= ethod to abort and rollback without completing. = metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java M-x untabify if all else fails :) metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java If the update fails because the location field is not a valid URI, does= n't that mean that the metastore record in question was already in an incon= sistent state? metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java Ok, sounds good. metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java We discussed this offline and reached an agreement. = metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java Ok, good point. Putting the objectstore initialization code in the cons= tructor 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 Ok, feel free to ignore my comment. metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java 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 Discussed offline. metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java 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 04= 5b550 = > metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.= java PRE-CREATION = > metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaTool.ja= va 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 > = > --===============8341325313154023130==--