HBase
  1. HBase
  2. HBASE-8283

Backport HBASE-7842 Add compaction policy that explores more storefile groups to 0.94

    Details

    • Type: Task Task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.94.0
    • Fix Version/s: 0.94.9
    • Component/s: Compaction
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    • Release Note:
      Added a configuration option to turn on a new compaction selection algorithm that searches more storefile groups in order to find the best set of files to compact.

      Description

      HBASE-7842 Add compaction policy that explores more storefile groups

      Added a new compaction policy that greatly improves selecting files if there are bulk loaded files.

      1. HBASE-8283-0.patch
        8 kB
        Elliott Clark
      2. HBASE-8283-1.patch
        10 kB
        Elliott Clark
      3. HBASE-8283-2.patch
        11 kB
        Elliott Clark
      4. HBASE-8283-3.patch
        11 kB
        Elliott Clark
      5. HBASE-8283-4.patch
        13 kB
        Elliott Clark

        Issue Links

          Activity

          Hide
          Elliott Clark added a comment -

          Here's the 0.94 version. It's a pretty drastic change in behavior to be putting into 0.94 so I'm undecided on if this should go in.

          It basically fixes bulk import workloads, but it could drastically change the amount of IO needed.

          Thoughts?

          Show
          Elliott Clark added a comment - Here's the 0.94 version. It's a pretty drastic change in behavior to be putting into 0.94 so I'm undecided on if this should go in. It basically fixes bulk import workloads, but it could drastically change the amount of IO needed. Thoughts?
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12577316/HBASE-8283-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/5158//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/12577316/HBASE-8283-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/5158//console This message is automatically generated.
          Hide
          Elliott Clark added a comment -

          Lars Hofhansl Any thoughts on this one ?

          Show
          Elliott Clark added a comment - Lars Hofhansl Any thoughts on this one ?
          Hide
          Lars Hofhansl added a comment -

          Hmm... This is a tough one. The patch looks nice.
          Is the goal always to compact the largest number of files?

          The results linked to from HBASE-7842 are truly impressive. I wonder if that translates to general workloads...?

          It's hard to see how this can lead to worse results, but it's hard to be sure about this.
          I'd have a cautious +1 here.

          Show
          Lars Hofhansl added a comment - Hmm... This is a tough one. The patch looks nice. Is the goal always to compact the largest number of files? The results linked to from HBASE-7842 are truly impressive. I wonder if that translates to general workloads...? It's hard to see how this can lead to worse results, but it's hard to be sure about this. I'd have a cautious +1 here.
          Hide
          Elliott Clark added a comment -

          There are really three goals.

          Goal One

          The first goal is to fix bulk load files. Right now their ordering gets messed up after a compaction happens. This leads to some weird compactions where the smallest files are being compacted with the largest. This is possible because the compaction policy right now approves the candidates list as soon as one file is less than or equal to the files after it. The bulk loaded files are always on the on the left. The new large file created from compaction does not have the bulk load flag (thats lost) and it will have a seqId of 0.

          Goal Two

          The other goal is to only compact files that are all inside of a ratio. All canidate files are selected if the is one file to the left that satisfies the ratio SizeFile(j) <= SumFileSize( j-1, 0). Workloads where there are large fluctuations can select weird groups of files.

          Suppose there's a write work load that's heavily sinusoidal.
          [ 1 1 50 150 180 150 50 1 1 1 1 ]

          Currently we'd pick 1 1 50 as the files to compact.
          1 1 1 are the most like each other.
          150 180 150 are also more similar and would logically be better matches than the ones currently picked.

          Goal Three

          Just because files are picked doesn't mean they are the best choice. Right now our compaction algorithm is pretty naive. This is a cut at choosing files based on more than one heuristic (ratio, num files removed, and IO required).

          Show
          Elliott Clark added a comment - There are really three goals. Goal One The first goal is to fix bulk load files. Right now their ordering gets messed up after a compaction happens. This leads to some weird compactions where the smallest files are being compacted with the largest. This is possible because the compaction policy right now approves the candidates list as soon as one file is less than or equal to the files after it. The bulk loaded files are always on the on the left. The new large file created from compaction does not have the bulk load flag (thats lost) and it will have a seqId of 0. Goal Two The other goal is to only compact files that are all inside of a ratio. All canidate files are selected if the is one file to the left that satisfies the ratio SizeFile(j) <= SumFileSize( j-1, 0). Workloads where there are large fluctuations can select weird groups of files. Suppose there's a write work load that's heavily sinusoidal. [ 1 1 50 150 180 150 50 1 1 1 1 ] Currently we'd pick 1 1 50 as the files to compact. 1 1 1 are the most like each other. 150 180 150 are also more similar and would logically be better matches than the ones currently picked. Goal Three Just because files are picked doesn't mean they are the best choice. Right now our compaction algorithm is pretty naive. This is a cut at choosing files based on more than one heuristic (ratio, num files removed, and IO required).
          Hide
          Elliott Clark added a comment -

          This will need to be updated with HBASE-8299

          Show
          Elliott Clark added a comment - This will need to be updated with HBASE-8299
          Hide
          Elliott Clark added a comment -

          Here's a port of the latest exploring compaction selection that includes the work to never get stuck.

          Show
          Elliott Clark added a comment - Here's a port of the latest exploring compaction selection that includes the work to never get stuck.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12582513/HBASE-8283-1.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/5618//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/12582513/HBASE-8283-1.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/5618//console This message is automatically generated.
          Hide
          Elliott Clark added a comment -

          ping.

          Show
          Elliott Clark added a comment - ping.
          Hide
          stack added a comment -

          Can it be turned on optionally Elliott? Backport w/ it off?

          We need the bulk load fix.

          Any testing on a cluster? If not, who'd be up for trying it?

          Show
          stack added a comment - Can it be turned on optionally Elliott? Backport w/ it off? We need the bulk load fix. Any testing on a cluster? If not, who'd be up for trying it?
          Hide
          Elliott Clark added a comment -

          Can it be turned on optionally Elliott? Backport w/ it off?

          Nope 0.94 doesn't have pluggable compaction policies. So without changing a lot of the Store code it's not easy to ship with it off.

          We need the bulk load fix.

          Yep that's the only reason I would consider pulling this into 0.94

          Any testing on a cluster? If not, who'd be up for trying it?

          JD tested this on a cluster for quite a while, before I put in the logic that gets compaction unstuck. So the majority is tested. The last 5% isn't. I can try it but it will be a few days.

          Show
          Elliott Clark added a comment - Can it be turned on optionally Elliott? Backport w/ it off? Nope 0.94 doesn't have pluggable compaction policies. So without changing a lot of the Store code it's not easy to ship with it off. We need the bulk load fix. Yep that's the only reason I would consider pulling this into 0.94 Any testing on a cluster? If not, who'd be up for trying it? JD tested this on a cluster for quite a while, before I put in the logic that gets compaction unstuck. So the majority is tested. The last 5% isn't. I can try it but it will be a few days.
          Hide
          Lars Hofhansl added a comment -

          The patch looks good. Would be a good addition to 0.94.
          I can offer to load this on our test cluster and run a large PE workload on it.

          Show
          Lars Hofhansl added a comment - The patch looks good. Would be a good addition to 0.94. I can offer to load this on our test cluster and run a large PE workload on it.
          Hide
          Lars Hofhansl added a comment -

          Maybe the pluggable compaction policy should be backported to 0.94 as well.
          A coworker and I were just looking at that code in trunk and found it could be improved and simplified, maybe we can do that and then port the simpler result to 0.94.

          Show
          Lars Hofhansl added a comment - Maybe the pluggable compaction policy should be backported to 0.94 as well. A coworker and I were just looking at that code in trunk and found it could be improved and simplified, maybe we can do that and then port the simpler result to 0.94.
          Hide
          Elliott Clark added a comment -

          If you want to do that it should be pretty easy to rebase this patch. I can hold off if you think it would be worth the wait.

          Show
          Elliott Clark added a comment - If you want to do that it should be pretty easy to rebase this patch. I can hold off if you think it would be worth the wait.
          Hide
          Jean-Daniel Cryans added a comment -

          I know Elliott Clark doesn't like my idea but it would be easy to have both policies:

          if (defaultCompactionPolicy)
            current code
          else
            Elliott's patch
          

          I did that when testing a previous version of the patch on 0.94 to be able to switch from one to another easily. I like it because it's not as involved as backporting pluggable policies and it's less risky then just committing the current patch that replaces the policy. The downside is that it's hacky, it makes the code less readable, and if we ever want more policies in 0.94 then we'll have to backport the pluggable policies code anyway.

          Show
          Jean-Daniel Cryans added a comment - I know Elliott Clark doesn't like my idea but it would be easy to have both policies: if (defaultCompactionPolicy) current code else Elliott's patch I did that when testing a previous version of the patch on 0.94 to be able to switch from one to another easily. I like it because it's not as involved as backporting pluggable policies and it's less risky then just committing the current patch that replaces the policy. The downside is that it's hacky, it makes the code less readable, and if we ever want more policies in 0.94 then we'll have to backport the pluggable policies code anyway.
          Hide
          Elliott Clark added a comment -

          Yep I still like a full replacement or a fully pluggable solution more, but if that's what the community wants I will go along.

          Show
          Elliott Clark added a comment - Yep I still like a full replacement or a fully pluggable solution more, but if that's what the community wants I will go along.
          Hide
          Elliott Clark added a comment -

          Here's a patch that makes this a config option, that's off by default.

          It's not a full pluggable as that started to introduce too many changes.

          Show
          Elliott Clark added a comment - Here's a patch that makes this a config option, that's off by default. It's not a full pluggable as that started to introduce too many changes.
          Hide
          Hadoop QA added a comment -

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

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

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

          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/5960//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/12586584/HBASE-8283-2.patch against trunk revision . +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/5960//console This message is automatically generated.
          Hide
          Elliott Clark added a comment -

          Woops had it turned on by default

          Show
          Elliott Clark added a comment - Woops had it turned on by 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/12586587/HBASE-8283-3.patch
          against trunk revision .

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

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

          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/5962//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/12586587/HBASE-8283-3.patch against trunk revision . +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/5962//console This message is automatically generated.
          Hide
          Elliott Clark added a comment -

          JD asked for a test to show it working. Here it is.

          Show
          Elliott Clark added a comment - JD asked for a test to show it working. Here it is.
          Hide
          Jean-Daniel Cryans added a comment -

          +1 if the test passes

          Show
          Jean-Daniel Cryans added a comment - +1 if the test passes
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12586836/HBASE-8283-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 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/5974//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/12586836/HBASE-8283-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 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/5974//console This message is automatically generated.
          Hide
          Elliott Clark added a comment -

          Committed revision 1491546.

          Show
          Elliott Clark added a comment - Committed revision 1491546.
          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:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development