Uploaded image for project: 'HBase'
  1. HBase
  2. HBASE-5656

LoadIncrementalHFiles createTable should detect and set compression algorithm

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.92.1
    • Fix Version/s: 0.94.0, 0.95.0
    • Component/s: util
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      LoadIncrementalHFiles doesn't set compression when creating the the table.

      This can be detected from the files within each family dir.

      1. HBASE-5656-0.92.patch
        1 kB
        Cosmin Lehene
      2. HBASE-5656-0.92.patch
        1 kB
        Cosmin Lehene
      3. 5656-simple.txt
        0.8 kB
        Lars Hofhansl
      4. HBASE-5656-0.92.patch
        0.8 kB
        Cosmin Lehene
      5. HBASE-5656-0.92.patch
        0.9 kB
        Cosmin Lehene

        Activity

        Hide
        hudson Hudson added a comment -

        Integrated in HBase-0.92-security #105 (See https://builds.apache.org/job/HBase-0.92-security/105/)
        HBASE-5656 LoadIncrementalHFiles createTable should detect and set compression algorithm(Cosmin Lehene) (Revision 1311105)

        Result = FAILURE
        larsh :
        Files :

        • /hbase/branches/0.92/CHANGES.txt
        • /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
        Show
        hudson Hudson added a comment - Integrated in HBase-0.92-security #105 (See https://builds.apache.org/job/HBase-0.92-security/105/ ) HBASE-5656 LoadIncrementalHFiles createTable should detect and set compression algorithm(Cosmin Lehene) (Revision 1311105) Result = FAILURE larsh : Files : /hbase/branches/0.92/CHANGES.txt /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
        Hide
        hudson Hudson added a comment -

        Integrated in HBase-0.94-security #9 (See https://builds.apache.org/job/HBase-0.94-security/9/)
        HBASE-5656 LoadIncrementalHFiles createTable should detect and set compression algorithm(Cosmin Lehene) (Revision 1311104)

        Result = SUCCESS
        larsh :
        Files :

        • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
        Show
        hudson Hudson added a comment - Integrated in HBase-0.94-security #9 (See https://builds.apache.org/job/HBase-0.94-security/9/ ) HBASE-5656 LoadIncrementalHFiles createTable should detect and set compression algorithm(Cosmin Lehene) (Revision 1311104) Result = SUCCESS larsh : Files : /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
        Hide
        hudson Hudson added a comment -

        Integrated in HBase-TRUNK-security #163 (See https://builds.apache.org/job/HBase-TRUNK-security/163/)
        HBASE-5656 LoadIncrementalHFiles createTable should detect and set compression algorithm(Cosmin Lehene) (Revision 1311106)

        Result = FAILURE
        larsh :
        Files :

        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
        Show
        hudson Hudson added a comment - Integrated in HBase-TRUNK-security #163 (See https://builds.apache.org/job/HBase-TRUNK-security/163/ ) HBASE-5656 LoadIncrementalHFiles createTable should detect and set compression algorithm(Cosmin Lehene) (Revision 1311106) Result = FAILURE larsh : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
        Hide
        hudson Hudson added a comment -

        Integrated in HBase-0.92 #363 (See https://builds.apache.org/job/HBase-0.92/363/)
        HBASE-5656 LoadIncrementalHFiles createTable should detect and set compression algorithm(Cosmin Lehene) (Revision 1311105)

        Result = SUCCESS
        larsh :
        Files :

        • /hbase/branches/0.92/CHANGES.txt
        • /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
        Show
        hudson Hudson added a comment - Integrated in HBase-0.92 #363 (See https://builds.apache.org/job/HBase-0.92/363/ ) HBASE-5656 LoadIncrementalHFiles createTable should detect and set compression algorithm(Cosmin Lehene) (Revision 1311105) Result = SUCCESS larsh : Files : /hbase/branches/0.92/CHANGES.txt /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
        Hide
        hudson Hudson added a comment -

        Integrated in HBase-0.94 #99 (See https://builds.apache.org/job/HBase-0.94/99/)
        HBASE-5656 LoadIncrementalHFiles createTable should detect and set compression algorithm(Cosmin Lehene) (Revision 1311104)

        Result = FAILURE
        larsh :
        Files :

        • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
        Show
        hudson Hudson added a comment - Integrated in HBase-0.94 #99 (See https://builds.apache.org/job/HBase-0.94/99/ ) HBASE-5656 LoadIncrementalHFiles createTable should detect and set compression algorithm(Cosmin Lehene) (Revision 1311104) Result = FAILURE larsh : Files : /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
        Hide
        hudson Hudson added a comment -

        Integrated in HBase-TRUNK #2728 (See https://builds.apache.org/job/HBase-TRUNK/2728/)
        HBASE-5656 LoadIncrementalHFiles createTable should detect and set compression algorithm(Cosmin Lehene) (Revision 1311106)

        Result = SUCCESS
        larsh :
        Files :

        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
        Show
        hudson Hudson added a comment - Integrated in HBase-TRUNK #2728 (See https://builds.apache.org/job/HBase-TRUNK/2728/ ) HBASE-5656 LoadIncrementalHFiles createTable should detect and set compression algorithm(Cosmin Lehene) (Revision 1311106) Result = SUCCESS larsh : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
        Hide
        lhofhansl Lars Hofhansl added a comment -

        Committed to 0.92, 0.94, and 0.96.

        Show
        lhofhansl Lars Hofhansl added a comment - Committed to 0.92, 0.94, and 0.96.
        Hide
        yuzhihong@gmail.com Ted Yu added a comment -

        Latest patch looks good to me.

        Show
        yuzhihong@gmail.com Ted Yu added a comment - Latest patch looks good to me.
        Hide
        lhofhansl Lars Hofhansl added a comment -

        +1 on patch. Failures seems unrelated.

        Show
        lhofhansl Lars Hofhansl added a comment - +1 on patch. Failures seems unrelated.
        Hide
        hadoopqa Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12521887/HBASE-5656-0.92.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 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 core tests. The patch failed these unit tests:
        org.apache.hadoop.hbase.coprocessor.TestRegionServerCoprocessorExceptionWithAbort

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1450//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1450//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1450//console

        This message is automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12521887/HBASE-5656-0.92.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 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 core tests. The patch failed these unit tests: org.apache.hadoop.hbase.coprocessor.TestRegionServerCoprocessorExceptionWithAbort Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1450//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1450//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1450//console This message is automatically generated.
        Hide
        hadoopqa Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12521885/HBASE-5656-0.92.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 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 core tests. The patch passed unit tests in .

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1449//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1449//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1449//console

        This message is automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12521885/HBASE-5656-0.92.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 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 core tests. The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1449//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1449//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1449//console This message is automatically generated.
        Hide
        clehene Cosmin Lehene added a comment -

        Attached new patch that includes logging.

        Show
        clehene Cosmin Lehene added a comment - Attached new patch that includes logging.
        Hide
        yuzhihong@gmail.com Ted Yu added a comment -

        Patch looks good. Minor comment:

        +            hcd.setCompressionType(reader.getCompressionAlgorithm());
        

        I think we should log the compression type used in the if block.

        Show
        yuzhihong@gmail.com Ted Yu added a comment - Patch looks good. Minor comment: + hcd.setCompressionType(reader.getCompressionAlgorithm()); I think we should log the compression type used in the if block.
        Hide
        yuzhihong@gmail.com Ted Yu added a comment -

        Latest patch applies to trunk cleanly.

        Show
        yuzhihong@gmail.com Ted Yu added a comment - Latest patch applies to trunk cleanly.
        Hide
        clehene Cosmin Lehene added a comment -

        Attached a new patch: set the compression algo from the HFile.reader if it's different than what we have on the HColumnDescriptor

        Show
        clehene Cosmin Lehene added a comment - Attached a new patch: set the compression algo from the HFile.reader if it's different than what we have on the HColumnDescriptor
        Hide
        lhofhansl Lars Hofhansl added a comment -

        Hmm... My thought was "what if not all imported files have the same compression setting", but I guess in that case we're doomed anyway.
        The HCD setting is only used to newly created files (flushes, compactions). So we should honor the default setting, no?

        Let's do your approach, wanna make a quick patch?

        Show
        lhofhansl Lars Hofhansl added a comment - Hmm... My thought was "what if not all imported files have the same compression setting", but I guess in that case we're doomed anyway. The HCD setting is only used to newly created files (flushes, compactions). So we should honor the default setting, no? Let's do your approach, wanna make a quick patch?
        Hide
        clehene Cosmin Lehene added a comment -

        Lars, so if we change the hcd default compression from NONE to LZO, but instead we write the HFile explicitly without compression this will create a table that actually has compression, which is not what we want.

        I guess if we want do be defensive we could have a reader.getCompression() != hcd.getCompression() condition.

        Show
        clehene Cosmin Lehene added a comment - Lars, so if we change the hcd default compression from NONE to LZO, but instead we write the HFile explicitly without compression this will create a table that actually has compression, which is not what we want. I guess if we want do be defensive we could have a reader.getCompression() != hcd.getCompression() condition.
        Hide
        lhofhansl Lars Hofhansl added a comment -

        How about this? Super simple.

        Show
        lhofhansl Lars Hofhansl added a comment - How about this? Super simple.
        Hide
        clehene Cosmin Lehene added a comment -

        Lars, currently we use 0.92 and have other patches in our build. I don't need it for 0.94.0, we'd just have to port the patch once we switch to 0.94.

        Btw. a minimal version of the patch would just do hcd.setCompressionType(reader.getCompressionAlgorithm()) inside the loop.

        Show
        clehene Cosmin Lehene added a comment - Lars, currently we use 0.92 and have other patches in our build. I don't need it for 0.94.0, we'd just have to port the patch once we switch to 0.94. Btw. a minimal version of the patch would just do hcd.setCompressionType(reader.getCompressionAlgorithm()) inside the loop.
        Hide
        clehene Cosmin Lehene added a comment -

        added new patch that closes the reader

        Show
        clehene Cosmin Lehene added a comment - added new patch that closes the reader
        Hide
        lhofhansl Lars Hofhansl added a comment -

        Comment crossing... Cosmin, would it make your life easier if this was in 0.94.0?
        I'd probably just close the reader.

        Show
        lhofhansl Lars Hofhansl added a comment - Comment crossing... Cosmin, would it make your life easier if this was in 0.94.0? I'd probably just close the reader.
        Hide
        lhofhansl Lars Hofhansl added a comment -

        It seems this has been like this. Moving out of 0.94.0. Will pull back in if I need to do another RC (which is likely)

        Show
        lhofhansl Lars Hofhansl added a comment - It seems this has been like this. Moving out of 0.94.0. Will pull back in if I need to do another RC (which is likely)
        Hide
        clehene Cosmin Lehene added a comment -

        Lars,
        It will work, it just won't have compression.

        Regarding inclusion in (any) release: this is a utility, I don't see any risk including it.

        Regarding the reader - yes I should close that or, perhaps move the logic inside the loop's try/catch block to avoid the boilerplate. Let me know if you have any preference.

        Show
        clehene Cosmin Lehene added a comment - Lars, It will work, it just won't have compression. Regarding inclusion in (any) release: this is a utility, I don't see any risk including it. Regarding the reader - yes I should close that or, perhaps move the logic inside the loop's try/catch block to avoid the boilerplate. Let me know if you have any preference.
        Hide
        lhofhansl Lars Hofhansl added a comment -
        +      Algorithm compression = HFile.createReader(fs, hfiles[0],
        +                                new CacheConfig(getConf())).getCompressionAlgorithm();
        

        Do you need to close the reader?

        Show
        lhofhansl Lars Hofhansl added a comment - + Algorithm compression = HFile.createReader(fs, hfiles[0], + new CacheConfig(getConf())).getCompressionAlgorithm(); Do you need to close the reader?
        Hide
        lhofhansl Lars Hofhansl added a comment -

        So it will work, just not start with a compressed table? Asking, because I need to decide whether needs to be in the first 0.94.0RC.

        Show
        lhofhansl Lars Hofhansl added a comment - So it will work, just not start with a compressed table? Asking, because I need to decide whether needs to be in the first 0.94.0RC.
        Hide
        clehene Cosmin Lehene added a comment -

        Lars, it will create a new table without compression. We're adding LZO compressed HFiles and expecting the destination table to inherit that.

        Show
        clehene Cosmin Lehene added a comment - Lars, it will create a new table without compression. We're adding LZO compressed HFiles and expecting the destination table to inherit that.
        Hide
        clehene Cosmin Lehene added a comment -

        Patch for 0.92

        Show
        clehene Cosmin Lehene added a comment - Patch for 0.92
        Hide
        lhofhansl Lars Hofhansl added a comment -

        What are the implications of this? Will it make the created table unreadable?
        Otherwise it will be eventually fixed by compactions.

        Show
        lhofhansl Lars Hofhansl added a comment - What are the implications of this? Will it make the created table unreadable? Otherwise it will be eventually fixed by compactions.

          People

          • Assignee:
            clehene Cosmin Lehene
            Reporter:
            clehene Cosmin Lehene
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 1h
              1h
              Remaining:
              Remaining Estimate - 1h
              1h
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development