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

stack commented on HBASE-7704:
------------------------------

license

Extra 'a' in "in the a given directory"

Would suggest you add to the class comment how to run it.

Does "./bin/hbase org.apache.hadoop.hbase.util.HFileV1Detector --h" work?

'--h' is not right.  gnu long form would be '--help'.  Java usual is '-h'.  
Pick one or the other (preferably support both) but not '--h'... no one will 
guess that as way to get help.

Suggest you show us what '-h' output looks like here in this issue:

Suggest it return -1 if hfilev1 files found: +    return 0;

It is odd that you create the executor in the run and do not then pass it to 
the methods that use it -- rather they presume it set as a data member.

Use GnuParser rather than PosixParser

This is good:

+      System.out
+          .println("In case no option is provided, it process hbase.rootdir 
with 10 threads.");

But 'processes' rather tha 'process' and 'using' rather than 'with'

What will you ignore?

+        LOG.info("Ignoring path: " + fsStat.getPath());

.logs etc?

Shouldn't be info level.

You keep saying you are going to print regions that have v1 files but you seem 
to be printing out full path

+          System.out.println("Region has a hfile v1: " + f.get());

Don't need that 'Region has a hfile v1' prefix

Suggest you list what the output looks like here in this issue.  Will help give 
you feedback.

Suggest an option that will fast fail... fail as soon as it finds the first v1. 
 This is probably not important so if it takes a while, just punt.

Don't need this

+      System.out.println("Table " + p + " has no HFileV1.");

and probably not this...

+      System.out.println("Table " + p + " has "+hfileV1Set.size()+" number of 
HFileV1.");


What is this about?

+                LOG.warn("Found a v1 hfile, " + storeFilePath);
+                System.out.println("Found a v1 hfile, " + storeFilePath);

A warn and a print?  And don't you print out v1 files found at a higher level?

Why not include original exception here: +              throw new 
IOException("Unknown version for hfile: " + storeFilePath);?

It looks like you do fail fast -- you stop scanning a family as soon as you 
find a v1 file?

Drop this kind of stuff:

+    System.out.println("==================Regions to Major 
Compact==============");








                
> migration tool that checks presence of HFile V1 files
> -----------------------------------------------------
>
>                 Key: HBASE-7704
>                 URL: https://issues.apache.org/jira/browse/HBASE-7704
>             Project: HBase
>          Issue Type: Task
>            Reporter: Ted Yu
>            Assignee: Himanshu Vashishtha
>            Priority: Blocker
>             Fix For: 0.95.1
>
>         Attachments: HBase-7704-v1.patch
>
>
> Below was Stack's comment from HBASE-7660:
> Regards the migration 'tool', or 'tool' to check for presence of v1 files, I 
> imagine it as an addition to the hfile tool 
> http://hbase.apache.org/book.html#hfile_tool2 The hfile tool already takes a 
> bunch of args including printing out meta. We could add an option to print 
> out version only – or return 1 if version 1 or some such – and then do a bit 
> of code to just list all hfiles and run this script against each. Could MR it 
> if too many files.

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