HBase
  1. HBase
  2. HBASE-7842

Add compaction policy that explores more storefile groups

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.98.0, 0.95.1
    • Component/s: Compaction
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    • Release Note:
      Default compaction policy has been changed to a new policy that will explore more groups of files and is more strict about enforcing the size ratio requirements.

      Description

      Some workloads that are not as stable can have compactions that are too large or too small using the current storefile selection algorithm.

      Currently:

      • Find the first file that Size(fi) <= Sum(0, i-1, FileSize(fx))
      • Ensure that there are the min number of files (if there aren't then bail out)
      • If there are too many files keep the larger ones.

      I would propose something like:

      • Find all sets of storefiles where every file satisfies
        • FileSize(fi) <= Sum(0, i-1, FileSize(fx))
        • Num files in set =< max
        • Num Files in set >= min
      • Then pick the set of files that maximizes ((# storefiles in set) / Sum(FileSize(fx)))

      The thinking is that the above algorithm is pretty easy reason about, all files satisfy the ratio, and should rewrite the least amount of data to get the biggest impact in seeks.

      1. HBASE-7842-ADD.patch
        2 kB
        Elliott Clark
      2. HBASE-7842-7.patch
        48 kB
        Elliott Clark
      3. HBASE-7842-6.patch
        48 kB
        Elliott Clark
      4. HBASE-7842-5.patch
        8 kB
        Elliott Clark
      5. HBASE-7842-4.patch
        8 kB
        Elliott Clark
      6. HBASE-7842-3.patch
        8 kB
        Elliott Clark
      7. HBASE-7842-2.patch
        8 kB
        Elliott Clark
      8. HBASE-7842-0.patch
        9 kB
        Elliott Clark

        Issue Links

          Activity

          Hide
          Sergey Shelukhin added a comment -

          what I don't understand is the first new condition for every file. E.g. (assume ratio 1) in order 10 7 4 5 is good to compact but 7 10 4 5 is not (if reordered by size while preserving contiguousness, some other example can probably be invented.

          Then, maximizing ratio without regard for number of files can be bad, e.g. files 0..3 with sizes 100 3 2 2 - max files 3, min files 2, assuming "2 2" passes ratio test - we can compact 2..3 for 2/4, or 1..3 for 3/7; 3/7 is less, but we probably want to do it anyway to minimize I/O amplification later.

          Just thinking aloud: what if we use ratio/size filter after we find best set out of all permutations (# of files/sum of sizes)? Presumably, good permutations already won't rewrite a lot of data needlessly, because otherwise they would have large sum of sizes and thus lower preference for being the "best set".
          For example, 10 2 2 2 2 3 2, max files 6 - if we choose size 6 sets, 0..5 gets us 6/21, and 1..6 gets us 6/13, we don't need ratio test for each set to establish that.
          So the only thing we need to do is find if the chosen set itself satisfies some ratio test... probably ratio for biggest file (without regard to ordering of files, e.g. 6 10 6 is good). If it can then ratio is good, it's the best compaction we can do (according to this criteria), if not the best compaction we can do is not good enough.

          Show
          Sergey Shelukhin added a comment - what I don't understand is the first new condition for every file. E.g. (assume ratio 1) in order 10 7 4 5 is good to compact but 7 10 4 5 is not (if reordered by size while preserving contiguousness, some other example can probably be invented. Then, maximizing ratio without regard for number of files can be bad, e.g. files 0..3 with sizes 100 3 2 2 - max files 3, min files 2, assuming "2 2" passes ratio test - we can compact 2..3 for 2/4, or 1..3 for 3/7; 3/7 is less, but we probably want to do it anyway to minimize I/O amplification later. Just thinking aloud: what if we use ratio/size filter after we find best set out of all permutations (# of files/sum of sizes)? Presumably, good permutations already won't rewrite a lot of data needlessly, because otherwise they would have large sum of sizes and thus lower preference for being the "best set". For example, 10 2 2 2 2 3 2, max files 6 - if we choose size 6 sets, 0..5 gets us 6/21, and 1..6 gets us 6/13, we don't need ratio test for each set to establish that. So the only thing we need to do is find if the chosen set itself satisfies some ratio test... probably ratio for biggest file (without regard to ordering of files, e.g. 6 10 6 is good). If it can then ratio is good, it's the best compaction we can do (according to this criteria), if not the best compaction we can do is not good enough.
          Hide
          Sergey Shelukhin added a comment -

          s/If it can then ratio is good/If it does, /

          Show
          Sergey Shelukhin added a comment - s/If it can then ratio is good/If it does, /
          Hide
          Sergey Shelukhin added a comment -

          Back of the napkin calculation tells me that dumb exploration of ALL ordered permutations should be fast, number is given by sum for i in [minfiles, maxfiles] of (numFiles - i + 1), which expands to (if I still remember math) (maxfiles - minfiles + 1)((numFiles + 1) - (maxfiles + minfiles)/2).
          For 20 files, min files 2, max files 7, that's 99 total permutations, which can be generated incrementally.

          Show
          Sergey Shelukhin added a comment - Back of the napkin calculation tells me that dumb exploration of ALL ordered permutations should be fast, number is given by sum for i in [minfiles, maxfiles] of (numFiles - i + 1), which expands to (if I still remember math) (maxfiles - minfiles + 1)((numFiles + 1) - (maxfiles + minfiles)/2). For 20 files, min files 2, max files 7, that's 99 total permutations, which can be generated incrementally.
          Hide
          Elliott Clark added a comment -

          what I don't understand is the first new condition for every file. E.g. (assume ratio 1) in order 10 7 4 5 is good to compact but 7 10 4 5 is not

          We're trying to use size as a way to group files that are similar. If there's a use case that has traffic come in waves we want to group the smaller files up to create larger files before compacting the larger files.

          For something like:

          [100 100 100 50 50 50 50 50 100 100]

          I'd rather have the 50's choosen and then compact the 100's later as they seem more similar and a good way to try and not re-write the same data over and over again.

          You are correct the main point that I want to dive into is that we are not looking at many of the possible groupings right now, and it seems like a deep search would be cheap and could find better compactions.

          One of the things Stack and I thought about was not using the ratio at all for grouping files. Instead using it for deciding if we found a compaction that's good enough.

          Something like:
          ratio = 1.2
          files to compact = 5
          sum of file size = 80
          average store file size (across all files in the store) = 20

          (80 / 20 ) / 5 < 1.2 so yes we compact.

          That's interesting and probably something that I'll try. However I wanted to start with something that's a tweak of the algorithm that we have now and then branch out.

          Back of the napkin calculation tells me that dumb exploration of ALL ordered permutations should be fast

          Yep. The runtime of choosing files isn't something that should be a major concern as long as things are not spiraling out of control.

          Show
          Elliott Clark added a comment - what I don't understand is the first new condition for every file. E.g. (assume ratio 1) in order 10 7 4 5 is good to compact but 7 10 4 5 is not We're trying to use size as a way to group files that are similar. If there's a use case that has traffic come in waves we want to group the smaller files up to create larger files before compacting the larger files. For something like: [100 100 100 50 50 50 50 50 100 100] I'd rather have the 50's choosen and then compact the 100's later as they seem more similar and a good way to try and not re-write the same data over and over again. You are correct the main point that I want to dive into is that we are not looking at many of the possible groupings right now, and it seems like a deep search would be cheap and could find better compactions. One of the things Stack and I thought about was not using the ratio at all for grouping files. Instead using it for deciding if we found a compaction that's good enough. Something like: ratio = 1.2 files to compact = 5 sum of file size = 80 average store file size (across all files in the store) = 20 (80 / 20 ) / 5 < 1.2 so yes we compact. That's interesting and probably something that I'll try. However I wanted to start with something that's a tweak of the algorithm that we have now and then branch out. Back of the napkin calculation tells me that dumb exploration of ALL ordered permutations should be fast Yep. The runtime of choosing files isn't something that should be a major concern as long as things are not spiraling out of control.
          Hide
          Sergey Shelukhin added a comment -

          Yeah, that was my thought above, look at all groups w/o taking ratio into account, find the best compaction, and then judge if its goodness is higher than some cutoff.
          The metric for best and for cutoff can vary...
          sum of file size - the compacting files or all files? I assume of compacting files.

          We're trying to use size as a way to group files that are similar. If there's a use case that has traffic come in waves we want to group the smaller files up to create larger files before compacting the larger files.

          There's no reason why finding similar files should depend on ordering (i.e. looking at sum of next N file sizes in sequence.)
          So 10 7 3 4 - all files satisfy the criteria (I am assuming it's not applied to last file). But in 7 10 3 4 only last two files do.

          Show
          Sergey Shelukhin added a comment - Yeah, that was my thought above, look at all groups w/o taking ratio into account, find the best compaction, and then judge if its goodness is higher than some cutoff. The metric for best and for cutoff can vary... sum of file size - the compacting files or all files? I assume of compacting files. We're trying to use size as a way to group files that are similar. If there's a use case that has traffic come in waves we want to group the smaller files up to create larger files before compacting the larger files. There's no reason why finding similar files should depend on ordering (i.e. looking at sum of next N file sizes in sequence.) So 10 7 3 4 - all files satisfy the criteria (I am assuming it's not applied to last file). But in 7 10 3 4 only last two files do.
          Hide
          Sergey Shelukhin added a comment -

          Sorry, in 7 10 3 4 two pairs do, but only separately.

          Show
          Sergey Shelukhin added a comment - Sorry, in 7 10 3 4 two pairs do, but only separately.
          Hide
          Hadoop QA added a comment -

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

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

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

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4684//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/12572209/HBASE-7842-0.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 4 new or modified tests. -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4684//console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 4 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.TestDefaultCompactSelection

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4686//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4686//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4686//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4686//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4686//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4686//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4686//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4686//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4686//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4686//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/12572222/HBASE-7842-2.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 4 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.TestDefaultCompactSelection Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4686//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4686//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4686//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4686//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4686//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4686//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4686//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4686//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4686//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4686//console This message is automatically generated.
          Hide
          Sergey Shelukhin added a comment -

          Is it to intended to be committed, or experimental? I can review tomorrow

          Show
          Sergey Shelukhin added a comment - Is it to intended to be committed, or experimental? I can review tomorrow
          Hide
          Elliott Clark added a comment -

          Lotta changes in trunk from the last time that I rebased. Lets try this.

          Show
          Elliott Clark added a comment - Lotta changes in trunk from the last time that I rebased. Lets try this.
          Hide
          Elliott Clark added a comment -

          I would like to commit this. I've seen some really nasty compaction behavior lately with what's currently the default.

          Show
          Elliott Clark added a comment - I would like to commit this. I've seen some really nasty compaction behavior lately with what's currently the default.
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 4 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.TestSplitTransactionOnCluster
          org.apache.hadoop.hbase.replication.TestReplicationQueueFailoverCompressed
          org.apache.hadoop.hbase.master.TestMasterFailover

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4689//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4689//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4689//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4689//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4689//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4689//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4689//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4689//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4689//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4689//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/12572226/HBASE-7842-4.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 4 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.TestSplitTransactionOnCluster org.apache.hadoop.hbase.replication.TestReplicationQueueFailoverCompressed org.apache.hadoop.hbase.master.TestMasterFailover Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4689//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4689//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4689//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4689//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4689//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4689//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4689//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4689//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4689//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4689//console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 4 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.replication.TestReplicationQueueFailoverCompressed
          org.apache.hadoop.hbase.master.TestMasterFailover
          org.apache.hadoop.hbase.regionserver.TestSplitTransactionOnCluster

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

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4688//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4688//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4688//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4688//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4688//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4688//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4688//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4688//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4688//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4688//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/12572226/HBASE-7842-4.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 4 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.replication.TestReplicationQueueFailoverCompressed org.apache.hadoop.hbase.master.TestMasterFailover org.apache.hadoop.hbase.regionserver.TestSplitTransactionOnCluster -1 core zombie tests . There are 1 zombie test(s): Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4688//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4688//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4688//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4688//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4688//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4688//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4688//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4688//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4688//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4688//console This message is automatically generated.
          Hide
          Sergey Shelukhin added a comment -
          compactionPolicy = new ExploringCompactionPolicy(this.conf, this.store/*as StoreConfigInfo*/);
          

          HBASE-7935 will make this pluggable separately, I am planning to commit it after running tests (it has 2 +1s).
          Regardless, it appears that this patch both replaces default policy and hijacks the default test, so the default ratio
          algorithm becomes a poor bastard child, not used and not tested Should we delete it altogether and swap it with yours in place?

          I've seen some really nasty compaction behavior lately with what's currently the default.

          Can you add tests for those with new policy? Would also be useful to illustrate.

                  if (potentialMatchFiles.size() > bestSelection.size() ||
                      (potentialMatchFiles.size() == bestSelection.size() && size < bestSize)) {
          

          Interesting heuristic... it looks reasonable, but have you tried ratio proposed above? E.g. getting rid of 3 files of 5Mb each is better than getting rid of 4 files of 500Mb each.

              if (files.size() <= 2) {
                return  true;
              }
          

          Why? What if files are 500 5?

                long sumAllOtherFilesize = 0;
                for (int j =0; j < files.size(); j++) {
                  if (i == j) continue;
                  sumAllOtherFilesize += files.get(j).getReader().length();
                }
          

          Double nested loop is unnecessary. In fact, we already get total size outside; we might as well do it before this check, pass it in, and then we can just substract each file from it in a loop to get size of all other files. Shorter code too

              minFiles = comConf.getMinFilesToCompact();
              maxFiles = comConf.getMaxFilesToCompact();
              minCompactionSize = comConf.getMinCompactSize();
              ratio = comConf.getCompactionRatio();
              offPeakRatio = comConf.getCompactionRatioOffPeak();
          

          Nit: necessary? The whole point of the compaction config from FB patch was to make these easily accessible everywhere,
          as far as I see.

              List<StoreFile> bestSelection = new ArrayList<StoreFile>(0);
          

          Nit: not necessary.

          for (int start = 0; start < candidates.size(); start++) {
          

          Doesn't have to consider with less than minfiles to the end.

           for(int currentEnd = start; currentEnd < candidates.size(); currentEnd++) {
          

          Can go from start plus minfiles, and check maxfiles too, then checks inside become unnecessary...

          return  true;
          for (int j =0;
          singleFileSize >  sumAllOtherFilesize
          

          Etc.
          Nit: spacing.
          Nit^2: Many blank lines.

          Nit: the main loop too could be done in a more optimal manner, probably doesn't matter.

          Show
          Sergey Shelukhin added a comment - compactionPolicy = new ExploringCompactionPolicy( this .conf, this .store/*as StoreConfigInfo*/); HBASE-7935 will make this pluggable separately, I am planning to commit it after running tests (it has 2 +1s). Regardless, it appears that this patch both replaces default policy and hijacks the default test, so the default ratio algorithm becomes a poor bastard child, not used and not tested Should we delete it altogether and swap it with yours in place? I've seen some really nasty compaction behavior lately with what's currently the default. Can you add tests for those with new policy? Would also be useful to illustrate. if (potentialMatchFiles.size() > bestSelection.size() || (potentialMatchFiles.size() == bestSelection.size() && size < bestSize)) { Interesting heuristic... it looks reasonable, but have you tried ratio proposed above? E.g. getting rid of 3 files of 5Mb each is better than getting rid of 4 files of 500Mb each. if (files.size() <= 2) { return true ; } Why? What if files are 500 5? long sumAllOtherFilesize = 0; for ( int j =0; j < files.size(); j++) { if (i == j) continue ; sumAllOtherFilesize += files.get(j).getReader().length(); } Double nested loop is unnecessary. In fact, we already get total size outside; we might as well do it before this check, pass it in, and then we can just substract each file from it in a loop to get size of all other files. Shorter code too minFiles = comConf.getMinFilesToCompact(); maxFiles = comConf.getMaxFilesToCompact(); minCompactionSize = comConf.getMinCompactSize(); ratio = comConf.getCompactionRatio(); offPeakRatio = comConf.getCompactionRatioOffPeak(); Nit: necessary? The whole point of the compaction config from FB patch was to make these easily accessible everywhere, as far as I see. List<StoreFile> bestSelection = new ArrayList<StoreFile>(0); Nit: not necessary. for ( int start = 0; start < candidates.size(); start++) { Doesn't have to consider with less than minfiles to the end. for ( int currentEnd = start; currentEnd < candidates.size(); currentEnd++) { Can go from start plus minfiles, and check maxfiles too, then checks inside become unnecessary... return true ; for ( int j =0; singleFileSize > sumAllOtherFilesize Etc. Nit: spacing. Nit^2: Many blank lines. Nit: the main loop too could be done in a more optimal manner, probably doesn't matter.
          Hide
          Elliott Clark added a comment -

          Can you add tests for those with new policy?

          Not really it relies on Bulk loading, regular inserts, and compactions to work together to make some strange orders of files and seq numbers. Though one of the added tests ((251, 253, 251, maxSize -1)) should cover cover things alright.

          Why? What if files are 500 5?

          yep off by one error. Should be < 2 rather than <= 2.

          Nit: not necessary.

          Yes it is. If the loop never finds something that passes the pre-conditions we need to have an empty list to return or there will be npe's.

          Show
          Elliott Clark added a comment - Can you add tests for those with new policy? Not really it relies on Bulk loading, regular inserts, and compactions to work together to make some strange orders of files and seq numbers. Though one of the added tests ((251, 253, 251, maxSize -1)) should cover cover things alright. Why? What if files are 500 5? yep off by one error. Should be < 2 rather than <= 2. Nit: not necessary. Yes it is. If the loop never finds something that passes the pre-conditions we need to have an empty list to return or there will be npe's.
          Hide
          Sergey Shelukhin added a comment -

          Hi. Is this stuck? I can try to pick up this patch next week if needed.

          Show
          Sergey Shelukhin added a comment - Hi. Is this stuck? I can try to pick up this patch next week if needed.
          Hide
          stack added a comment -

          Elliott Clark See Sergey Shelukhin's note above.

          Show
          stack added a comment - Elliott Clark See Sergey Shelukhin 's note above.
          Hide
          Elliott Clark added a comment -

          Not stuck, I've just been pretty busy. I'll try and get a refresh up sometime early next week.

          Show
          Elliott Clark added a comment - Not stuck, I've just been pretty busy. I'll try and get a refresh up sometime early next week.
          Hide
          Elliott Clark added a comment -

          Interesting heuristic... it looks reasonable, but have you tried ratio proposed above? E.g. getting rid of 3 files of 5Mb each is better than getting rid of 4 files of 500Mb each.

          Yes the ratio proposed when this started has a tendency to get too stuck on small files.

          Double nested loop is unnecessary.

          Removed that. Though I didn't pass it in as the parameters should be instructive of usage not just a performance optimization.

          Doesn't have to consider with less than minfiles to the end.

          Done.

          Can go from start plus minfiles, and check maxfiles too, then checks inside become unnecessary...

          Makes computing the end really ugly and for this loop it's easier to just use the continue. No harm in being more careful here.

          Show
          Elliott Clark added a comment - Interesting heuristic... it looks reasonable, but have you tried ratio proposed above? E.g. getting rid of 3 files of 5Mb each is better than getting rid of 4 files of 500Mb each. Yes the ratio proposed when this started has a tendency to get too stuck on small files. Double nested loop is unnecessary. Removed that. Though I didn't pass it in as the parameters should be instructive of usage not just a performance optimization. Doesn't have to consider with less than minfiles to the end. Done. Can go from start plus minfiles, and check maxfiles too, then checks inside become unnecessary... Makes computing the end really ugly and for this loop it's easier to just use the continue. No harm in being more careful here.
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 4 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 site. The mvn site goal succeeds with this patch.

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.master.TestTableLockManager

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/5054//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5054//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5054//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5054//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5054//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5054//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5054//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5054//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5054//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/5054//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/12576159/HBASE-7842-5.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 4 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 site . The mvn site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.master.TestTableLockManager Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/5054//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5054//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5054//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5054//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5054//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5054//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5054//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5054//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5054//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/5054//console This message is automatically generated.
          Hide
          Sergey Shelukhin added a comment -

          Looks good as such; one question left is:

          Regardless, it appears that this patch both replaces default policy and hijacks the default test, so the default ratio

          algorithm becomes a poor bastard child, not used and not tested Should we delete it altogether and swap it with yours in place?

          Do you want to just replace the default one?

          Show
          Sergey Shelukhin added a comment - Looks good as such; one question left is: Regardless, it appears that this patch both replaces default policy and hijacks the default test, so the default ratio algorithm becomes a poor bastard child, not used and not tested Should we delete it altogether and swap it with yours in place? Do you want to just replace the default one?
          Hide
          Sergey Shelukhin added a comment -

          quote should not be cut...

          Show
          Sergey Shelukhin added a comment - quote should not be cut...
          Hide
          Elliott Clark added a comment -

          I'd rather leave it there, just in case someone has a workload that relied on the old behavior. Perhaps it should be re-named ?

          Show
          Elliott Clark added a comment - I'd rather leave it there, just in case someone has a workload that relied on the old behavior. Perhaps it should be re-named ?
          Hide
          Sergey Shelukhin added a comment -

          Yeah, maybe this one should be default and the old one should be Old

          Show
          Sergey Shelukhin added a comment - Yeah, maybe this one should be default and the old one should be Old
          Hide
          Elliott Clark added a comment -

          Renamed DefaultCompactionPolicy to RatioBasedCompactionPolicy

          Show
          Elliott Clark added a comment - Renamed DefaultCompactionPolicy to RatioBasedCompactionPolicy
          Hide
          Sergey Shelukhin added a comment -

          I thought inheritance would be the other way around anyway, LGTM

          +1

          Show
          Sergey Shelukhin added a comment - I thought inheritance would be the other way around anyway, LGTM +1
          Hide
          Elliott Clark added a comment -

          Added the new policy to the perf simulations.

          Got these results. WOW they have me a little floored.....

          https://docs.google.com/spreadsheet/ccc?key=0AqJ3FqeHriCkdGloRjlpYXdzMU5kR0NpT28xWmRnaFE#gid=0

          Once the pre-commit comes back I'll commit and file an issue for 0.94 backport.

          Show
          Elliott Clark added a comment - Added the new policy to the perf simulations. Got these results. WOW they have me a little floored..... https://docs.google.com/spreadsheet/ccc?key=0AqJ3FqeHriCkdGloRjlpYXdzMU5kR0NpT28xWmRnaFE#gid=0 Once the pre-commit comes back I'll commit and file an issue for 0.94 backport.
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 16 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 site. The mvn site goal succeeds with this patch.

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

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/5154//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5154//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5154//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5154//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5154//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5154//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5154//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5154//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5154//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/5154//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/12577271/HBASE-7842-6.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 16 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 site . The mvn site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.client.TestHTableMultiplexer Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/5154//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5154//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5154//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5154//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5154//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5154//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5154//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5154//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5154//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/5154//console This message is automatically generated.
          Hide
          Elliott Clark added a comment -

          Thanks for the review Sergey Shelukhin

          Committed to trunk and 0.95. Filing an issue for 0.94 now.

          Show
          Elliott Clark added a comment - Thanks for the review Sergey Shelukhin Committed to trunk and 0.95. Filing an issue for 0.94 now.
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 16 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 site. The mvn site goal succeeds with this patch.

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.master.TestTableLockManager

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/5156//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5156//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5156//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5156//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5156//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5156//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5156//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5156//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5156//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/5156//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/12577292/HBASE-7842-7.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 16 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 site . The mvn site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.master.TestTableLockManager Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/5156//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5156//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5156//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5156//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5156//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5156//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5156//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5156//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5156//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/5156//console This message is automatically generated.
          Hide
          Ted Yu added a comment -

          Looks like ExploringCompactionPolicy.java misses license header.

          Show
          Ted Yu added a comment - Looks like ExploringCompactionPolicy.java misses license header.
          Hide
          Ted Yu added a comment -
          +   * Check that all files satisfy the r
          +   * @param files
          +   * @return
          +   */
          +  private boolean filesInRatio(List<StoreFile> files, boolean isOffPeak) {
          

          The first sentence seems incomplete. There is no javadoc for param isOffPeak. There is no specification for what gets returned.

          Show
          Ted Yu added a comment - + * Check that all files satisfy the r + * @param files + * @ return + */ + private boolean filesInRatio(List<StoreFile> files, boolean isOffPeak) { The first sentence seems incomplete. There is no javadoc for param isOffPeak. There is no specification for what gets returned.
          Hide
          Elliott Clark added a comment -

          Add the header

          Show
          Elliott Clark added a comment - Add the header
          Hide
          Hudson added a comment -

          Integrated in HBase-TRUNK #4021 (See https://builds.apache.org/job/HBase-TRUNK/4021/)
          HBASE-7842 Add missed Header (Revision 1465162)
          HBASE-7842 Add compaction policy that explores more storefile groups (Revision 1465133)

          Result = SUCCESS
          eclark :
          Files :

          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/ExploringCompactionPolicy.java

          eclark :
          Files :

          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultStoreEngine.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreEngine.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/ExploringCompactionPolicy.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/RatioBasedCompactionPolicy.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/TestDefaultStoreEngine.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/compactions/PerfTestCompactionPolicies.java
          Show
          Hudson added a comment - Integrated in HBase-TRUNK #4021 (See https://builds.apache.org/job/HBase-TRUNK/4021/ ) HBASE-7842 Add missed Header (Revision 1465162) HBASE-7842 Add compaction policy that explores more storefile groups (Revision 1465133) Result = SUCCESS eclark : Files : /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/ExploringCompactionPolicy.java eclark : Files : /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultStoreEngine.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreEngine.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/ExploringCompactionPolicy.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/RatioBasedCompactionPolicy.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/TestDefaultStoreEngine.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/compactions/PerfTestCompactionPolicies.java
          Hide
          Hudson added a comment -

          Integrated in hbase-0.95 #129 (See https://builds.apache.org/job/hbase-0.95/129/)
          HBASE-7842 Add missed Header (Revision 1465160)
          HBASE-7842 Add compaction policy that explores more storefile groups (Revision 1465135)

          Result = SUCCESS
          eclark :
          Files :

          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/ExploringCompactionPolicy.java

          eclark :
          Files :

          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultStoreEngine.java
          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreEngine.java
          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/DefaultCompactionPolicy.java
          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/ExploringCompactionPolicy.java
          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/RatioBasedCompactionPolicy.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompaction.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultCompactSelection.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultStoreEngine.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/compactions/PerfTestCompactionPolicies.java
          Show
          Hudson added a comment - Integrated in hbase-0.95 #129 (See https://builds.apache.org/job/hbase-0.95/129/ ) HBASE-7842 Add missed Header (Revision 1465160) HBASE-7842 Add compaction policy that explores more storefile groups (Revision 1465135) Result = SUCCESS eclark : Files : /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/ExploringCompactionPolicy.java eclark : Files : /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultStoreEngine.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreEngine.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/DefaultCompactionPolicy.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/ExploringCompactionPolicy.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/RatioBasedCompactionPolicy.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompaction.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultCompactSelection.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultStoreEngine.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/compactions/PerfTestCompactionPolicies.java
          Hide
          Hudson added a comment -

          Integrated in hbase-0.95-on-hadoop2 #57 (See https://builds.apache.org/job/hbase-0.95-on-hadoop2/57/)
          HBASE-7842 Add missed Header (Revision 1465160)
          HBASE-7842 Add compaction policy that explores more storefile groups (Revision 1465135)

          Result = FAILURE
          eclark :
          Files :

          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/ExploringCompactionPolicy.java

          eclark :
          Files :

          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultStoreEngine.java
          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreEngine.java
          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/DefaultCompactionPolicy.java
          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/ExploringCompactionPolicy.java
          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/RatioBasedCompactionPolicy.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompaction.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultCompactSelection.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultStoreEngine.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/compactions/PerfTestCompactionPolicies.java
          Show
          Hudson added a comment - Integrated in hbase-0.95-on-hadoop2 #57 (See https://builds.apache.org/job/hbase-0.95-on-hadoop2/57/ ) HBASE-7842 Add missed Header (Revision 1465160) HBASE-7842 Add compaction policy that explores more storefile groups (Revision 1465135) Result = FAILURE eclark : Files : /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/ExploringCompactionPolicy.java eclark : Files : /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultStoreEngine.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreEngine.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/DefaultCompactionPolicy.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/ExploringCompactionPolicy.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/RatioBasedCompactionPolicy.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompaction.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultCompactSelection.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultStoreEngine.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/compactions/PerfTestCompactionPolicies.java
          Hide
          Hudson added a comment -

          Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #479 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/479/)
          HBASE-7842 Add missed Header (Revision 1465162)
          HBASE-7842 Add compaction policy that explores more storefile groups (Revision 1465133)

          Result = FAILURE
          eclark :
          Files :

          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/ExploringCompactionPolicy.java

          eclark :
          Files :

          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultStoreEngine.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreEngine.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/ExploringCompactionPolicy.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/RatioBasedCompactionPolicy.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/TestDefaultStoreEngine.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/compactions/PerfTestCompactionPolicies.java
          Show
          Hudson added a comment - Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #479 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/479/ ) HBASE-7842 Add missed Header (Revision 1465162) HBASE-7842 Add compaction policy that explores more storefile groups (Revision 1465133) Result = FAILURE eclark : Files : /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/ExploringCompactionPolicy.java eclark : Files : /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultStoreEngine.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreEngine.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/ExploringCompactionPolicy.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/RatioBasedCompactionPolicy.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/TestDefaultStoreEngine.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/compactions/PerfTestCompactionPolicies.java
          Hide
          stack added a comment -

          Those are some nice numbers Elliott Clark

          Show
          stack added a comment - Those are some nice numbers Elliott Clark
          Hide
          Hudson added a comment -

          Integrated in HBase-0.94-security #162 (See https://builds.apache.org/job/HBase-0.94-security/162/)
          HBASE-8283 Backport HBASE-7842 Add compaction policy that explores more storefile groups to 0.94 (Revision 1491546)

          Result = SUCCESS
          eclark :
          Files :

          • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
          Show
          Hudson added a comment - Integrated in HBase-0.94-security #162 (See https://builds.apache.org/job/HBase-0.94-security/162/ ) HBASE-8283 Backport HBASE-7842 Add compaction policy that explores more storefile groups to 0.94 (Revision 1491546) Result = SUCCESS eclark : Files : /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
          Hide
          Hudson added a comment -

          Integrated in HBase-0.94 #1008 (See https://builds.apache.org/job/HBase-0.94/1008/)
          HBASE-8283 Backport HBASE-7842 Add compaction policy that explores more storefile groups to 0.94 (Revision 1491546)

          Result = FAILURE
          eclark :
          Files :

          • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
          Show
          Hudson added a comment - Integrated in HBase-0.94 #1008 (See https://builds.apache.org/job/HBase-0.94/1008/ ) HBASE-8283 Backport HBASE-7842 Add compaction policy that explores more storefile groups to 0.94 (Revision 1491546) Result = FAILURE eclark : Files : /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java

            People

            • Assignee:
              Elliott Clark
              Reporter:
              Elliott Clark
            • Votes:
              0 Vote for this issue
              Watchers:
              14 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development