Hive
  1. Hive
  2. HIVE-2799

change the following thrift apis to add a region

    Details

    • Type: New Feature New Feature
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Not a Problem
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Metastore, Thrift API
    • Labels:
      None

      Description

      list<string> get_tables(1: string db_name, 2: string pattern) throws (1: MetaException o1)
      list<string> get_all_tables(1: string db_name) throws (1: MetaException o1)
      Table get_table(1:string dbname, 2:string tbl_name)
      throws (1:MetaException o1, 2:NoSuchObjectException o2)
      list<Table> get_table_objects_by_name(1:string dbname, 2:list<string> tbl_names)
      throws (1:MetaException o1, 2:InvalidOperationException o2, 3:UnknownDBException o3)
      list<string> get_table_names_by_filter(1:string dbname, 2:string filter, 3:i16 max_tables=-1)
      throws (1:MetaException o1, 2:InvalidOperationException o2, 3:UnknownDBException o3)
      Partition add_partition(1:Partition new_part)
      throws(1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
      i32 add_partitions(1:list<Partition> new_parts)
      throws(1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
      Partition append_partition(1:string db_name, 2:string tbl_name, 3:list<string> part_vals)
      throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
      Partition append_partition_by_name(1:string db_name, 2:string tbl_name, 3:string part_name)
      throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
      bool drop_partition(1:string db_name, 2:string tbl_name, 3:list<string> part_vals, 4:bool deleteData)
      throws(1:NoSuchObjectException o1, 2:MetaException o2)
      bool drop_partition_by_name(1:string db_name, 2:string tbl_name, 3:string part_name, 4:bool deleteData)
      throws(1:NoSuchObjectException o1, 2:MetaException o2)
      Partition get_partition(1:string db_name, 2:string tbl_name, 3:list<string> part_vals)
      throws(1:MetaException o1, 2:NoSuchObjectException o2)

      Partition get_partition_with_auth(1:string db_name, 2:string tbl_name, 3:list<string> part_vals,
      4: string user_name, 5: list<string> group_names) throws(1:MetaException o1, 2:NoSuchObjectException o2)

      Partition get_partition_by_name(1:string db_name 2:string tbl_name, 3:string part_name)
      throws(1:MetaException o1, 2:NoSuchObjectException o2)
      list<Partition> get_partitions(1:string db_name, 2:string tbl_name, 3:i16 max_parts=-1)
      throws(1:NoSuchObjectException o1, 2:MetaException o2)
      list<Partition> get_partitions_with_auth(1:string db_name, 2:string tbl_name, 3:i16 max_parts=-1,
      4: string user_name, 5: list<string> group_names) throws(1:NoSuchObjectException o1, 2:MetaException o2)

      list<string> get_partition_names(1:string db_name, 2:string tbl_name, 3:i16 max_parts=-1)
      throws(1:MetaException o2)
      list<Partition> get_partitions_ps(1:string db_name 2:string tbl_name
      3:list<string> part_vals, 4:i16 max_parts=-1)
      throws(1:MetaException o1, 2:NoSuchObjectException o2)
      list<Partition> get_partitions_ps_with_auth(1:string db_name, 2:string tbl_name, 3:list<string> part_vals, 4:i16 max_parts=-1,
      5: string user_name, 6: list<string> group_names) throws(1:NoSuchObjectException o1, 2:MetaException o2)

      list<string> get_partition_names_ps(1:string db_name,
      2:string tbl_name, 3:list<string> part_vals, 4:i16 max_parts=-1)
      throws(1:MetaException o1, 2:NoSuchObjectException o2)
      list<Partition> get_partitions_by_filter(1:string db_name 2:string tbl_name
      3:string filter, 4:i16 max_parts=-1)
      throws(1:MetaException o1, 2:NoSuchObjectException o2)
      list<Partition> get_partitions_by_names(1:string db_name 2:string tbl_name 3:list<string> names)
      throws(1:MetaException o1, 2:NoSuchObjectException o2)
      bool drop_index_by_name(1:string db_name, 2:string tbl_name, 3:string index_name, 4:bool deleteData)
      throws(1:NoSuchObjectException o1, 2:MetaException o2)
      Index get_index_by_name(1:string db_name 2:string tbl_name, 3:string index_name)
      throws(1:MetaException o1, 2:NoSuchObjectException o2)

      list<Index> get_indexes(1:string db_name, 2:string tbl_name, 3:i16 max_indexes=-1)
      throws(1:NoSuchObjectException o1, 2:MetaException o2)
      list<string> get_index_names(1:string db_name, 2:string tbl_name, 3:i16 max_indexes=-1)
      throws(1:MetaException o2)

        Activity

        Hide
        Ashutosh Chauhan added a comment -

        As a developer of Hive, I am definitely in favor of keeping code simpler and interfaces cleaner. As an open source user, I am fine with wrapping the new calls in existing apis.

        Show
        Ashutosh Chauhan added a comment - As a developer of Hive, I am definitely in favor of keeping code simpler and interfaces cleaner. As an open source user, I am fine with wrapping the new calls in existing apis.
        Hide
        Kevin Wilfong added a comment -

        I'm definitely in favor of that approach. The primary reason I intended to add duplicate Thrift API calls was to keep open source users happy, but if people are content with simply wrapping them in HiveMetaStoreClient, I am more than happy to oblige.

        Show
        Kevin Wilfong added a comment - I'm definitely in favor of that approach. The primary reason I intended to add duplicate Thrift API calls was to keep open source users happy, but if people are content with simply wrapping them in HiveMetaStoreClient, I am more than happy to oblige.
        Hide
        Ashutosh Chauhan added a comment -

        For that we can modify HiveMetaStoreClient.java (the most widely used client) to wrap these methods in the one which don't take region as an argument (which is the current api) and then passing null for server param through rpc client. Those folks who are using real rpc clients their clients will continue to work without recompilation and if they are indeed recompiling they can pass on a null in there.

        At this point, I think we should reconsider whether we want to add a new set of apis or want to modify the existing ones. To me, latter seems a better choice to avoid code duplication and confusion.

        Show
        Ashutosh Chauhan added a comment - For that we can modify HiveMetaStoreClient.java (the most widely used client) to wrap these methods in the one which don't take region as an argument (which is the current api) and then passing null for server param through rpc client. Those folks who are using real rpc clients their clients will continue to work without recompilation and if they are indeed recompiling they can pass on a null in there. At this point, I think we should reconsider whether we want to add a new set of apis or want to modify the existing ones. To me, latter seems a better choice to avoid code duplication and confusion.
        Hide
        Kevin Wilfong added a comment -

        @Ashutosh

        I agree, that will work as well. I meant I wanted to make sure I don't force people who don't want to use multi-region to have to add a region to their Thrift API calls, once both the server and client are upgraded.

        Show
        Kevin Wilfong added a comment - @Ashutosh I agree, that will work as well. I meant I wanted to make sure I don't force people who don't want to use multi-region to have to add a region to their Thrift API calls, once both the server and client are upgraded.
        Hide
        Ashutosh Chauhan added a comment -

        In case client is upgraded but server is not, my impression is that extra param passed on by client is automatically dropped by rpc before making a call on server side. So, that will still work.

        Show
        Ashutosh Chauhan added a comment - In case client is upgraded but server is not, my impression is that extra param passed on by client is automatically dropped by rpc before making a call on server side. So, that will still work.
        Hide
        Kevin Wilfong added a comment -

        I was under the impression that that's true in the sense that, if you are using an old client which thinks a method takes 1 parameter and the server is running new code which thinks that method takes 2 parameters it will still work. However, once you upgrade your client to use new code, you will always have to provide 2 parameters, at least if the client is in Java, I'm not sure if this applies to all languages Thrift supports.

        I wanted to makes sure that this would not break code that uses the current Thrift APIs, even after the clients are upgraded.

        Show
        Kevin Wilfong added a comment - I was under the impression that that's true in the sense that, if you are using an old client which thinks a method takes 1 parameter and the server is running new code which thinks that method takes 2 parameters it will still work. However, once you upgrade your client to use new code, you will always have to provide 2 parameters, at least if the client is in Java, I'm not sure if this applies to all languages Thrift supports. I wanted to makes sure that this would not break code that uses the current Thrift APIs, even after the clients are upgraded.
        Hide
        Ashutosh Chauhan added a comment -

        My understanding of thrift is limited. But, AFAIK you can add new params into existing thrift api and they will still be backward compatible. Is that not the case? Or, is there any other reason that you want to add a whole new set of parallel apis?

        Show
        Ashutosh Chauhan added a comment - My understanding of thrift is limited. But, AFAIK you can add new params into existing thrift api and they will still be backward compatible. Is that not the case? Or, is there any other reason that you want to add a whole new set of parallel apis?
        Hide
        Kevin Wilfong added a comment -

        Thanks, Carl, that sounds like a good idea.

        Unfortunately, Thrift doesn't support method overloading so I'll still have to create new methods with new names to provide backwards compatibility. This should make it easier to make changes like this in the future though.

        Show
        Kevin Wilfong added a comment - Thanks, Carl, that sounds like a good idea. Unfortunately, Thrift doesn't support method overloading so I'll still have to create new methods with new names to provide backwards compatibility. This should make it easier to make changes like this in the future though.
        Hide
        Carl Steinbach added a comment -

        I recommend adding methods that take a single "request" object as input, e.g:

        void create_table(1: CreateTableReq req) throws (1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3, 4:NoSuchObjectException o4)

        My understanding of Thrift is that new properties can be added to the CreateTableReq object without breaking backwards compatibility.

        Show
        Carl Steinbach added a comment - I recommend adding methods that take a single "request" object as input, e.g: void create_table(1: CreateTableReq req) throws (1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3, 4:NoSuchObjectException o4) My understanding of Thrift is that new properties can be added to the CreateTableReq object without breaking backwards compatibility.
        Hide
        Kevin Wilfong added a comment -

        I will leave the current API's unchanged, I intend to add additional APIs, like create_table_using_region, which will allow the user to provide a region name in addition to all normal arguments.

        Show
        Kevin Wilfong added a comment - I will leave the current API's unchanged, I intend to add additional APIs, like create_table_using_region, which will allow the user to provide a region name in addition to all normal arguments.
        Hide
        Ashutosh Chauhan added a comment -

        I assume all of these apis will be backward compatible because thrift rpc will take care of it. Namit/Kevin can you please confirm?

        Show
        Ashutosh Chauhan added a comment - I assume all of these apis will be backward compatible because thrift rpc will take care of it. Namit/Kevin can you please confirm?
        Hide
        Kevin Wilfong added a comment -

        Adding the following to the list to be changed.

        void create_table(1:Table tbl) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3, 4:NoSuchObjectException o4)
        void drop_table(1:string dbname, 2:string name, 3:bool deleteData)
        throws(1:NoSuchObjectException o1, 2:MetaException o3)
        Index add_index(1:Index new_index, 2: Table index_table)
        throws(1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)

        Show
        Kevin Wilfong added a comment - Adding the following to the list to be changed. void create_table(1:Table tbl) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3, 4:NoSuchObjectException o4) void drop_table(1:string dbname, 2:string name, 3:bool deleteData) throws(1:NoSuchObjectException o1, 2:MetaException o3) Index add_index(1:Index new_index, 2: Table index_table) throws(1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)

          People

          • Assignee:
            Kevin Wilfong
            Reporter:
            Namit Jain
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development