Details

    • Improvement
    • Status: Closed
    • Critical
    • Resolution: Fixed
    • None
    • 0.90.0
    • None
    • None
    • Reviewed

    Description

      HBASE-2248 turned Gets into Scans server-side. It also removed the invariant that deletes in a file only apply to other files and not itself (no longer processes MemStore deletes when the delete happens). This has implications for our minor compaction policy.

      We are currently processing deletes during minor compactions in a way that makes it so we do the actual deleting as we compact, but we retain the delete records themselves. This makes it so we retain the invariant of deletes only applying to other files.

      Since this is now gone post HBASE-2248, we should revisit our compaction policies.

      Attachments

        Issue Links

        Activity

          streamy Jonathan Gray added a comment -

          I think we should remove all delete processing from minor compactions in order to make minor compactions as fast as possible. For me, that would suffice for closing this jira.

          Major compactions are another consideration. As I described in HBASE-2450, the fact that delete markers actually get removed during major compactions makes it so a background process impacts user-facing behavior. This is because old delete records can impact new puts (if i put a value with an older timestamp than a row delete, for example). Before the major it would not show up, after the major this put would be valid.

          One possibility is we change it so minors don't do anything, then majors do what minors do now (doing the actually deleting, but retaining the deletes themselves). Only downside of that is that once you delete a row at a timestamp, you can never re-insert values older than that delete. Today, this is the case until there is a major compaction. The way to fix this is by taking storefile age into account so that deletes in previous storefiles don't apply to newer storefiles. If we did that, we would have to process deletes during regular compactions because you'd need to look at the relative ages of the storefiles to determine if a particular delete applied or not.

          For now, I'd be happy just removing delete tracking in minors and worrying about the rest of these issues for 0.21.

          streamy Jonathan Gray added a comment - I think we should remove all delete processing from minor compactions in order to make minor compactions as fast as possible. For me, that would suffice for closing this jira. Major compactions are another consideration. As I described in HBASE-2450 , the fact that delete markers actually get removed during major compactions makes it so a background process impacts user-facing behavior. This is because old delete records can impact new puts (if i put a value with an older timestamp than a row delete, for example). Before the major it would not show up, after the major this put would be valid. One possibility is we change it so minors don't do anything, then majors do what minors do now (doing the actually deleting, but retaining the deletes themselves). Only downside of that is that once you delete a row at a timestamp, you can never re-insert values older than that delete. Today, this is the case until there is a major compaction. The way to fix this is by taking storefile age into account so that deletes in previous storefiles don't apply to newer storefiles. If we did that, we would have to process deletes during regular compactions because you'd need to look at the relative ages of the storefiles to determine if a particular delete applied or not. For now, I'd be happy just removing delete tracking in minors and worrying about the rest of these issues for 0.21.
          streamy Jonathan Gray added a comment -

          Patch removes delete tracking from minor compactions and updates the unit test. Have not run against other tests, just a quick first go.

          streamy Jonathan Gray added a comment - Patch removes delete tracking from minor compactions and updates the unit test. Have not run against other tests, just a quick first go.
          stack Michael Stack added a comment -

          Applied to 0.20.4 and 0.20.4 and trunk. Made HBASE-2454 for major compaction delete issue. Thanks for patch Jon.

          stack Michael Stack added a comment - Applied to 0.20.4 and 0.20.4 and trunk. Made HBASE-2454 for major compaction delete issue. Thanks for patch Jon.

          Not totally convinced about this. If your workload is IO bound, processing the deletes more aggressively (i.e. during minor compactions) even if it meant some extra CPU might be good overall.

          Can we atleast make this a configurable option? We should do some numbers to see what the savings of not doing delete processing are.

          kannanm Kannan Muthukkaruppan added a comment - Not totally convinced about this. If your workload is IO bound, processing the deletes more aggressively (i.e. during minor compactions) even if it meant some extra CPU might be good overall. Can we atleast make this a configurable option? We should do some numbers to see what the savings of not doing delete processing are.
          streamy Jonathan Gray added a comment -

          I agree that if you have an IO bound workload, and you have a significant number of deletes, then this may not be a win. This seems this a fairly rare use case except for those applications which do a ton of deleting. In those cases, you could always trigger majors.

          What we get by taking this out is not having to track deletes during minors (yes, we should measure this), retaining deleted records for sake of recovery/snapshotscanner type stuff, and (possibly) being able to do minor compactions against any storefiles (not just neighbors).

          Looking at the old minor and scan delete tracker code paths, it's not a ton of work in the case that there are no deletes, so I imagine it's not a big performance impact. However once you take the delete tracking out, minors can actually be refactored to not keep track row by row as it still does even after this patch... Having to do row compares for every kv, plus the previous delete tracking, we could see some incremental boost from stripping down minors to the bare minimum.

          Let's keep the conversation going on these topics.

          Of note but not really that important, I believe the bigtable paper explicitly talks about deletes only being deleted during major compactions.

          streamy Jonathan Gray added a comment - I agree that if you have an IO bound workload, and you have a significant number of deletes, then this may not be a win. This seems this a fairly rare use case except for those applications which do a ton of deleting. In those cases, you could always trigger majors. What we get by taking this out is not having to track deletes during minors (yes, we should measure this), retaining deleted records for sake of recovery/snapshotscanner type stuff, and (possibly) being able to do minor compactions against any storefiles (not just neighbors). Looking at the old minor and scan delete tracker code paths, it's not a ton of work in the case that there are no deletes, so I imagine it's not a big performance impact. However once you take the delete tracking out, minors can actually be refactored to not keep track row by row as it still does even after this patch... Having to do row compares for every kv, plus the previous delete tracking, we could see some incremental boost from stripping down minors to the bare minimum. Let's keep the conversation going on these topics. Of note but not really that important, I believe the bigtable paper explicitly talks about deletes only being deleted during major compactions.
          stack Michael Stack added a comment -

          Lets make a new issue to address the important performance considerations raised above.

          stack Michael Stack added a comment - Lets make a new issue to address the important performance considerations raised above.
          stack Michael Stack added a comment -

          Does HBASE-2462 work as the issue for looking at performance doing deletes at minor compaction time?

          stack Michael Stack added a comment - Does HBASE-2462 work as the issue for looking at performance doing deletes at minor compaction time?
          streamy Jonathan Gray added a comment -

          +1, thanks stack let's move discussion there

          streamy Jonathan Gray added a comment - +1, thanks stack let's move discussion there
          stack Michael Stack added a comment -

          Marking these as fixed against 0.21.0 rather than against 0.20.5.

          stack Michael Stack added a comment - Marking these as fixed against 0.21.0 rather than against 0.20.5.
          larsfrancke Lars Francke added a comment -

          This issue was closed as part of a bulk closing operation on 2015-11-20. All issues that have been resolved and where all fixVersions have been released have been closed (following discussions on the mailing list).

          larsfrancke Lars Francke added a comment - This issue was closed as part of a bulk closing operation on 2015-11-20. All issues that have been resolved and where all fixVersions have been released have been closed (following discussions on the mailing list).

          People

            streamy Jonathan Gray
            streamy Jonathan Gray
            Votes:
            0 Vote for this issue
            Watchers:
            Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Slack