[ 
https://issues.apache.org/jira/browse/HBASE-8348?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13746848#comment-13746848
 ] 

stack commented on HBASE-8348:
------------------------------

Regards this message, +        System.out.println("ERROR: Please look for 
corrupt files, or regions to major compact.");

... what would a corrupt file be?  Are you going to freak folks out if they see 
this message?  Does the v1 checker print out the instnaces of v1 hfiles?

I'm w/ Jeffrey that these messages should be LOG.warn or LOG.error rather than 
System.out.println... then you don' thave to invent your own style of ERROR 
logging w/ your ERROR prefix.  Will also have date stamp on it... and there 
other facilities you could use like printing stack trace

Yeah, stuff like this could then be logged as LOG.warn:

+          System.err.println("Backup master(s) " + backupMasters
+              + " are alive or backup-master znodes not expired.");

Why bother w/ the system.err.println if we are going to throw a IOE?

+      System.err.println(e);
+      throw new IOException(e);

Good..

+      if (zkw != null) {
+        zkw.close();
+      }


Won't the first res be overwritten by the second -- is that what you want?  
Shoudl you proceed if the first has a non-zero result?  And why not just do 
return upgradeZNodes instead of catch result in a res and the in new statement 
doing the return res.

+    int res = upgradeNamespace();
+    res = upgradeZnodes();


Should be LOG.info: +    System.out.println("Upgrading Namespace");

Should be LOG.error: +      System.err.println("FAILURE: while updating 
Namespace"); (You are inventing your own language and it is inconsistent... it 
is ERROR earlier and then here it is FAILURE)

Is this generally useful or should it instead be under the migration package:

diff --git 
hbase-server/src/main/java/org/apache/hadoop/hbase/util/ZKDataMigrator.java 
hbase-server/src/main/java/org/apache/hadoop/hbase/util/ZKDataMigrator.java


Why get a logger and then do this?

+        System.out.println("No hbase related data available in zookeeper. 
returning..");
+        return 0;

Just use loggers everywhere I'd say except for the usage output.

I would be interested in seeing a log of a run of this tool.

Thanks [[email protected]]  Lets get this turned around quick.  Main 
blocker on a 0.96RC.  Thanks.
+    return res;
                
> Polish the migration to 0.96
> ----------------------------
>
>                 Key: HBASE-8348
>                 URL: https://issues.apache.org/jira/browse/HBASE-8348
>             Project: HBase
>          Issue Type: Bug
>    Affects Versions: 0.95.0
>            Reporter: Jean-Daniel Cryans
>            Assignee: rajeshbabu
>            Priority: Critical
>             Fix For: 0.96.0
>
>         Attachments: HBASE-8348-approach-2.patch, 
> HBASE-8348-approach-2-v2.1.patch, HBASE-8348-approach-2-v2.2.patch, 
> HBASE-8348-approach-3.patch, HBASE-8348_trunk.patch, 
> HBASE-8348_trunk_v2.patch, HBASE-8348_trunk_v3.patch
>
>
> Currently, migration works but there's still a couple of rough edges:
>  - HBASE-8045 finished the .META. migration but didn't remove ROOT, so it's 
> still on the filesystem.
>  - Data in ZK needs to be removed manually. Either we fix up the data in ZK 
> or we delete it ourselves.
>  - TestMetaMigrationRemovingHTD has a testMetaUpdatedFlagInROOT method, but 
> ROOT is gone now.
> Elliott was also mentioning that we could have "hbase migrate" do the HFileV1 
> checks, clear ZK, remove ROOT, etc.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to