Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.95.0
    • Component/s: hbck, HFile, migration
    • Labels:
      None

      Description

      HFileV1 should be removed from the regionserver because it is somewhat of a drag on development for working on the lower level read paths. It's an impediment to cleaning up the Store code.

      V1 HFiles ceased to be written in 0.92, but the V1 reader was left in place so users could upgrade from 0.90 to 0.92. Once all HFiles are compacted in 0.92, then the V1 code is no longer needed. We then decided to leave the V1 code in place in 0.94 so users could upgrade directly from 0.90 to 0.94. The code is still there in trunk but should probably be shown the door. I see a few options:

      1) just delete the code and tell people to make sure they compact everything using 0.92 or 0.94
      2) create a standalone script that people can run on their 0.92 or 0.94 cluster that iterates the filesystem and prints out any v1 files with a message that the user should run a major compaction
      3) add functionality to 0.96.0 (first release, maybe in hbck) that proactively kills v1 files, so that we can be sure there are none when upgrading from 0.96 to 0.98
      4) punt to 0.98 and probably do one of the above options in a year

      I would vote for #1 or #2 which will allow us to have a v1-free 0.96.0. HFileV1 has already survived 2 major release upgrades which i think many would agree is more than enough for a pre-1.0, free product. If we can remove it in 0.96.0 it will be out of the way to introduce some nice performance improvements in subsequent 0.96.x releases.

      1. 7660-v4.txt
        65 kB
        Ted Yu
      2. 7660-v3.txt
        65 kB
        Ted Yu
      3. 7660-v2.txt
        64 kB
        Ted Yu
      4. 7660-v1.txt
        51 kB
        Ted Yu

        Issue Links

          Activity

          Hide
          Ted Yu added a comment -

          I would vote for option #2.

          Show
          Ted Yu added a comment - I would vote for option #2.
          Hide
          Matt Corgan added a comment -

          I guess the question for #2 is how to package and distribute such a script.

          Show
          Matt Corgan added a comment - I guess the question for #2 is how to package and distribute such a script.
          Hide
          Matt Corgan added a comment -

          Have there been a favorite migration strategy in the past? Could we tell people to save the script into one server's hbase/bin directory and run it? Could it be the same script for 0.90, 0.92, 0.94?

          Show
          Matt Corgan added a comment - Have there been a favorite migration strategy in the past? Could we tell people to save the script into one server's hbase/bin directory and run it? Could it be the same script for 0.90, 0.92, 0.94?
          Hide
          Jonathan Hsieh added a comment - - edited

          When HDFS upgrades incompatibly (hadoop1 -> hadoop2), it happens in is in "upgrade mode" and "finalize" command that I assume forces everything to the newer format. Once finalized, you can't go back but you should be safe to go forward. Before finalized, I believe it continues to only write in the older format. This feels like #3 (except front loaded instead of lazy before the next upgrade).

          We've actually run into some HFile format problems between 0.92 and 0.94 recently – Elliott Clark's HBASE-7647, and I believe he's found another.

          Show
          Jonathan Hsieh added a comment - - edited When HDFS upgrades incompatibly (hadoop1 -> hadoop2), it happens in is in "upgrade mode" and "finalize" command that I assume forces everything to the newer format. Once finalized, you can't go back but you should be safe to go forward. Before finalized, I believe it continues to only write in the older format. This feels like #3 (except front loaded instead of lazy before the next upgrade). We've actually run into some HFile format problems between 0.92 and 0.94 recently – Elliott Clark 's HBASE-7647 , and I believe he's found another.
          Hide
          Andrew Purtell added a comment -

          Would removing V1 involve more than removing the reader and writer? Would touch the abstract classes?

          Show
          Andrew Purtell added a comment - Would removing V1 involve more than removing the reader and writer? Would touch the abstract classes?
          Hide
          Matt Corgan added a comment -

          I think for this jira we'd only want to remove HFileReaderV1, HFileWriterV1, and FSReaderV1. The main goal being to forcibly drop support for them before branching 0.96. It would leave only one implementation of the interfaces and abstract classes.

          Later, we can go back and look at the purpose of the interfaces and abstract classes. I'm guessing they are partly designed specifically as a shim layer between v1/v2. A shim layer between v2 and future unknown v3 may have completely different needs.

          Getting out of the scope of this jira - we may want to flatten the interface, abstractClass, and implementation to make it easier to clean up the Store code, and then pull out a different interface in the future when we actually have a use case for it.

          Show
          Matt Corgan added a comment - I think for this jira we'd only want to remove HFileReaderV1, HFileWriterV1, and FSReaderV1. The main goal being to forcibly drop support for them before branching 0.96. It would leave only one implementation of the interfaces and abstract classes. Later, we can go back and look at the purpose of the interfaces and abstract classes. I'm guessing they are partly designed specifically as a shim layer between v1/v2. A shim layer between v2 and future unknown v3 may have completely different needs. Getting out of the scope of this jira - we may want to flatten the interface, abstractClass, and implementation to make it easier to clean up the Store code, and then pull out a different interface in the future when we actually have a use case for it.
          Hide
          Matteo Bertozzi added a comment -

          I agree that the old code limits the evolution.
          Is there a way to move the code out without too much work,
          and put it in something like hbase-support/hfile-v1-to-v2-tool that can be used as migration tool?

          Show
          Matteo Bertozzi added a comment - I agree that the old code limits the evolution. Is there a way to move the code out without too much work, and put it in something like hbase-support/hfile-v1-to-v2-tool that can be used as migration tool?
          Hide
          Ted Yu added a comment -

          Looking at HFile.Reader, most methods don't have javadoc, leaving interpretation of the interface to each developer.

          In the process of creating Cell interface, we have avoided such mistake.

          put it in something like hbase-support/hfile-v1-to-v2-tool that can be used as migration tool?

          Some users are using 0.94.x and to my knowledge, there was no request for creating migration tool.

          Show
          Ted Yu added a comment - Looking at HFile.Reader, most methods don't have javadoc, leaving interpretation of the interface to each developer. In the process of creating Cell interface, we have avoided such mistake. put it in something like hbase-support/hfile-v1-to-v2-tool that can be used as migration tool? Some users are using 0.94.x and to my knowledge, there was no request for creating migration tool.
          Hide
          Matt Corgan added a comment -

          Should have mentioned in the description: One could probably reason that if someone is using 0.90.x right now, they are looking for max stability and will not jump straight to 0.96, but rather go to 0.94.5+ first.

          Regarding distributing the script: We could bundle the HFileV1-scanner script in future 0.94 version as well as make it downloadable to hbase/bin from the hbase website for people on 0.92 and earlier 0.94 versions.

          Is there a way to move the code out without too much work, and put it in something like hbase-support/hfile-v1-to-v2-tool that can be used as migration tool?

          Sounds like a good idea, though I wonder if anyone should spend time creating it before anyone actually requests it.

          Show
          Matt Corgan added a comment - Should have mentioned in the description: One could probably reason that if someone is using 0.90.x right now, they are looking for max stability and will not jump straight to 0.96, but rather go to 0.94.5+ first. Regarding distributing the script: We could bundle the HFileV1-scanner script in future 0.94 version as well as make it downloadable to hbase/bin from the hbase website for people on 0.92 and earlier 0.94 versions. Is there a way to move the code out without too much work, and put it in something like hbase-support/hfile-v1-to-v2-tool that can be used as migration tool? Sounds like a good idea, though I wonder if anyone should spend time creating it before anyone actually requests it.
          Hide
          Andrew Purtell added a comment -

          I think for this jira we'd only want to remove HFileReaderV1, HFileWriterV1, and FSReaderV1. The main goal being to forcibly drop support for them before branching 0.96.

          This sounds like a good idea. I was worried about more substantial changes because I am currently working in HFile V2. That said, with V1 gone it would be a good thing that we can simplify and remove code in this area over time.

          hbase-support/hfile-v1-to-v2-tool that can be used as migration tool

          This would seem like working against the goal to remove the old code. I think anyone upgrading to 0.96 would be doing it from 0.92 or 0.94, which auto-upgrades HFiles. We could document in release notes that before upgrading the user should run a major compaction.

          Show
          Andrew Purtell added a comment - I think for this jira we'd only want to remove HFileReaderV1, HFileWriterV1, and FSReaderV1. The main goal being to forcibly drop support for them before branching 0.96. This sounds like a good idea. I was worried about more substantial changes because I am currently working in HFile V2. That said, with V1 gone it would be a good thing that we can simplify and remove code in this area over time. hbase-support/hfile-v1-to-v2-tool that can be used as migration tool This would seem like working against the goal to remove the old code. I think anyone upgrading to 0.96 would be doing it from 0.92 or 0.94, which auto-upgrades HFiles. We could document in release notes that before upgrading the user should run a major compaction.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12566651/7660-v1.txt
          against trunk revision .

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

          +1 tests included. The patch appears to include 6 new or modified tests.

          +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

          +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 lineLengths. The patch does not introduce lines longer than 100

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.regionserver.TestStoreFile
          org.apache.hadoop.hbase.regionserver.TestSplitTransaction
          org.apache.hadoop.hbase.io.hfile.TestHFileReaderV1

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4202//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4202//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4202//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4202//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4202//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4202//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4202//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4202//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/12566651/7660-v1.txt against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 6 new or modified tests. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. +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 lineLengths . The patch does not introduce lines longer than 100 -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.regionserver.TestStoreFile org.apache.hadoop.hbase.regionserver.TestSplitTransaction org.apache.hadoop.hbase.io.hfile.TestHFileReaderV1 Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4202//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4202//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4202//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4202//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4202//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4202//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4202//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4202//console This message is automatically generated.
          Hide
          Ted Yu added a comment -

          stack:
          Can you provide your opinion ?

          Show
          Ted Yu added a comment - stack : Can you provide your opinion ?
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12566653/7660-v2.txt
          against trunk revision .

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

          +1 tests included. The patch appears to include 21 new or modified tests.

          +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

          +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 lineLengths. The patch does not introduce lines longer than 100

          +1 core tests. The patch passed unit tests in .

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4203//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4203//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4203//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4203//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4203//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4203//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4203//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4203//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/12566653/7660-v2.txt against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 21 new or modified tests. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. +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 lineLengths . The patch does not introduce lines longer than 100 +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4203//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4203//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4203//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4203//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4203//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4203//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4203//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4203//console This message is automatically generated.
          Hide
          Matt Corgan added a comment -

          Looks good to me Ted. Thanks for spending time on it.

          Only comment is that maybe you should preserve the constant HFileWriterV1.BLOOM_FILTER_DATA_KEY. Unless you had a specific reason, might be better to move it to HFileWriterV2 instead of the following:

          -          bloom = reader.getMetaBlock(HFileWriterV1.BLOOM_FILTER_DATA_KEY,
          +          bloom = reader.getMetaBlock("BLOOM_FILTER_DATA",
                         true);
          
          Show
          Matt Corgan added a comment - Looks good to me Ted. Thanks for spending time on it. Only comment is that maybe you should preserve the constant HFileWriterV1.BLOOM_FILTER_DATA_KEY. Unless you had a specific reason, might be better to move it to HFileWriterV2 instead of the following: - bloom = reader.getMetaBlock(HFileWriterV1.BLOOM_FILTER_DATA_KEY, + bloom = reader.getMetaBlock( "BLOOM_FILTER_DATA" , true );
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12566658/7660-v3.txt
          against trunk revision .

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

          +1 tests included. The patch appears to include 21 new or modified tests.

          +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

          +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 lineLengths. The patch does not introduce lines longer than 100

          +1 core tests. The patch passed unit tests in .

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4204//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4204//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4204//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4204//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4204//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4204//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4204//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4204//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/12566658/7660-v3.txt against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 21 new or modified tests. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. +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 lineLengths . The patch does not introduce lines longer than 100 +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4204//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4204//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4204//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4204//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4204//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4204//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4204//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4204//console This message is automatically generated.
          Hide
          Andrew Purtell added a comment -

          Only comment is that maybe you should preserve the constant HFileWriterV1.BLOOM_FILTER_DATA_KEY. Unless you had a specific reason, might be better to move it to HFileWriterV2 instead of the following:

          • bloom = reader.getMetaBlock(HFileWriterV1.BLOOM_FILTER_DATA_KEY,
            + bloom = reader.getMetaBlock("BLOOM_FILTER_DATA",
            true);

          While on this, it would be a good idea to consolidate these constants from HFileWriterV1 and HFileWriterV2 into HFile.

          Show
          Andrew Purtell added a comment - Only comment is that maybe you should preserve the constant HFileWriterV1.BLOOM_FILTER_DATA_KEY. Unless you had a specific reason, might be better to move it to HFileWriterV2 instead of the following: bloom = reader.getMetaBlock(HFileWriterV1.BLOOM_FILTER_DATA_KEY, + bloom = reader.getMetaBlock("BLOOM_FILTER_DATA", true); While on this, it would be a good idea to consolidate these constants from HFileWriterV1 and HFileWriterV2 into HFile.
          Hide
          Ted Yu added a comment -

          How about moving HFileWriterV1.BLOOM_FILTER_DATA_KEY into HFile in this JIRA and handle the other constants in separate issue ?

          Show
          Ted Yu added a comment - How about moving HFileWriterV1.BLOOM_FILTER_DATA_KEY into HFile in this JIRA and handle the other constants in separate issue ?
          Hide
          Andrew Purtell added a comment -

          How about moving HFileWriterV1.BLOOM_FILTER_DATA_KEY into HFile in this JIRA and handle the other constants in separate issue ?

          At your preference Ted, sounds good.

          Show
          Andrew Purtell added a comment - How about moving HFileWriterV1.BLOOM_FILTER_DATA_KEY into HFile in this JIRA and handle the other constants in separate issue ? At your preference Ted, sounds good.
          Hide
          Ted Yu added a comment -

          Patch v4 moves BLOOM_FILTER_DATA_KEY to HFile.

          Show
          Ted Yu added a comment - Patch v4 moves BLOOM_FILTER_DATA_KEY to HFile.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12566824/7660-v4.txt
          against trunk revision .

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

          +1 tests included. The patch appears to include 21 new or modified tests.

          +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

          +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 lineLengths. The patch does not introduce lines longer than 100

          +1 core tests. The patch passed unit tests in .

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4216//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4216//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4216//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4216//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4216//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4216//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4216//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4216//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/12566824/7660-v4.txt against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 21 new or modified tests. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. +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 lineLengths . The patch does not introduce lines longer than 100 +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4216//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4216//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4216//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4216//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4216//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4216//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4216//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4216//console This message is automatically generated.
          Hide
          Matt Corgan added a comment -

          Looks good Ted.

          Show
          Matt Corgan added a comment - Looks good Ted.
          Hide
          Ted Yu added a comment -

          Thanks for the reviews, Matt and Andy.

          Plan to integrate tomorrow.

          Show
          Ted Yu added a comment - Thanks for the reviews, Matt and Andy. Plan to integrate tomorrow.
          Hide
          stack added a comment -

          Regards the migration 'tool', or 'tool' to check for presence of v1 files, I imagine it as an addition to the hfile tool http://hbase.apache.org/book.html#hfile_tool2 The hfile tool already takes a bunch of args including printing out meta. We could add an option to print out version only – or return 1 if version 1 or some such – and then do a bit of code to just list all hfiles and run this script against each. Could MR it if too many files.

          I'd see this tool as a blocker on 0.96. Have we filed an issue for it?

          On the patch:

          testBloomEdgeCases can be removed because? I don't see obvious v1 reference.

          Rather than remove these lines:

          • case 1:
          • return new HFileWriterV1.WriterFactoryV1(conf, cacheConf);

          ... throw an unsupportedoperationexception or some such? Maybe it is ok because we fall through to CorruptHFileException?

          If so, that is fine.

          I'm +1 on commit.

          Show
          stack added a comment - Regards the migration 'tool', or 'tool' to check for presence of v1 files, I imagine it as an addition to the hfile tool http://hbase.apache.org/book.html#hfile_tool2 The hfile tool already takes a bunch of args including printing out meta. We could add an option to print out version only – or return 1 if version 1 or some such – and then do a bit of code to just list all hfiles and run this script against each. Could MR it if too many files. I'd see this tool as a blocker on 0.96. Have we filed an issue for it? On the patch: testBloomEdgeCases can be removed because? I don't see obvious v1 reference. Rather than remove these lines: case 1: return new HFileWriterV1.WriterFactoryV1(conf, cacheConf); ... throw an unsupportedoperationexception or some such? Maybe it is ok because we fall through to CorruptHFileException? If so, that is fine. I'm +1 on commit.
          Hide
          Ted Yu added a comment -

          For testBloomEdgeCases:

          -    // This test only runs for HFile format version 1.
          -    conf.setInt(HFile.FORMAT_VERSION_KEY, 1);
          

          For:

          -    case 1:
          -      return new HFileWriterV1.WriterFactoryV1(conf, cacheConf);
          

          the control would fall through:

              default:
                throw new IllegalArgumentException("Cannot create writer for HFile " +
                    "format version " + version);
          

          The migration tool would be handled separately.

          Show
          Ted Yu added a comment - For testBloomEdgeCases: - // This test only runs for HFile format version 1. - conf.setInt(HFile.FORMAT_VERSION_KEY, 1); For: - case 1: - return new HFileWriterV1.WriterFactoryV1(conf, cacheConf); the control would fall through: default : throw new IllegalArgumentException( "Cannot create writer for HFile " + "format version " + version); The migration tool would be handled separately.
          Hide
          stack added a comment -

          +1 on commit. File blocker issue for tool. Thanks.

          Show
          stack added a comment - +1 on commit. File blocker issue for tool. Thanks.
          Hide
          Ted Yu added a comment -

          Integrated to trunk.

          Thanks for the reviews, Andy, Matt and Stack.

          Show
          Ted Yu added a comment - Integrated to trunk. Thanks for the reviews, Andy, Matt and Stack.
          Hide
          Matt Corgan added a comment -

          Great work Ted. I think this will free us up to make some big improvements to the Store/Region.

          Show
          Matt Corgan added a comment - Great work Ted. I think this will free us up to make some big improvements to the Store/Region.
          Hide
          Hudson added a comment -

          Integrated in HBase-TRUNK #3821 (See https://builds.apache.org/job/HBase-TRUNK/3821/)
          HBASE-7660 Remove HFileV1 code (Ted Yu) (Revision 1439753)

          Result = FAILURE
          tedyu :
          Files :

          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlock.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestUpgradeFromHFileV1ToEncoding.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestForceCacheImportantBlocks.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlock.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockCompatibility.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java
          Show
          Hudson added a comment - Integrated in HBase-TRUNK #3821 (See https://builds.apache.org/job/HBase-TRUNK/3821/ ) HBASE-7660 Remove HFileV1 code (Ted Yu) (Revision 1439753) Result = FAILURE tedyu : Files : /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlock.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestUpgradeFromHFileV1ToEncoding.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestForceCacheImportantBlocks.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlock.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockCompatibility.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java
          Hide
          Hudson added a comment -

          Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #382 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/382/)
          HBASE-7660 Remove HFileV1 code (Ted Yu) (Revision 1439753)

          Result = FAILURE
          tedyu :
          Files :

          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlock.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestUpgradeFromHFileV1ToEncoding.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestForceCacheImportantBlocks.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlock.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockCompatibility.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java
          Show
          Hudson added a comment - Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #382 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/382/ ) HBASE-7660 Remove HFileV1 code (Ted Yu) (Revision 1439753) Result = FAILURE tedyu : Files : /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlock.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV1.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestUpgradeFromHFileV1ToEncoding.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestForceCacheImportantBlocks.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlock.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockCompatibility.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileReaderV1.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java
          Hide
          stack added a comment -

          Committed a while back. Resolving.

          Show
          stack added a comment - Committed a while back. Resolving.
          Hide
          stack added a comment -

          Marking closed.

          Show
          stack added a comment - Marking closed.

            People

            • Assignee:
              Ted Yu
              Reporter:
              Matt Corgan
            • Votes:
              0 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development