hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "jiraposter@reviews.apache.org (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-5616) Make compaction code standalone
Date Fri, 23 Mar 2012 17:49:29 GMT

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

jiraposter@reviews.apache.org commented on HBASE-5616:

bq.  On 2012-03-23 17:32:04, Lars Hofhansl wrote:
bq.  > CompactionTool is a cool idea.
bq.  > It's early, so I don't follow completely why this required moving the compaction
code into a separate class, especially because you need an instance of a store to run its

Store is 2200 lines long.

Need to make a start somewhere breaking up these Store and HRegion, etc., behemoths.  The
tendency otherwise is that they get bigger with time.

Also, arguing its all a ball of wax so we should shut down any attempt at untangling is probably
not what you were reasoning (smile).

My main motive breaking out CompactionTool made is so I could run compactions in a profiler
w/o a bunch of other moving parts in the way.  I did some of that yesterday.  Nothing obviously
dumb going on at first glance.  Requires more study figuring optimizations.

Thanks for the review.

bq.  On 2012-03-23 17:32:04, Lars Hofhansl wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/regionserver/Compactor.java, line 45
bq.  > <https://reviews.apache.org/r/4469/diff/1/?file=95053#file95053line45>
bq.  >
bq.  >     If you need to pass in an instance of Store, what is the advantage?
bq.  >     Could also make it a public static helper method.

I could do that.  If I went this way, it'd be harder to evolve though (public, static passing
in all args).

This review comment makes me want to shutdown access to Compactor even more -- go via CompactionTool
if you need it.   I should add @InterfaceAudience.Private and remove the 'public' from the
Class.  If you are good w/ this, I'll stick up another patch w/ that on it.

bq.  On 2012-03-23 17:32:04, Lars Hofhansl wrote:
bq.  > src/test/java/org/apache/hadoop/hbase/regionserver/CompactionTool.java, line 135
bq.  > <https://reviews.apache.org/r/4469/diff/1/?file=95059#file95059line135>
bq.  >
bq.  >     That would have been possible before separating the compaction code out, right?

Yes.  Would have made no sense since only the one client.

- Michael

This is an automatically generated e-mail. To reply, visit:

On 2012-03-23 17:02:01, Michael Stack wrote:
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/4469/
bq.  -----------------------------------------------------------
bq.  (Updated 2012-03-23 17:02:01)
bq.  Review request for hbase.
bq.  Summary
bq.  -------
bq.  Introduces a standalone CompactionTool under src/test. Can call it from its main
bq.  and have it run compactions on arbitrary storefiles.
bq.  Compaction code is moved out of Store into a new Compactor class.
bq.  CompactionTool needs a Store and a mocked up Region to run because
bq.  compacting uses a StoreScanner (A StoreScanner needs a Store. A
bq.  Store needs an HRegion). Rather than expect to be passed a coherent
bq.  HRegion pointer, instead, we fake up one using Mockito.
bq.  I tried to break out of HRegion a "Region" Interface. This Region Interface
bq.  would have a basic subset of HRegion functionality and we'd pass this
bq.  instead of HRegion to SplitTransaction, Store, CompactionRequest, etc.
bq.  but the change would be massive. Everything expects to be able to do
bq.  anything on an HRegion. This is work we need to do but I"m not doing
bq.  it as part of this patch.
bq.  M src/test/java/org/apache/hadoop/hbase/regionserver/TestCompaction.java
bq.  Use new CompactionTool instead of asking Store directly to compact.
bq.  M src/main/java/org/apache/hadoop/hbase/util/ChecksumType.java
bq.  Formatting.
bq.  M src/main/java/org/apache/hadoop/hbase/util/FSUtils.java
bq.  Format the log message so properly spaced.
bq.  A src/test/java/org/apache/hadoop/hbase/regionserver/CompactionTool.java
bq.  New compactiontool. Runs all compactions.
bq.  M src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
bq.  Make getOpenAndCloseThreadPool static.
bq.  M src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
bq.  Move compaction code out to CompactionTool.
bq.  Refactor so can override ttl and Store home directory so Store
bq.  is more mockable; can now stand up a Store on its own w/o real
bq.  HRegion context.
bq.  M src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java
bq.  Formatting.
bq.  This addresses bug hbase-5616.
bq.      https://issues.apache.org/jira/browse/hbase-5616
bq.  Diffs
bq.  -----
bq.    src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java b52e5d3 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/Compactor.java PRE-CREATION 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 4efdc6b 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/Store.java dcede5a 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CompactionProgress.java
bq.    src/main/java/org/apache/hadoop/hbase/util/ChecksumType.java d2329e1 
bq.    src/main/java/org/apache/hadoop/hbase/util/FSUtils.java 4bfd42f 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/CompactionTool.java PRE-CREATION

bq.    src/test/java/org/apache/hadoop/hbase/regionserver/TestCompaction.java 91ac652 
bq.  Diff: https://reviews.apache.org/r/4469/diff
bq.  Testing
bq.  -------
bq.  Thanks,
bq.  Michael

> Make compaction code standalone
> -------------------------------
>                 Key: HBASE-5616
>                 URL: https://issues.apache.org/jira/browse/HBASE-5616
>             Project: HBase
>          Issue Type: Improvement
>            Reporter: stack
>            Assignee: stack
>         Attachments: 5616.txt, 5616v3.txt, 5616v6.txt, standalone.txt
> This is part of hbase-2462.  Make the compaction code standalone so can run it independent
of hbase.  Will make it easier to profile and try stuff out.

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira


View raw message