Pig
  1. Pig
  2. PIG-4005

depend on hbase-hadoop2-compat rather than hbase-hadoop1-compat when hbaseversion is 95

    Details

    • Type: Wish Wish
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.12.0
    • Fix Version/s: 0.13.0, 0.14.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      If users set hbaseversion to 95, shall we use hbase-hadoop2-compat instead of hbase-hadoop1-compat?

      1. PIG-4005-1.patch
        3 kB
        Daniel Dai
      2. PIG-4005-2.patch
        3 kB
        Daniel Dai
      3. PIG-4005-3.patch
        1 kB
        Daniel Dai

        Activity

        Hide
        Jarek Jarcec Cecho added a comment -

        That depends on Hadoop version that you're using and not on HBase version. If you're using Hadoop 1.x, then you should use hbase-hadoop1-compat, if you're using Hadoop 2.x then you should use hbase-hadoop2-compat.

        Show
        Jarek Jarcec Cecho added a comment - That depends on Hadoop version that you're using and not on HBase version. If you're using Hadoop 1.x, then you should use hbase-hadoop1-compat , if you're using Hadoop 2.x then you should use hbase-hadoop2-compat .
        Hide
        Xiaoshuang LU added a comment -

        Thanks Jarek. `hbase-hadoop1-compat' and `hbase-hadoop2-compat' have the same functionality and interface, right?

        Show
        Xiaoshuang LU added a comment - Thanks Jarek. `hbase-hadoop1-compat' and `hbase-hadoop2-compat' have the same functionality and interface, right?
        Hide
        Daniel Dai added a comment -

        Attach patch. Also change default hbaseversion to 95 since it has been out for a while, and it works with both hadoop 1 & 2 which will make Pig's life easier. With this change, I enabled TestHBaseStorage when hadoopversion=23, but note if hbaseversion=94 and hadoopversion=23, the test will fail. In the future, we can hardcode hbaseversion to 95 and simplify the build.xml.

        Show
        Daniel Dai added a comment - Attach patch. Also change default hbaseversion to 95 since it has been out for a while, and it works with both hadoop 1 & 2 which will make Pig's life easier. With this change, I enabled TestHBaseStorage when hadoopversion=23, but note if hbaseversion=94 and hadoopversion=23, the test will fail. In the future, we can hardcode hbaseversion to 95 and simplify the build.xml.
        Hide
        Rohini Palaniswamy added a comment -

        Two minor comments:

        • Can we rename hbase95.hadoop.version to hbase.hadoop.version? I guess it can be common even if another version like hbase98 is added to the mix.
        • +hbase95.version=0.96.0-$ {hbase95.hadoop.version}

          - It is confusing to call it hbase95.version when it is hbase96. Can we rename references of hbase95 to hbase96?

        Show
        Rohini Palaniswamy added a comment - Two minor comments: Can we rename hbase95.hadoop.version to hbase.hadoop.version? I guess it can be common even if another version like hbase98 is added to the mix. +hbase95.version=0.96.0-$ {hbase95.hadoop.version} - It is confusing to call it hbase95.version when it is hbase96. Can we rename references of hbase95 to hbase96?
        Hide
        Daniel Dai added a comment -

        Actually I want to remove the hbaseversion in future release and stick with hbaseversion=95 layout. Maintaining both hbase layout is costly. "hbase95.hadoop.version" is temporary, I am opening to rename it to "hbase.hadoop.version" but that's also confusing since this flag only works with hbaseversion=95 layout.

        I can change hbase95.version to hbase96.version, again, that is still confusing since user will use -Dhbaseversion=95 to compile.

        Show
        Daniel Dai added a comment - Actually I want to remove the hbaseversion in future release and stick with hbaseversion=95 layout. Maintaining both hbase layout is costly. "hbase95.hadoop.version" is temporary, I am opening to rename it to "hbase.hadoop.version" but that's also confusing since this flag only works with hbaseversion=95 layout. I can change hbase95.version to hbase96.version, again, that is still confusing since user will use -Dhbaseversion=95 to compile.
        Hide
        Rohini Palaniswamy added a comment -

        Ok. I guess it is ok to leave hbase95.version as it was that way before. We should rename hbase.hadoop.version though. We will soon be adding hbase98 and does not make sense to add hbase98.hadoop.version then.

        Show
        Rohini Palaniswamy added a comment - Ok. I guess it is ok to leave hbase95.version as it was that way before. We should rename hbase.hadoop.version though. We will soon be adding hbase98 and does not make sense to add hbase98.hadoop.version then.
        Hide
        Daniel Dai added a comment -

        Addressing Rohini's review comments.

        Show
        Daniel Dai added a comment - Addressing Rohini's review comments.
        Hide
        Rohini Palaniswamy added a comment -

        +1

        Show
        Rohini Palaniswamy added a comment - +1
        Hide
        Daniel Dai added a comment -

        Patch committed to both 0.13 branch and trunk. Thanks Rohini for review!

        Show
        Daniel Dai added a comment - Patch committed to both 0.13 branch and trunk. Thanks Rohini for review!
        Hide
        Rohini Palaniswamy added a comment -

        Test fails with class org.apache.hadoop.hbase.protobuf.generated.FSProtos$HBaseVersionFileContent overrides final method getUnknownFields.()Lcom/google/protobuf/UnknownFieldSet; in branch-0.13 with hadoopversion=23 as it has protobuf 2.4. I guess that is ok as we never had TestHBaseStorage running for hadoopversion=23 till now and more testing will be required if we change protobuf version on branch now.

        In trunk I see - Tests run: 26, Failures: 5, Errors: 1, Skipped: 0, Time elapsed: 80.308 sec. Created PIG-4037 for that.

        Show
        Rohini Palaniswamy added a comment - Test fails with class org.apache.hadoop.hbase.protobuf.generated.FSProtos$HBaseVersionFileContent overrides final method getUnknownFields.()Lcom/google/protobuf/UnknownFieldSet; in branch-0.13 with hadoopversion=23 as it has protobuf 2.4. I guess that is ok as we never had TestHBaseStorage running for hadoopversion=23 till now and more testing will be required if we change protobuf version on branch now. In trunk I see - Tests run: 26, Failures: 5, Errors: 1, Skipped: 0, Time elapsed: 80.308 sec. Created PIG-4037 for that.
        Hide
        Daniel Dai added a comment -

        On trunk, it is still due to mixed mode in TestHBaseStorage. Attach a fix.

        On 0.13 branch, I don't have a solution unless we upgrade hadoop version, but that seems too late for 0.13.

        Show
        Daniel Dai added a comment - On trunk, it is still due to mixed mode in TestHBaseStorage. Attach a fix. On 0.13 branch, I don't have a solution unless we upgrade hadoop version, but that seems too late for 0.13.
        Hide
        Daniel Dai added a comment -

        Note on 0.13 branch, TestHBaseStorage is still disabled for Hadoop 2, in that:

        • When hbaseversion=94, we don't have hbase build for Hadoop 2, test fail
        • When hbaseversion=95, test fail due to protobuf version conflict, need to upgrade hadoop from 2.0.3-alpha to 2.4.0, but it is too late for 0.13.0 (we upgrade that on trunk)
        Show
        Daniel Dai added a comment - Note on 0.13 branch, TestHBaseStorage is still disabled for Hadoop 2, in that: When hbaseversion=94, we don't have hbase build for Hadoop 2, test fail When hbaseversion=95, test fail due to protobuf version conflict, need to upgrade hadoop from 2.0.3-alpha to 2.4.0, but it is too late for 0.13.0 (we upgrade that on trunk)

          People

          • Assignee:
            Daniel Dai
            Reporter:
            Xiaoshuang LU
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development