[jira] [Commented] (HBASE-4081) Issues with HRegion.compactStores methods

["Ted Yu (JIRA)" <ji...@apache.org>]

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

Ted Yu commented on HBASE-4081:
-------------------------------

It doesn't hurt to keep the return value.

According to javadoc:
{code}
return the split key of the first store that needs to be split
{code}
I think call to s.CheckSplit() should be kept. Instead of returning immediately, we can continue iterating Store's. If we want to honor the above contract, splitRow should be declared outside for loop and used to remember the split key of the first store.
Since no one is using the return value, we can return the split key of the last store and modify javadoc accordingly.


> 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