HBase
  1. HBase
  2. HBASE-5656

LoadIncrementalHFiles createTable should detect and set compression algorithm

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major 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 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 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 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 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 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 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 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 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 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 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 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 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
        Lars Hofhansl added a comment -

        Committed to 0.92, 0.94, and 0.96.

        Show
        Lars Hofhansl added a comment - Committed to 0.92, 0.94, and 0.96.
        Hide
        Ted Yu added a comment -

        Latest patch looks good to me.

        Show
        Ted Yu added a comment - Latest patch looks good to me.
        Hide
        Lars Hofhansl added a comment -

        +1 on patch. Failures seems unrelated.

        Show
        Lars Hofhansl added a comment - +1 on patch. Failures seems unrelated.
        Hide
        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
        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
        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
        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
        Cosmin Lehene added a comment -

        Attached new patch that includes logging.

        Show
        Cosmin Lehene added a comment - Attached new patch that includes logging.
        Hide
        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
        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
        Ted Yu added a comment -

        Latest patch applies to trunk cleanly.

        Show
        Ted Yu added a comment - Latest patch applies to trunk cleanly.
        Hide
        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
        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
        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
        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
        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
        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
        Lars Hofhansl added a comment -

        How about this? Super simple.

        Show
        Lars Hofhansl added a comment - How about this? Super simple.
        Hide
        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
        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
        Cosmin Lehene added a comment -

        added new patch that closes the reader

        Show
        Cosmin Lehene added a comment - added new patch that closes the reader
        Hide
        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
        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
        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
        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
        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
        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
        Lars Hofhansl added a comment -
        +      Algorithm compression = HFile.createReader(fs, hfiles[0],
        +                                new CacheConfig(getConf())).getCompressionAlgorithm();
        

        Do you need to close the reader?

        Show
        Lars Hofhansl added a comment - + Algorithm compression = HFile.createReader(fs, hfiles[0], + new CacheConfig(getConf())).getCompressionAlgorithm(); Do you need to close the reader?
        Hide
        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
        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
        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
        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
        Cosmin Lehene added a comment -

        Patch for 0.92

        Show
        Cosmin Lehene added a comment - Patch for 0.92
        Hide
        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
        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:
            Cosmin Lehene
            Reporter:
            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