Mailing-List: contact issues-help@hbase.apache.org; run by ezmlm
Precedence: bulk
Date: Sat, 9 Jul 2011 04:18:18 +0000 (UTC)
From: "stack (JIRA)" <jira@apache.org>
To: issues@hbase.apache.org
Message-ID: 
 <1555746218.13690.1310185098121.JavaMail.tomcat@hel.zones.apache.org>
In-Reply-To: 
 <361029769.13003.1310168176892.JavaMail.tomcat@hel.zones.apache.org>
Subject: [jira] [Commented] (HBASE-4081) Issues with HRegion.compactStores
 methods
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


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

stack commented on HBASE-4081:
------------------------------

@Ming Ma I see Ted's point (as long as the javadoc is updated) but I could also go your way.  As is the method is 'messy' and its method name misleads -- and if no one is using the split return anyways, then, it would make sense to clean it up (This code smells of an incomplete refactoring).

> Issues with HRegion.compactStores methods
> -----------------------------------------
>
>                 Key: HBASE-4081
>                 URL: https://issues.apache.org/jira/browse/HBASE-4081
>             Project: HBase
>          Issue Type: Bug
>          Components: regionserver
>            Reporter: Ming Ma
>            Assignee: Ming Ma
>
> HRegion.java,
>   byte [] compactStores(final boolean majorCompaction)
>   throws IOException {
>     if (majorCompaction) {
>       this.triggerMajorCompaction();
>     }
>     return compactStores();
>   }
>   /**
>    * Compact all the stores and return the split key of the first store that needs
>    * to be split.
>    */
>   public byte[] compactStores() throws IOException {
>     for(Store s : getStores().values()) {
>       CompactionRequest cr = s.requestCompaction();
>       if(cr != null) {
>         try {
>           compact(cr);
>         } finally {
>           s.finishRequest(cr);
>         }
>       }
>       byte[] splitRow = s.checkSplit();
>       if (splitRow != null) {
>         return splitRow;
>       }
>     }
>     return null;
>   }
> 1. It seems the second method's intention is to compact all the stores. However, if a store requires split, the process will stop.
> 2. Only MetaUtils, HRegion.merge, HRegion.processTable use these two methods. No caller uses the return value.
> 3. HRegion.merge expects major compaction for each store after the call and has code like below to check error condition.
>       // Because we compacted the source regions we should have no more than two
>       // HStoreFiles per family and there will be no reference store
>       if (srcFiles.size() == 2)
> So it seems like the fixes are: a) take out s.CheckSplit() call inside compactStores. b) make the return type "void" for these two compactStores functions.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira