HBase
  1. HBase
  2. HBASE-3513

upgrade thrift to 0.5.0 and use mvn version

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.92.0
    • Component/s: Thrift
    • Labels:
      None

      Description

      We should upgrade our thrift to 0.5.0, it is the latest and greatest and is in apache maven repo.

      Doing some testing with a thrift 0.5.0 server, and an older pre-release php client shows the two are on-wire compatible.

      Given that the upgrade is entirely on the server side, and has no wire-impact this should be a relatively low-impact change.

      1. HBASE-3515.txt
        967 kB
        ryan rawson
      2. HBASE-3513.txt
        1.04 MB
        ryan rawson

        Activity

        Hide
        ryan rawson added a comment -

        this patch is a little WIP, it's based on 0.90 with this commit cherry picked in:

        https://github.com/stumbleupon/hbase/commit/67a5b46d069f0e7660d2de998c7b311103708fb4

        Show
        ryan rawson added a comment - this patch is a little WIP, it's based on 0.90 with this commit cherry picked in: https://github.com/stumbleupon/hbase/commit/67a5b46d069f0e7660d2de998c7b311103708fb4
        Hide
        stack added a comment -

        That patch looks fine. I don't see a pom edit though included in your patch.

        Show
        stack added a comment - That patch looks fine. I don't see a pom edit though included in your patch.
        Hide
        stack added a comment -

        Sorry, I meant, the patch looks great. Its cool getting ICV and parallelGet into our thrift interface.

        Show
        stack added a comment - Sorry, I meant, the patch looks great. Its cool getting ICV and parallelGet into our thrift interface.
        Hide
        Lars Francke added a comment -

        This is a duplicate of HBASE-3117.

        Also Thrift 0.5 isn't available in any official repository. The issue is still unresolved (THRIFT-363).

        Show
        Lars Francke added a comment - This is a duplicate of HBASE-3117 . Also Thrift 0.5 isn't available in any official repository. The issue is still unresolved ( THRIFT-363 ).
        Hide
        Lars George added a comment -

        And now there is 0.6.0 which we should use I presume?

        Show
        Lars George added a comment - And now there is 0.6.0 which we should use I presume?
        Hide
        ryan rawson added a comment -

        thrift 0.5.0 is in the apache maven official repo... there was no
        0.6.0 when i went looking.

        Show
        ryan rawson added a comment - thrift 0.5.0 is in the apache maven official repo... there was no 0.6.0 when i went looking.
        Hide
        Lars Francke added a comment -

        Ryan, I'm pretty sure Thrift is not in any repository. As I said the issue is still unresolved and they are looking for help getting it into the repository.

        https://repository.apache.org/index.html#nexus-search;quick~thrift

        Show
        Lars Francke added a comment - Ryan, I'm pretty sure Thrift is not in any repository. As I said the issue is still unresolved and they are looking for help getting it into the repository. https://repository.apache.org/index.html#nexus-search;quick~thrift
        Show
        ryan rawson added a comment - Try this search: https://repository.apache.org/index.html#nexus-search;quick~libthrift
        Hide
        Kazuki Ohta added a comment -

        I prefer 0.6.0 rather than 0.5.0, because THRIFT-923 supports asynchronous client in C++.

        FYI

        Show
        Kazuki Ohta added a comment - I prefer 0.6.0 rather than 0.5.0, because THRIFT-923 supports asynchronous client in C++. https://issues.apache.org/jira/browse/THRIFT-923 https://github.com/apache/thrift/blob/trunk/CHANGES FYI
        Hide
        ryan rawson added a comment -

        a patch against current trunk.

        Show
        ryan rawson added a comment - a patch against current trunk.
        Hide
        ryan rawson added a comment -

        Kazuki, 0.6.0 doesnt exist yet (in maven) so we cant depend on it.

        The on the wire serialization should be the same between 0.5.0 and 0.6.0, so you can use whatever version of C++ client you wish. This is about the hbase serverside thrift version.

        Show
        ryan rawson added a comment - Kazuki, 0.6.0 doesnt exist yet (in maven) so we cant depend on it. The on the wire serialization should be the same between 0.5.0 and 0.6.0, so you can use whatever version of C++ client you wish. This is about the hbase serverside thrift version.
        Hide
        Kazuki Ohta added a comment -

        Hi, ryan. Thanks for the comment.

        > hbase serverside thrift version.

        That's right. I'll test the wire compatibility in my environment.

        Big thanks for your patch anyway!

        Show
        Kazuki Ohta added a comment - Hi, ryan. Thanks for the comment. > hbase serverside thrift version. That's right. I'll test the wire compatibility in my environment. Big thanks for your patch anyway!
        Hide
        stack added a comment -

        +1 on patch (if tests pass). This breaks old thrift clients? Needs to go into INCOMPATIBLE CHANGES section. Add a release note on resolve?

        Show
        stack added a comment - +1 on patch (if tests pass). This breaks old thrift clients? Needs to go into INCOMPATIBLE CHANGES section. Add a release note on resolve?
        Hide
        ryan rawson added a comment -

        it is wire compatible with older clients, tested with an older SU client. There is no actual incompatibility!

        Show
        ryan rawson added a comment - it is wire compatible with older clients, tested with an older SU client. There is no actual incompatibility!
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK #1771 (See https://hudson.apache.org/hudson/job/HBase-TRUNK/1771/)

        Show
        Hudson added a comment - Integrated in HBase-TRUNK #1771 (See https://hudson.apache.org/hudson/job/HBase-TRUNK/1771/ )
        Hide
        Lars Francke added a comment -

        Oh, I never saw your answer. Sorry Ryan I never searched for libthrift, thanks for pointing it out Stack. As this is a custom release by the Hadoop folks are we sure this is not in any way customized by them?

        I haven't looked at the Maven stuff in a while but by being in a custom group we might get a different version by accident as a transitive dependency if any other project decides to do the same as Hadoop.

        Show
        Lars Francke added a comment - Oh, I never saw your answer. Sorry Ryan I never searched for libthrift , thanks for pointing it out Stack. As this is a custom release by the Hadoop folks are we sure this is not in any way customized by them? I haven't looked at the Maven stuff in a while but by being in a custom group we might get a different version by accident as a transitive dependency if any other project decides to do the same as Hadoop.
        Hide
        stack added a comment -

        I didn't notice the thrift 0.5.0 a hadoop built thing.

        Thanks for the warning @LarsF.

        Show
        stack added a comment - I didn't notice the thrift 0.5.0 a hadoop built thing. Thanks for the warning @LarsF.

          People

          • Assignee:
            ryan rawson
            Reporter:
            ryan rawson
          • Votes:
            1 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development