hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "stack (JIRA)" <j...@apache.org>
Subject [jira] Commented: (HBASE-483) Merge tool won't merge two overlapping regions
Date Wed, 19 Mar 2008 20:18:24 GMT

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

stack commented on HBASE-483:
-----------------------------

Looks great.  A few notes:

Below works on windows?
{code}
Index: src/test/org/apache/hadoop/hbase/util/TestMigrate.java
===================================================================
--- src/test/org/apache/hadoop/hbase/util/TestMigrate.java  (revision 638708)
+++ src/test/org/apache/hadoop/hbase/util/TestMigrate.java  (working copy)
@@ -96,11 +96,11 @@

           // this path is for running test with ant

-          "../../../../../src/contrib/hbase/src/testdata/HADOOP-2478-testdata.zip")
+          "../../../src/testdata/HADOOP-2478-testdata.zip")
{code}

Why not remove the below from same file?

Why in +  public TestMergeTool() { do you do some setup in constructor rather than go by the
usual idiom of putting up (all of) the environment to test in the 'setup' method? (Great test,
by the way).

If you look at your patch, it completely removes HSF and then readds it.  I can't see whats
changed.

Want to fix the class comment on HMerge?

I like the way its done -- moving mapfiles and then running compactor.  I like that we're
reusing code.

This code belongs in a method of its own -- perhaps in a utility class somewhere?  It should
be in a method of its own because it acts on intimate knowledge of the region dir layout and
it should be in util because I wouldn't be surprised we'd want to make use of it again outside
of this current context.

{code}
+      fs.mkdirs(HStoreFile.getMapDir(basedir, encodedRegionName,
+          colFamily));
+      fs.mkdirs(HStoreFile.getInfoDir(basedir, encodedRegionName,
+          colFamily));
+      if (tabledesc.families().get(new Text(colFamily + ":")).getBloomFilter()
+          != null) {
+        fs.mkdirs(HStoreFile.getFilterDir(basedir, encodedRegionName,
+            colFamily));
+      }
{code}

Does this method not already exist up in hadoop?

{code}
private static void listPaths
{code}

Why the '\n's in this message: +      LOG.debug("\n\n\nOpening region: " + info);  Ain't that
going to be annoying?  Capitalize rather than '\n\n\n'... i'd say.

Merge needs a class comment (Needs to be distingushed from HMerge -- should we remove HMerge?).

This is unnecessary in the JenkinsHash main: +    System.exit(0);

MetaUtils class javadoc should be HBase Meta/Catalog tables?

Verifying that hbase is not running should be moved out of MetaUtil?  Other tools will need
this facility too.

The below is a path in hdfs?  You might want to do it in the users home directory -- you can
get that from fs IIRC -- rather than at top-level to avoid permission problems in hdfs.  If
its local filesystem, add name of the program to the name I'd say so that folks who are looking
/tmp can have some idea where these files are coming from.... if they haven't been cleaned
up properly (Should we have a tmp dir in the hbase.rootdir for doing stuff like this?)
{code}
+      this.log = new HLog(this.fs, 
+          new Path("/tmp", HConstants.HREGION_LOGDIR_NAME + "_" +
+              System.currentTimeMillis()
+          ),
{code}

Why this line in Migrate? Its not needed, right?  Or if it is, then something needs to be
documented here.

Good stuff





> Merge tool won't merge two overlapping regions
> ----------------------------------------------
>
>                 Key: HBASE-483
>                 URL: https://issues.apache.org/jira/browse/HBASE-483
>             Project: Hadoop HBase
>          Issue Type: Task
>    Affects Versions: 0.1.0
>            Reporter: stack
>            Assignee: Jim Kellerman
>            Priority: Blocker
>             Fix For: 0.1.0
>
>         Attachments: patch.txt, subset_of_meta.txt
>
>
> hbase-480 adds a merge tool. Seems to work for two adjacent regions in tests but not
for the case where we've made two overlapping regions: i.e. both have a start key of 'A' but
one regions endkey is less than the end key of the others (See up on Lars cluster hbase-471).
 I ran the merge and it made a new region with the right start and end keys but there was
nothing in it.
> We need this tool to do fix up when hbase goes awry.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message