Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.90.0
    • Fix Version/s: 0.90.2
    • Component/s: Thrift
    • Labels:
      None

      Description

      The Thrift API does not expose multi-get operations. This patch adds the meyhods getRows, getRowsWithColumns, getRowsTs and getRowsWithColumnsTs.

      1. HBASE-3542[0.90].patch
        307 kB
        Martin Blom
      2. HBASE-3542[0.90].v2.patch
        147 kB
        Martin Blom

        Activity

        Hide
        Martin Blom added a comment -

        ThriftServer.java and Hbase.thrift patched manually, the other files generated using thrift-0.2.0

        Show
        Martin Blom added a comment - ThriftServer.java and Hbase.thrift patched manually, the other files generated using thrift-0.2.0
        Hide
        stack added a comment -

        @Martin You tested this? Does it work for you?

        We should pull this in for 0.90.2 if its working (though a 'feature' rather than bugfix).

        @Ryan Did you put up your patch for trunk adding multiget to thrift?

        Show
        stack added a comment - @Martin You tested this? Does it work for you? We should pull this in for 0.90.2 if its working (though a 'feature' rather than bugfix). @Ryan Did you put up your patch for trunk adding multiget to thrift?
        Hide
        ryan rawson added a comment -

        not yet, there is a big pile of junk in there that needs to be worked thru, im still testing other things at the moment.

        our api is not that fancy and uses weird names like 'parallelGet'.

        Any implementation though should use multi() to accomplish it's task.

        Show
        ryan rawson added a comment - not yet, there is a big pile of junk in there that needs to be worked thru, im still testing other things at the moment. our api is not that fancy and uses weird names like 'parallelGet'. Any implementation though should use multi() to accomplish it's task.
        Hide
        ryan rawson added a comment -

        some major things:

        • adding new methods to the bottom of the .thrift file means the resulting generated files have a minimum of changes.
        • I'm showing files that are "changed" by virtue of having their license removed and having trailing spaces added. Should be fixed!
        • lots of white space changes in Hbase.java, shouldnt exist. many IDEs can strip whitespace changes for you.
        • indentation in ThriftServer.java is off. The project standard is no tabs, 2 spaces per level, 4 spaces for continuing indent. If you set 2/4 in intellij it will do the right thing, eclipse has similar settings.
        Show
        ryan rawson added a comment - some major things: adding new methods to the bottom of the .thrift file means the resulting generated files have a minimum of changes. I'm showing files that are "changed" by virtue of having their license removed and having trailing spaces added. Should be fixed! lots of white space changes in Hbase.java, shouldnt exist. many IDEs can strip whitespace changes for you. indentation in ThriftServer.java is off. The project standard is no tabs, 2 spaces per level, 4 spaces for continuing indent. If you set 2/4 in intellij it will do the right thing, eclipse has similar settings.
        Hide
        Martin Blom added a comment -

        You prefer the getRows* functions at the bottom and not together with getRow*? I can do that. I was just trying to arrange them like the mutateRow*/mutateRows* functions are arranged.

        Are you also suggesting that I should hand-edit the gernerated code? Or do you have a pointer to the exact version of thrift that was used to generate the current files?

        Indentation in ThriftServer.java should be like the rest of the code in that file, except for the tabs. Sorry about that. ThriftServer.java does not comply to the standard indentation. Should I reformat the whole file?

        Show
        Martin Blom added a comment - You prefer the getRows* functions at the bottom and not together with getRow*? I can do that. I was just trying to arrange them like the mutateRow*/mutateRows* functions are arranged. Are you also suggesting that I should hand-edit the gernerated code? Or do you have a pointer to the exact version of thrift that was used to generate the current files? Indentation in ThriftServer.java should be like the rest of the code in that file, except for the tabs. Sorry about that. ThriftServer.java does not comply to the standard indentation. Should I reformat the whole file?
        Hide
        ryan rawson added a comment -

        I think if you removed trailing spaces from generated files, added in the
        license and double checked the ThriftServer tabs we'll be there.

        Part of the reason to put calls at the bottom is to minimize the changes to
        the huge Hbase.java and make the change easier to understand/rationalize.

        Show
        ryan rawson added a comment - I think if you removed trailing spaces from generated files, added in the license and double checked the ThriftServer tabs we'll be there. Part of the reason to put calls at the bottom is to minimize the changes to the huge Hbase.java and make the change easier to understand/rationalize.
        Hide
        Martin Blom added a comment -

        Deleted trailing spaces from generated code and re-added the license.

        Replaced tabs with spaces in ThriftServer.java.

        Show
        Martin Blom added a comment - Deleted trailing spaces from generated code and re-added the license. Replaced tabs with spaces in ThriftServer.java.
        Hide
        Martin Blom added a comment -

        I used ".../thrift-0.2.0/bin/thrift --gen java:hashcode .../Hbase.thrift" to generate the code, then removed trailing spaces and added the license.

        The generated changes in AlreadyExists.java are rather pointless, so maybe it should be excluded.

        Show
        Martin Blom added a comment - I used ".../thrift-0.2.0/bin/thrift --gen java:hashcode .../Hbase.thrift" to generate the code, then removed trailing spaces and added the license. The generated changes in AlreadyExists.java are rather pointless, so maybe it should be excluded.
        Hide
        stack added a comment -

        Marking patch available; just needs last review.

        Show
        stack added a comment - Marking patch available; just needs last review.
        Hide
        ryan rawson added a comment -

        trunk and branch

        Show
        ryan rawson added a comment - trunk and branch
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK #1748 (See https://hudson.apache.org/hudson/job/HBase-TRUNK/1748/)
        HBASE-3542 MultiGet methods in Thrift

        Show
        Hudson added a comment - Integrated in HBase-TRUNK #1748 (See https://hudson.apache.org/hudson/job/HBase-TRUNK/1748/ ) HBASE-3542 MultiGet methods in Thrift

          People

          • Assignee:
            Martin Blom
            Reporter:
            Martin Blom
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development