HBase
  1. HBase
  2. HBASE-10076

Backport MapReduce over snapshot files [0.94]

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Won't Fix
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      MapReduce over Snapshots would be valuable on 0.94.

        Issue Links

          Activity

          Hide
          Lars Hofhansl added a comment -
          Show
          Lars Hofhansl added a comment - Jesse Yates , FYI.
          Hide
          Jesse Yates added a comment -

          First cut patch.

          Unit tests (all the Snapshot ones) pass, but haven't had a chance to try the integration test yet - waiting until everyone is happy with the rest of the code (should be simple, just a minimal amount of setup and then uses the rest of the test code).

          Show
          Jesse Yates added a comment - First cut patch. Unit tests (all the Snapshot ones) pass, but haven't had a chance to try the integration test yet - waiting until everyone is happy with the rest of the code (should be simple, just a minimal amount of setup and then uses the rest of the test code).
          Hide
          Lars Hofhansl added a comment -

          Looks good upon first inspection.

          Show
          Lars Hofhansl added a comment - Looks good upon first inspection.
          Hide
          Enis Soztutar added a comment -

          Looks all the pieces are here

          • remove System.out.println("In restore!");
          • we should remove the scanMetrics from ClientScanner from the original patch:
             - protected ScanMetrics scanMetrics = null;
            
          • Any interest in bringing in the new test TestCellUtil. testOverlappingKeys() ? Not needed that much, just checking
          • It would be good to have IntegrationTestTableSnapshotInputFormat in the same package (mapreduce)
          • Not sure about the chang in TableInputFormatBase. Is this needed? Let's leave this out otherwise.
          • In the original patch, TableMapReduceUtil. initTableMapperJob() now accepts an initCredentials param, because we do not want to get tokens from HBase at all. Otherwise, if hbase is used with security, offline clusters won't work.
          Show
          Enis Soztutar added a comment - Looks all the pieces are here remove System.out.println("In restore!"); we should remove the scanMetrics from ClientScanner from the original patch: - protected ScanMetrics scanMetrics = null ; Any interest in bringing in the new test TestCellUtil. testOverlappingKeys() ? Not needed that much, just checking It would be good to have IntegrationTestTableSnapshotInputFormat in the same package (mapreduce) Not sure about the chang in TableInputFormatBase. Is this needed? Let's leave this out otherwise. In the original patch, TableMapReduceUtil. initTableMapperJob() now accepts an initCredentials param, because we do not want to get tokens from HBase at all. Otherwise, if hbase is used with security, offline clusters won't work.
          Show
          Andrew Purtell added a comment - See https://issues.apache.org/jira/browse/HBASE-8369?focusedCommentId=13846957&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13846957
          Hide
          Jesse Yates added a comment -

          thanks for the feedback Enis! I'll update the patch, in the event that someone wants it for their installation, or have it ready if we can get it into 0.96 (as per discussion on HBASE-8369).

          Show
          Jesse Yates added a comment - thanks for the feedback Enis! I'll update the patch, in the event that someone wants it for their installation, or have it ready if we can get it into 0.96 (as per discussion on HBASE-8369 ).
          Hide
          Jesse Yates added a comment -

          Any interest in bringing in the new test TestCellUtil. testOverlappingKeys() ? Not needed that much, just checking

          Done - good call.

          Not sure about the change in TableInputFormatBase. Is this needed? Let's leave this out otherwise.

          Not needed, but this is a cleaner, better implementation and good to reuse the util, now that we have it. Agree its a little extra and a bit unneccessary, but trivially so.

          In the original patch, TableMapReduceUtil. initTableMapperJob() now accepts an initCredentials param, because we do not want to get tokens from HBase at all. Otherwise, if hbase is used with security, offline clusters won't work.

          Looks like in 0.94 its just added based on the config. In trunk, looks like TableMapReduceUtil.initTableSnapshotMapperJob just calls initTableMapperJob without any parameter, which always initializes the credentials. Maybe a bug in trunk? Seems like you would need a more sweeping change to make it configurable as well.

          Show
          Jesse Yates added a comment - Any interest in bringing in the new test TestCellUtil. testOverlappingKeys() ? Not needed that much, just checking Done - good call. Not sure about the change in TableInputFormatBase. Is this needed? Let's leave this out otherwise. Not needed, but this is a cleaner, better implementation and good to reuse the util, now that we have it. Agree its a little extra and a bit unneccessary, but trivially so. In the original patch, TableMapReduceUtil. initTableMapperJob() now accepts an initCredentials param, because we do not want to get tokens from HBase at all. Otherwise, if hbase is used with security, offline clusters won't work. Looks like in 0.94 its just added based on the config. In trunk, looks like TableMapReduceUtil.initTableSnapshotMapperJob just calls initTableMapperJob without any parameter, which always initializes the credentials. Maybe a bug in trunk? Seems like you would need a more sweeping change to make it configurable as well.
          Hide
          Jesse Yates added a comment -

          Enis Soztutar other questions Lars and I had offline - how can we test that the locality selection is correct? Its not really covered anywhere in this patch or the original

          Show
          Jesse Yates added a comment - Enis Soztutar other questions Lars and I had offline - how can we test that the locality selection is correct? Its not really covered anywhere in this patch or the original
          Hide
          Enis Soztutar added a comment -

          Looks like in 0.94 its just added based on the config

          In trunk, initTableSnapshotMapper() calls:

          initTableMapperJob(snapshotName, scan, mapper, outputKeyClass,
                  outputValueClass, job, addDependencyJars, false, TableSnapshotInputFormat.class);
          

          the false is initCredentials to the overloaded function, no? I don't have eclipse open, I cannot check : )

          how can we test that the locality selection is correct? Its not really covered anywhere in this patch or the original

          It is using the HDFSBlocksDistribution, which I though is tested on it's own. Did not check whether there is actual coverage though. It should be possible to mock that up I guess.

          Show
          Enis Soztutar added a comment - Looks like in 0.94 its just added based on the config In trunk, initTableSnapshotMapper() calls: initTableMapperJob(snapshotName, scan, mapper, outputKeyClass, outputValueClass, job, addDependencyJars, false , TableSnapshotInputFormat.class); the false is initCredentials to the overloaded function, no? I don't have eclipse open, I cannot check : ) how can we test that the locality selection is correct? Its not really covered anywhere in this patch or the original It is using the HDFSBlocksDistribution, which I though is tested on it's own. Did not check whether there is actual coverage though. It should be possible to mock that up I guess.
          Hide
          Lars Hofhansl added a comment -

          Based on recent discussion we might just add the necessary hooks to 0.94, in order to allow implementing the M/R outside of HBase.

          Show
          Lars Hofhansl added a comment - Based on recent discussion we might just add the necessary hooks to 0.94, in order to allow implementing the M/R outside of HBase.
          Hide
          Lars Hofhansl added a comment -

          With a little bit of cut'n'paste (TableMapReduceUtil's convertScanToString, convertStringToScan) this can indeed be done with Bryan Keller's latest 0.94 on HBASE-8369 without any additional code in HBase.

          Show
          Lars Hofhansl added a comment - With a little bit of cut'n'paste (TableMapReduceUtil's convertScanToString, convertStringToScan) this can indeed be done with Bryan Keller 's latest 0.94 on HBASE-8369 without any additional code in HBase.
          Hide
          Bryan Keller added a comment -

          Yes, there is some minor cut-n-pasting involved. We could easily package this to be available outside of the distribution if that is deemed necessary.

          Show
          Bryan Keller added a comment - Yes, there is some minor cut-n-pasting involved. We could easily package this to be available outside of the distribution if that is deemed necessary.
          Hide
          Bryan Keller added a comment -

          BTW I have made a few minor enhancements since submitting that patch. I'll check this weekend to see if it is anything worthwhile.

          Show
          Bryan Keller added a comment - BTW I have made a few minor enhancements since submitting that patch. I'll check this weekend to see if it is anything worthwhile.
          Hide
          Lars Hofhansl added a comment -

          Thanks Bryan. Much appreciated!

          Show
          Lars Hofhansl added a comment - Thanks Bryan. Much appreciated!
          Hide
          Lars Hofhansl added a comment -

          Bryan Keller, did you find anything worthwhile to include?

          Show
          Lars Hofhansl added a comment - Bryan Keller , did you find anything worthwhile to include?
          Hide
          Bryan Keller added a comment -

          No, the map-reduce code from the v5 patch is the same as I'm using now in our production servers. I created some utilities for creating and cleaning up snapshots seamlessly, but it is orthogonal to this.

          I'm curious what kind of performance improvement you are seeing with the snapshot scan (if you have tested it)?

          Show
          Bryan Keller added a comment - No, the map-reduce code from the v5 patch is the same as I'm using now in our production servers. I created some utilities for creating and cleaning up snapshots seamlessly, but it is orthogonal to this. I'm curious what kind of performance improvement you are seeing with the snapshot scan (if you have tested it)?
          Hide
          Bryan Keller added a comment -

          I should say the code is the same, except for the TableMapReduceUtil code I needed to cut-n-paste...

          Show
          Bryan Keller added a comment - I should say the code is the same, except for the TableMapReduceUtil code I needed to cut-n-paste...
          Hide
          Enis Soztutar added a comment -

          I'm curious what kind of performance improvement you are seeing with the snapshot scan

          In my tests, I was able to get 50-60 MB/s per region, versus 10-11 MB/s for regular scans (see https://issues.apache.org/jira/browse/HBASE-8369?focusedCommentId=13805748&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13805748).
          Bryan, can you also share your performance numbers if you can quantify?

          Show
          Enis Soztutar added a comment - I'm curious what kind of performance improvement you are seeing with the snapshot scan In my tests, I was able to get 50-60 MB/s per region, versus 10-11 MB/s for regular scans (see https://issues.apache.org/jira/browse/HBASE-8369?focusedCommentId=13805748&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13805748 ). Bryan, can you also share your performance numbers if you can quantify?
          Hide
          Lars Hofhansl added a comment -

          We're just taking a slightly modified version of Bryan's change along with some cut'n'pasted code from HBase.

          Show
          Lars Hofhansl added a comment - We're just taking a slightly modified version of Bryan's change along with some cut'n'pasted code from HBase.
          Hide
          Bryan Keller added a comment -

          In some simple tests, I saw a 4-5x speed improvement, which is similar to what you are seeing. In production, we saw a lower 3x improvement in our main jobs, but that is because we are more bound by CPU now than before. I did some quick profiling and it appears there is room for some optimization in HRegion to reduce CPU usage, which would further improve performance for more CPU intensive jobs.

          Show
          Bryan Keller added a comment - In some simple tests, I saw a 4-5x speed improvement, which is similar to what you are seeing. In production, we saw a lower 3x improvement in our main jobs, but that is because we are more bound by CPU now than before. I did some quick profiling and it appears there is room for some optimization in HRegion to reduce CPU usage, which would further improve performance for more CPU intensive jobs.
          Hide
          Ian Friedman added a comment -

          Hey Lars Hofhansl, Bryan Keller, is this the latest version of the patch for 94? If not, anyone know where I can get the latest version?

          Show
          Ian Friedman added a comment - Hey Lars Hofhansl , Bryan Keller , is this the latest version of the patch for 94? If not, anyone know where I can get the latest version?
          Hide
          Ian Friedman added a comment -

          never mind, I see it was in the parent ticket, I should probably read before I comment

          Show
          Ian Friedman added a comment - never mind, I see it was in the parent ticket, I should probably read before I comment

            People

            • Assignee:
              Jesse Yates
              Reporter:
              Lars Hofhansl
            • Votes:
              0 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development