Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.95.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Currently, the compaction selection is pluggable. It will be great to make the compaction algorithm pluggable too so that we can implement and play with other compaction algorithms.

      1. trunk-7516_v3.2.patch
        80 kB
        Jimmy Xiang
      2. trunk-7516_v3.1.patch
        80 kB
        Jimmy Xiang
      3. trunk-7516_v3.patch
        80 kB
        Jimmy Xiang
      4. trunk-7516_v2.patch
        83 kB
        Jimmy Xiang
      5. trunk-7516.patch
        82 kB
        Jimmy Xiang
      6. HBASE-7516-v2.patch
        34 kB
        Sergey Shelukhin
      7. HBASE-7516-v1.patch
        21 kB
        Sergey Shelukhin
      8. HBASE-7516-v0.patch
        33 kB
        Sergey Shelukhin

        Issue Links

          Activity

          Hide
          stack added a comment -

          Marking closed.

          Show
          stack added a comment - Marking closed.
          Hide
          Sergey Shelukhin added a comment -

          When we were discussing it there wasn't any; the final trunk design is definitely too intrusive... maybe this JIRA separately may make sense.

          Show
          Sergey Shelukhin added a comment - When we were discussing it there wasn't any; the final trunk design is definitely too intrusive... maybe this JIRA separately may make sense.
          Hide
          Liu Shaohui added a comment -

          Any plan to backport this issue to 0.94?

          Show
          Liu Shaohui added a comment - Any plan to backport this issue to 0.94?
          Hide
          Hudson added a comment -

          Integrated in HBase-TRUNK #3830 (See https://builds.apache.org/job/HBase-TRUNK/3830/)
          HBASE-7516 Make compaction policy pluggable (Revision 1440737)

          Result = FAILURE
          jxiang :
          Files :

          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactionTool.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Compactor.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CompactionPolicy.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/Compactor.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/DefaultCompactionPolicy.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/DefaultCompactor.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/HFileReadWriteTest.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompaction.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultCompactSelection.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java
          Show
          Hudson added a comment - Integrated in HBase-TRUNK #3830 (See https://builds.apache.org/job/HBase-TRUNK/3830/ ) HBASE-7516 Make compaction policy pluggable (Revision 1440737) Result = FAILURE jxiang : Files : /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactionTool.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Compactor.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CompactionPolicy.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/Compactor.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/DefaultCompactionPolicy.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/DefaultCompactor.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/HFileReadWriteTest.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompaction.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultCompactSelection.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java
          Hide
          Hudson added a comment -

          Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #385 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/385/)
          HBASE-7516 Make compaction policy pluggable (Revision 1440737)

          Result = FAILURE
          jxiang :
          Files :

          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactionTool.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Compactor.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CompactionPolicy.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/Compactor.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/DefaultCompactionPolicy.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/DefaultCompactor.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/HFileReadWriteTest.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompaction.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultCompactSelection.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java
          Show
          Hudson added a comment - Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #385 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/385/ ) HBASE-7516 Make compaction policy pluggable (Revision 1440737) Result = FAILURE jxiang : Files : /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactionTool.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Compactor.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CompactionPolicy.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/Compactor.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/DefaultCompactionPolicy.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/DefaultCompactor.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/HFileReadWriteTest.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompaction.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultCompactSelection.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java
          Hide
          Jimmy Xiang added a comment -

          Integrated into trunk. Thanks all for reviewing it.

          Show
          Jimmy Xiang added a comment - Integrated into trunk. Thanks all for reviewing it.
          Hide
          Sergey Shelukhin added a comment -

          +1

          Show
          Sergey Shelukhin added a comment - +1
          Hide
          Ted Yu added a comment -

          +1

          Show
          Ted Yu added a comment - +1
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12567212/trunk-7516_v3.2.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 12 new or modified tests.

          +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 lineLengths. The patch does not introduce lines longer than 100

          +1 core tests. The patch passed unit tests in .

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4262//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4262//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4262//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4262//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4262//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4262//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4262//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4262//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12567212/trunk-7516_v3.2.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 12 new or modified tests. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. +1 javadoc . The javadoc tool did not generate any warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4262//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4262//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4262//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4262//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4262//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4262//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4262//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4262//console This message is automatically generated.
          Hide
          Jimmy Xiang added a comment -

          Addressed Ted and Sergey's comments.

          Show
          Jimmy Xiang added a comment - Addressed Ted and Sergey's comments.
          Hide
          Sergey Shelukhin added a comment -

          + String className = family.getConfigurationValue(COMPACTION_POLICY_KEY);
          this is actually not necessary since the family setting is already included in HStore compound configuration

          Show
          Sergey Shelukhin added a comment - + String className = family.getConfigurationValue(COMPACTION_POLICY_KEY); this is actually not necessary since the family setting is already included in HStore compound configuration
          Hide
          Ted Yu added a comment -
          Show
          Ted Yu added a comment - Can you fix the findbugs warning ? You can use the following as baseline: https://builds.apache.org/job/PreCommit-HBASE-Build/4256/artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.xml Thanks
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12567171/trunk-7516_v3.1.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 12 new or modified tests.

          +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          -1 findbugs. The patch appears to introduce 1 new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 lineLengths. The patch does not introduce lines longer than 100

          +1 core tests. The patch passed unit tests in .

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4257//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4257//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4257//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4257//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4257//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4257//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4257//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4257//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12567171/trunk-7516_v3.1.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 12 new or modified tests. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. +1 javadoc . The javadoc tool did not generate any warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. -1 findbugs . The patch appears to introduce 1 new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4257//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4257//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4257//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4257//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4257//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4257//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4257//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4257//console This message is automatically generated.
          Hide
          Jimmy Xiang added a comment -

          Rebased to trunk latest.

          Show
          Jimmy Xiang added a comment - Rebased to trunk latest.
          Hide
          Jimmy Xiang added a comment -

          try hadoop-qa again

          Show
          Jimmy Xiang added a comment - try hadoop-qa again
          Hide
          Ted Yu added a comment -

          Can you rebase that patch ?

          Reversed (or previously applied) patch detected! Assume -R? [n]
          Apply anyway? [n]
          Skipping patch.
          1 out of 1 hunk ignored – saving rejects to file hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Compactor.java.rej

          Show
          Ted Yu added a comment - Can you rebase that patch ? Reversed (or previously applied) patch detected! Assume -R? [n] Apply anyway? [n] Skipping patch. 1 out of 1 hunk ignored – saving rejects to file hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Compactor.java.rej
          Hide
          Jimmy Xiang added a comment -

          trunk-7516_v3.patch is the latest patch.

          Show
          Jimmy Xiang added a comment - trunk-7516_v3.patch is the latest patch.
          Hide
          Ted Yu added a comment -

          @Jimmy:
          Can you attach the latest patch here ?

          Thanks

          Show
          Ted Yu added a comment - @Jimmy: Can you attach the latest patch here ? Thanks
          Hide
          Jimmy Xiang added a comment -

          Since HBASE-7571 is in, I will rebase and post another patch.

          Show
          Jimmy Xiang added a comment - Since HBASE-7571 is in, I will rebase and post another patch.
          Hide
          Sergey Shelukhin added a comment -

          btw, I don't know if this also needs a committer +1, my +1 is only half +1

          Show
          Sergey Shelukhin added a comment - btw, I don't know if this also needs a committer +1, my +1 is only half +1
          Hide
          Sergey Shelukhin added a comment -

          latest patch looks good, except the metadata setting can now be cleaned up, HBASE-7571 went in

          Show
          Sergey Shelukhin added a comment - latest patch looks good, except the metadata setting can now be cleaned up, HBASE-7571 went in
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12566412/trunk-7516_v2.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 12 new or modified tests.

          +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          -1 findbugs. The patch appears to introduce 2 new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 lineLengths. The patch does not introduce lines longer than 100

          +1 core tests. The patch passed unit tests in .

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4170//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4170//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4170//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4170//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4170//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4170//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4170//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4170//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12566412/trunk-7516_v2.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 12 new or modified tests. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. +1 javadoc . The javadoc tool did not generate any warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. -1 findbugs . The patch appears to introduce 2 new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4170//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4170//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4170//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4170//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4170//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4170//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4170//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4170//console This message is automatically generated.
          Hide
          Jimmy Xiang added a comment -

          Sure, run hadoop-qa with it.

          Show
          Jimmy Xiang added a comment - Sure, run hadoop-qa with it.
          Hide
          Sergey Shelukhin added a comment -

          Can you also attach it here for Hadoop QA run?

          Show
          Sergey Shelukhin added a comment - Can you also attach it here for Hadoop QA run?
          Hide
          Jimmy Xiang added a comment -

          I posted the second patch on RB: https://reviews.apache.org/r/9044/.

          Show
          Jimmy Xiang added a comment - I posted the second patch on RB: https://reviews.apache.org/r/9044/ .
          Hide
          Jimmy Xiang added a comment -

          Thanks a lot for the review, I will address your comment in next patch.

          Show
          Jimmy Xiang added a comment - Thanks a lot for the review, I will address your comment in next patch.
          Hide
          Sergey Shelukhin added a comment -

          btw, my review is on /r/

          Show
          Sergey Shelukhin added a comment - btw, my review is on /r/
          Hide
          Sergey Shelukhin added a comment -

          This code will also have to change depending on compation algo, in particular for level.

                    // exclude all files older than the newest file we're currently
                    // compacting. this allows us to preserve contiguity (HBASE-2856)
                    StoreFile last = filesCompacting.get(filesCompacting.size() - 1);
                    int idx = candidates.indexOf(last);
                    Preconditions.checkArgument(idx != -1);
                    candidates.subList(0, idx + 1).clear();
                  }

          I will try to get a prototype patch for store files into the refactor jira by tomorrow, tomorrow we can discuss the necessary changes

          Show
          Sergey Shelukhin added a comment - This code will also have to change depending on compation algo, in particular for level. // exclude all files older than the newest file we're currently // compacting. this allows us to preserve contiguity (HBASE-2856) StoreFile last = filesCompacting.get(filesCompacting.size() - 1); int idx = candidates.indexOf(last); Preconditions.checkArgument(idx != -1); candidates.subList(0, idx + 1).clear(); } I will try to get a prototype patch for store files into the refactor jira by tomorrow, tomorrow we can discuss the necessary changes
          Hide
          Sergey Shelukhin added a comment -

          I took a cursory look; this patch makes compactor also pluggable and makes it return multiple files, and moves the default compaction policy stuff off the base class.
          If compactionPolicy returns, I wonder if (while Compactor is separate for reuse) it makes sense to make compactionPolicy interface simpler, and just let it compact. E.g. if Store has no default selection stuff anymore it doesn't make sense for it to get selection and feed it into compactor, right?
          Then, does it make sense to change CP interface to be called for all files post compaction instead of one file? I am not sure what use-cases it has, but there's no way to tell apart different compaction algorithms.

          My main concern is that this patch does not allow us to implement level compaction as described (see txt in level compaction issue). We can implement different algos which will allow for gradual compaction at the cost of IO, but not level algorithm, because that will remove the file ordering by seqNum, break the heuristic for determining mid-point for split, and other things.

          Show
          Sergey Shelukhin added a comment - I took a cursory look; this patch makes compactor also pluggable and makes it return multiple files, and moves the default compaction policy stuff off the base class. If compactionPolicy returns, I wonder if (while Compactor is separate for reuse) it makes sense to make compactionPolicy interface simpler, and just let it compact. E.g. if Store has no default selection stuff anymore it doesn't make sense for it to get selection and feed it into compactor, right? Then, does it make sense to change CP interface to be called for all files post compaction instead of one file? I am not sure what use-cases it has, but there's no way to tell apart different compaction algorithms. My main concern is that this patch does not allow us to implement level compaction as described (see txt in level compaction issue). We can implement different algos which will allow for gradual compaction at the cost of IO, but not level algorithm, because that will remove the file ordering by seqNum, break the heuristic for determining mid-point for split, and other things.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12565819/trunk-7516.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 12 new or modified tests.

          +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          -1 findbugs. The patch appears to introduce 4 new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 lineLengths. The patch does not introduce lines longer than 100

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.master.TestCatalogJanitor
          org.apache.hadoop.hbase.regionserver.TestBlocksScanned
          org.apache.hadoop.hbase.regionserver.TestResettingCounters
          org.apache.hadoop.hbase.regionserver.TestScanWithBloomError
          org.apache.hadoop.hbase.regionserver.TestColumnSeeking
          org.apache.hadoop.hbase.regionserver.TestHBase7051
          org.apache.hadoop.hbase.regionserver.TestSplitTransaction
          org.apache.hadoop.hbase.filter.TestColumnPrefixFilter
          org.apache.hadoop.hbase.regionserver.TestDefaultCompactSelection
          org.apache.hadoop.hbase.client.TestIntraRowPagination
          org.apache.hadoop.hbase.filter.TestDependentColumnFilter
          org.apache.hadoop.hbase.coprocessor.TestRegionObserverStacking
          org.apache.hadoop.hbase.regionserver.TestHRegionInfo
          org.apache.hadoop.hbase.filter.TestMultipleColumnPrefixFilter
          org.apache.hadoop.hbase.regionserver.TestKeepDeletes
          org.apache.hadoop.hbase.regionserver.TestMinVersions
          org.apache.hadoop.hbase.filter.TestFilter
          org.apache.hadoop.hbase.regionserver.TestScanner
          org.apache.hadoop.hbase.regionserver.TestWideScanner
          org.apache.hadoop.hbase.coprocessor.TestCoprocessorInterface

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4112//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4112//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4112//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4112//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4112//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4112//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4112//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4112//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4112//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12565819/trunk-7516.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 12 new or modified tests. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. +1 javadoc . The javadoc tool did not generate any warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. -1 findbugs . The patch appears to introduce 4 new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.master.TestCatalogJanitor org.apache.hadoop.hbase.regionserver.TestBlocksScanned org.apache.hadoop.hbase.regionserver.TestResettingCounters org.apache.hadoop.hbase.regionserver.TestScanWithBloomError org.apache.hadoop.hbase.regionserver.TestColumnSeeking org.apache.hadoop.hbase.regionserver.TestHBase7051 org.apache.hadoop.hbase.regionserver.TestSplitTransaction org.apache.hadoop.hbase.filter.TestColumnPrefixFilter org.apache.hadoop.hbase.regionserver.TestDefaultCompactSelection org.apache.hadoop.hbase.client.TestIntraRowPagination org.apache.hadoop.hbase.filter.TestDependentColumnFilter org.apache.hadoop.hbase.coprocessor.TestRegionObserverStacking org.apache.hadoop.hbase.regionserver.TestHRegionInfo org.apache.hadoop.hbase.filter.TestMultipleColumnPrefixFilter org.apache.hadoop.hbase.regionserver.TestKeepDeletes org.apache.hadoop.hbase.regionserver.TestMinVersions org.apache.hadoop.hbase.filter.TestFilter org.apache.hadoop.hbase.regionserver.TestScanner org.apache.hadoop.hbase.regionserver.TestWideScanner org.apache.hadoop.hbase.coprocessor.TestCoprocessorInterface Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4112//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4112//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4112//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4112//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4112//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4112//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4112//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4112//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4112//console This message is automatically generated.
          Hide
          Jimmy Xiang added a comment -

          I put my patch on RB https://reviews.apache.org/r/9044/. It is very different from Sergey's patch. Instead just making the configuration pluggable, I made the compactor part of the policy. I assume the configuration is tightly related to the compactor for each compaction algorithm.

          Show
          Jimmy Xiang added a comment - I put my patch on RB https://reviews.apache.org/r/9044/ . It is very different from Sergey's patch. Instead just making the configuration pluggable, I made the compactor part of the policy. I assume the configuration is tightly related to the compactor for each compaction algorithm.
          Hide
          Jimmy Xiang added a comment -

          Attached trunk-7516.patch, which supports level compaction and other pluggable compaction policies.

          Show
          Jimmy Xiang added a comment - Attached trunk-7516.patch, which supports level compaction and other pluggable compaction policies.
          Hide
          Sergey Shelukhin added a comment -

          Er, in HBASE-7519

          Show
          Sergey Shelukhin added a comment - Er, in HBASE-7519
          Hide
          Sergey Shelukhin added a comment -

          Attached notes in the parent, the first section (General notes) is relevant

          Show
          Sergey Shelukhin added a comment - Attached notes in the parent, the first section (General notes) is relevant
          Hide
          Sergey Shelukhin added a comment -

          Please see comments above...
          For level compactions even things like store file management will have to be pluggable.
          For many possible compaction algorithms on the other hand this change is sufficient.
          We can build on top of it for leveldb; I think that on top of this change, there should be an ability to replace CompactionPolicy/Compactor/etc. together. Including, for example for tiered algorithm, keeping standard compactor and replacing the policy. Or the other way around. First JIRA for that is HBASE-7603.

          Show
          Sergey Shelukhin added a comment - Please see comments above... For level compactions even things like store file management will have to be pluggable. For many possible compaction algorithms on the other hand this change is sufficient. We can build on top of it for leveldb; I think that on top of this change, there should be an ability to replace CompactionPolicy/Compactor/etc. together. Including, for example for tiered algorithm, keeping standard compactor and replacing the policy. Or the other way around. First JIRA for that is HBASE-7603 .
          Hide
          Jimmy Xiang added a comment -

          When I filed this Jira, I meant to support pluggable compactions like level compaction.
          I think we don't need two levels of plugs for compaction, which may cause confusion.

          Show
          Jimmy Xiang added a comment - When I filed this Jira, I meant to support pluggable compactions like level compaction. I think we don't need two levels of plugs for compaction, which may cause confusion.
          Hide
          Sergey Shelukhin added a comment -

          Hi. Any objections to this? I will create another JIRA for preparation for LevelDB compactions.

          Show
          Sergey Shelukhin added a comment - Hi. Any objections to this? I will create another JIRA for preparation for LevelDB compactions.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12564792/HBASE-7516-v2.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          -1 findbugs. The patch appears to introduce 1 new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 lineLengths. The patch introduces lines longer than 100

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.util.TestRegionSplitter
          org.apache.hadoop.hbase.client.TestFromClientSideWithCoprocessor

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4012//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4012//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4012//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4012//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4012//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4012//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4012//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4012//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4012//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12564792/HBASE-7516-v2.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 new or modified tests. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. +1 javadoc . The javadoc tool did not generate any warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. -1 findbugs . The patch appears to introduce 1 new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 lineLengths . The patch introduces lines longer than 100 -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.util.TestRegionSplitter org.apache.hadoop.hbase.client.TestFromClientSideWithCoprocessor Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4012//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4012//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4012//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4012//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4012//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4012//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4012//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4012//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4012//console This message is automatically generated.
          Hide
          Sergey Shelukhin added a comment -

          *custom, not default, configuration. How do I get to edit comments?

          Show
          Sergey Shelukhin added a comment - *custom, not default, configuration. How do I get to edit comments?
          Hide
          Sergey Shelukhin added a comment - - edited

          Renamed StoreConfiguration to make it more clear. Its main purpose is basically to avoid passing a very large Store interface for things that only need to ask for a couple of numbers.
          Removed external visibility of compaction configuration. While it is convenient, having base policy aware of custom configuration muddies that interfaces, and there's no real reason for it to see derived config. Policies can handle their private config however they desire.
          Moved the reflection code into ReflectionUtils.
          Some comment cleanup.

          Show
          Sergey Shelukhin added a comment - - edited Renamed StoreConfiguration to make it more clear. Its main purpose is basically to avoid passing a very large Store interface for things that only need to ask for a couple of numbers. Removed external visibility of compaction configuration. While it is convenient, having base policy aware of custom configuration muddies that interfaces, and there's no real reason for it to see derived config. Policies can handle their private config however they desire. Moved the reflection code into ReflectionUtils. Some comment cleanup.
          Hide
          Sergey Shelukhin added a comment -

          Ok, there will be no patch today. I need to sleep on the issue some more, I don't like all these configuration classes.

          Show
          Sergey Shelukhin added a comment - Ok, there will be no patch today. I need to sleep on the issue some more, I don't like all these configuration classes.
          Show
          Sergey Shelukhin added a comment - Link: http://leveldb.googlecode.com/svn/trunk/doc/impl.html
          Hide
          Sergey Shelukhin added a comment -

          I have left some comments in the /r/ and will come up with updated patch today.
          With regard to level compaction and making more things pluggable.
          Initially I just thought I will make Compactor pluggable and change its interface a bit (e.g. make it produce multiple files).
          However, as I see now, with full leveldb compaction algorithm (with levels and overlapping ranges across levels), it will be insufficient - we'd need to change a some logic in HStore. Around compactions, obviously; storefiles list will also be suboptimal (all LevelDB algorithms work with levels, each of which except one have non-overlapping ranges; so selection from the list, while doable, is not pretty; various checks/logic relying on one sequence of files no longer make sense at all); also, StoreScanner file selection for scans will be different if we use ranges (ditto for gets). Other things may change too.

          We can either fully refactor StoreFile management out or HStore (tempting), or do minimum abstraction when we already have implementation in mind.
          In either case it will be in separate JIRA. I can try to do the former in another JIRA.

          Show
          Sergey Shelukhin added a comment - I have left some comments in the /r/ and will come up with updated patch today. With regard to level compaction and making more things pluggable. Initially I just thought I will make Compactor pluggable and change its interface a bit (e.g. make it produce multiple files). However, as I see now, with full leveldb compaction algorithm (with levels and overlapping ranges across levels), it will be insufficient - we'd need to change a some logic in HStore. Around compactions, obviously; storefiles list will also be suboptimal (all LevelDB algorithms work with levels, each of which except one have non-overlapping ranges; so selection from the list, while doable, is not pretty; various checks/logic relying on one sequence of files no longer make sense at all); also, StoreScanner file selection for scans will be different if we use ranges (ditto for gets). Other things may change too. We can either fully refactor StoreFile management out or HStore (tempting), or do minimum abstraction when we already have implementation in mind. In either case it will be in separate JIRA. I can try to do the former in another JIRA.
          Hide
          Sergey Shelukhin added a comment -

          TestLocalHBaseCluster appears to fail in all JIRAs. Will remove lines longer than 100 as part of next review iteration.

          Show
          Sergey Shelukhin added a comment - TestLocalHBaseCluster appears to fail in all JIRAs. Will remove lines longer than 100 as part of next review iteration.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12563870/HBASE-7516-v1.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 6 new or modified tests.

          +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 lineLengths. The patch introduces lines longer than 100

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.replication.TestReplication
          org.apache.hadoop.hbase.TestLocalHBaseCluster

          -1 core zombie tests. There are 8 zombie test(s): at org.apache.hadoop.hbase.master.TestMasterFailover.testMasterFailoverWithMockedRITOnDeadRS(TestMasterFailover.java:833)
          at org.apache.hadoop.hbase.catalog.TestCatalogTracker.testServerNotRunningIOException(TestCatalogTracker.java:250)

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/3941//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3941//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3941//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3941//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3941//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3941//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3941//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3941//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/3941//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12563870/HBASE-7516-v1.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 6 new or modified tests. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. +1 javadoc . The javadoc tool did not generate any warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 lineLengths . The patch introduces lines longer than 100 -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.replication.TestReplication org.apache.hadoop.hbase.TestLocalHBaseCluster -1 core zombie tests . There are 8 zombie test(s): at org.apache.hadoop.hbase.master.TestMasterFailover.testMasterFailoverWithMockedRITOnDeadRS(TestMasterFailover.java:833) at org.apache.hadoop.hbase.catalog.TestCatalogTracker.testServerNotRunningIOException(TestCatalogTracker.java:250) Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/3941//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3941//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3941//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3941//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3941//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3941//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3941//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3941//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/3941//console This message is automatically generated.
          Show
          Sergey Shelukhin added a comment - https://reviews.apache.org/r/8895/
          Hide
          Sergey Shelukhin added a comment -

          Renames, javadoc... will post to /r/ momentarily.

          Show
          Sergey Shelukhin added a comment - Renames, javadoc... will post to /r/ momentarily.
          Hide
          Jimmy Xiang added a comment -

          Not a big deal, but wondering, can we call the existing policy DefaultCompactionPolicy or something else, and make CompactionPolicy the actual base?

          -  CompactionPolicy compactionPolicy;
          +  CompactionPolicyBase<?> compactionPolicy;
          

          The compactionPolicy has two abstract methods. How do you expect them to be used? Where will the actually compaction algorithm logic be if applyCompactionPolicy returns a CompactSelection? What's a CfgType?

          +  protected abstract CfgType createCompactionConfiguration(Configuration configuration,
          +    StoreConfiguration storeConfig);
          +
          +  protected abstract CompactSelection applyCompactionPolicy(CompactSelection candidates)
          +    throws IOException;
          

          Some javadoc will be very helpful. By the way, should we use RB for the next patch?

          Sergey Shelukhin, thanks a lot for taking this issue.

          Show
          Jimmy Xiang added a comment - Not a big deal, but wondering, can we call the existing policy DefaultCompactionPolicy or something else, and make CompactionPolicy the actual base? - CompactionPolicy compactionPolicy; + CompactionPolicyBase<?> compactionPolicy; The compactionPolicy has two abstract methods. How do you expect them to be used? Where will the actually compaction algorithm logic be if applyCompactionPolicy returns a CompactSelection? What's a CfgType? + protected abstract CfgType createCompactionConfiguration(Configuration configuration, + StoreConfiguration storeConfig); + + protected abstract CompactSelection applyCompactionPolicy(CompactSelection candidates) + throws IOException; Some javadoc will be very helpful. By the way, should we use RB for the next patch? Sergey Shelukhin , thanks a lot for taking this issue.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12563792/HBASE-7516-v0.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 6 new or modified tests.

          +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 lineLengths. The patch does not introduce lines longer than 100

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.regionserver.TestSplitTransaction

          -1 core zombie tests. There are 1 zombie test(s):

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/3931//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3931//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3931//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3931//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3931//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3931//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3931//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3931//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/3931//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12563792/HBASE-7516-v0.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 6 new or modified tests. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. +1 javadoc . The javadoc tool did not generate any warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.regionserver.TestSplitTransaction -1 core zombie tests . There are 1 zombie test(s): Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/3931//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3931//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3931//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3931//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3931//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3931//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3931//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/3931//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/3931//console This message is automatically generated.
          Hide
          Sergey Shelukhin added a comment -

          Extracted from 7055

          Show
          Sergey Shelukhin added a comment - Extracted from 7055
          Hide
          Sergey Shelukhin added a comment -

          Oh, I see. Pluggable can be done by extracting part of HBASE-7055 patch; configurable per table would need HBASE-7236

          Show
          Sergey Shelukhin added a comment - Oh, I see. Pluggable can be done by extracting part of HBASE-7055 patch; configurable per table would need HBASE-7236
          Hide
          Sergey Shelukhin added a comment -
          Show
          Sergey Shelukhin added a comment - See HBASE-7055 , HBASE-7236

            People

            • Assignee:
              Jimmy Xiang
              Reporter:
              Jimmy Xiang
            • Votes:
              0 Vote for this issue
              Watchers:
              11 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development