Hive
  1. Hive
  2. HIVE-2767

Optionally use framed transport with metastore

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.10.0
    • Component/s: Metastore
    • Labels:
      None

      Description

      Users may want/need to use thrift's framed transport when communicating with the Hive MetaStore. This patch adds a new property hive.metastore.thrift.framed.transport.enabled that enables the framed transport (defaults to off, aka no change from before the patch). This property must be set for both clients and the HMS server.

      It wasn't immediately clear how to use the framed transport with SASL, so as written an exception is thrown if you try starting the server with both options. If SASL and the framed transport will indeed work together I can update the patch (although I don't have a secured environment to test in).

      1. ASF.LICENSE.NOT.GRANTED--HIVE-2767.D2661.3.patch
        7 kB
        Phabricator
      2. ASF.LICENSE.NOT.GRANTED--HIVE-2767.D2661.2.patch
        7 kB
        Phabricator
      3. ASF.LICENSE.NOT.GRANTED--HIVE-2767.D2661.1.patch
        6 kB
        Phabricator
      4. HIVE-2767_a.patch.txt
        6 kB
        Julien Le Dem
      5. HIVE-2767.patch.txt
        4 kB
        Travis Crawford

        Activity

        Hide
        Ashutosh Chauhan added a comment -

        Patch looks ok to me. I presume this is for performance. Just curious, have you seen substantial performance gains because of this. Since, most of the rpc call in MetaStore are of fairly small payload. To me, it appears it won't bring in much perf gains.

        Show
        Ashutosh Chauhan added a comment - Patch looks ok to me. I presume this is for performance. Just curious, have you seen substantial performance gains because of this. Since, most of the rpc call in MetaStore are of fairly small payload. To me, it appears it won't bring in much perf gains.
        Hide
        Travis Crawford added a comment -

        Thanks for the feedback Ashutosh - if this is something you'd consider adding I can update based on current trunk if things have changed.

        While there may be perf gains, this is needed to integrate with our compute grid. Basically a wrapper registers the metastore host:port in zookeeper, and a thrift framed-transport-only proxy proxies metastore requests based on its ZK registration. This lets us launch the metastore on our mesos cluster and still give clients the hard-coded host:port they expect.

        Show
        Travis Crawford added a comment - Thanks for the feedback Ashutosh - if this is something you'd consider adding I can update based on current trunk if things have changed. While there may be perf gains, this is needed to integrate with our compute grid. Basically a wrapper registers the metastore host:port in zookeeper, and a thrift framed-transport-only proxy proxies metastore requests based on its ZK registration. This lets us launch the metastore on our mesos cluster and still give clients the hard-coded host:port they expect.
        Hide
        Ashutosh Chauhan added a comment -

        Thanks Travis for explaining the usecase. Makes sense. Without that I was shooting in dark about why you need Framed Transport. : ) Yeah, update the patch. I will test and commit.

        Show
        Ashutosh Chauhan added a comment - Thanks Travis for explaining the usecase. Makes sense. Without that I was shooting in dark about why you need Framed Transport. : ) Yeah, update the patch. I will test and commit.
        Hide
        Julien Le Dem added a comment -

        I have an updated patch, as Hive changed since this patch. Will post it soon.

        Show
        Julien Le Dem added a comment - I have an updated patch, as Hive changed since this patch. Will post it soon.
        Hide
        Julien Le Dem added a comment -

        patch updated to latest Hive

        Show
        Julien Le Dem added a comment - patch updated to latest Hive
        Hide
        Ashutosh Chauhan added a comment -

        Not to the latest trunk : ) Patch fails to apply. Also, can you use arc to submit the patch. Instructions are here: https://cwiki.apache.org/confluence/display/Hive/PhabricatorCodeReview

        Show
        Ashutosh Chauhan added a comment - Not to the latest trunk : ) Patch fails to apply. Also, can you use arc to submit the patch. Instructions are here: https://cwiki.apache.org/confluence/display/Hive/PhabricatorCodeReview
        Hide
        Ashutosh Chauhan added a comment -

        Patch needs to be rebased.

        Show
        Ashutosh Chauhan added a comment - Patch needs to be rebased.
        Hide
        Phabricator added a comment -

        travis requested code review of "HIVE-2767 [jira] Optionally use framed transport with metastore".
        Reviewers: JIRA

        Add support for optionally using the thrift framed transport, enabling integration with environments where that is necessary.

        Users may want/need to use thrift's framed transport when communicating with the Hive MetaStore. This patch adds a new property hive.metastore.thrift.framed.transport.enabled that enables the framed transport (defaults to off, aka no change from before the patch). This property must be set for both clients and the HMS server.

        It wasn't immediately clear how to use the framed transport with SASL, so as written an exception is thrown if you try starting the server with both options. If SASL and the framed transport will indeed work together I can update the patch (although I don't have a secured environment to test in).

        TEST PLAN
        Tested locally that client and server can connect, both with and without the flag. Tests pass.

        REVISION DETAIL
        https://reviews.facebook.net/D2661

        AFFECTED FILES
        common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
        metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
        metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java

        MANAGE HERALD DIFFERENTIAL RULES
        https://reviews.facebook.net/herald/view/differential/

        WHY DID I GET THIS EMAIL?
        https://reviews.facebook.net/herald/transcript/6111/

        Tip: use the X-Herald-Rules header to filter Herald messages in your client.

        Show
        Phabricator added a comment - travis requested code review of " HIVE-2767 [jira] Optionally use framed transport with metastore". Reviewers: JIRA Add support for optionally using the thrift framed transport, enabling integration with environments where that is necessary. Users may want/need to use thrift's framed transport when communicating with the Hive MetaStore. This patch adds a new property hive.metastore.thrift.framed.transport.enabled that enables the framed transport (defaults to off, aka no change from before the patch). This property must be set for both clients and the HMS server. It wasn't immediately clear how to use the framed transport with SASL, so as written an exception is thrown if you try starting the server with both options. If SASL and the framed transport will indeed work together I can update the patch (although I don't have a secured environment to test in). TEST PLAN Tested locally that client and server can connect, both with and without the flag. Tests pass. REVISION DETAIL https://reviews.facebook.net/D2661 AFFECTED FILES common/src/java/org/apache/hadoop/hive/conf/HiveConf.java metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java MANAGE HERALD DIFFERENTIAL RULES https://reviews.facebook.net/herald/view/differential/ WHY DID I GET THIS EMAIL? https://reviews.facebook.net/herald/transcript/6111/ Tip: use the X-Herald-Rules header to filter Herald messages in your client.
        Hide
        Phabricator added a comment -

        ashutoshc has requested changes to the revision "HIVE-2767 [jira] Optionally use framed transport with metastore".

        What will happen if the config variable is set to true in client, but is false in server and vice-versa?

        INLINE COMMENTS
        common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:269 This also needs to be added in conf/hive-default.xml.template which serve as a documentation.
        metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java:3020 In your earlier version of patch you were throwing exception in case both secure mode as well as framed transport is set to true. If you have tested with both true and it works then its fine, otherwise we should throw an exception here.

        REVISION DETAIL
        https://reviews.facebook.net/D2661

        BRANCH
        HIVE-2767_optional_framed_transport

        Show
        Phabricator added a comment - ashutoshc has requested changes to the revision " HIVE-2767 [jira] Optionally use framed transport with metastore". What will happen if the config variable is set to true in client, but is false in server and vice-versa? INLINE COMMENTS common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:269 This also needs to be added in conf/hive-default.xml.template which serve as a documentation. metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java:3020 In your earlier version of patch you were throwing exception in case both secure mode as well as framed transport is set to true. If you have tested with both true and it works then its fine, otherwise we should throw an exception here. REVISION DETAIL https://reviews.facebook.net/D2661 BRANCH HIVE-2767 _optional_framed_transport
        Hide
        Phabricator added a comment -

        travis has commented on the revision "HIVE-2767 [jira] Optionally use framed transport with metastore".

        If just one side of the connection is using the framed transport the connection will fail, likely after the timeout.

        As this is something that is setup just once per site its not likely to change often, and it defaults off, so the risk of casual misconfiguration is low.

        Any suggestions on how to better clue the user in that the transport may be the issue?

        INLINE COMMENTS
        common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:269 Good suggestion, done.
        metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java:3020 Good suggestion. I don't have an environment to test SASL in, so I'll add the check back.

        REVISION DETAIL
        https://reviews.facebook.net/D2661

        BRANCH
        HIVE-2767_optional_framed_transport

        Show
        Phabricator added a comment - travis has commented on the revision " HIVE-2767 [jira] Optionally use framed transport with metastore". If just one side of the connection is using the framed transport the connection will fail, likely after the timeout. As this is something that is setup just once per site its not likely to change often, and it defaults off, so the risk of casual misconfiguration is low. Any suggestions on how to better clue the user in that the transport may be the issue? INLINE COMMENTS common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:269 Good suggestion, done. metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java:3020 Good suggestion. I don't have an environment to test SASL in, so I'll add the check back. REVISION DETAIL https://reviews.facebook.net/D2661 BRANCH HIVE-2767 _optional_framed_transport
        Hide
        Phabricator added a comment -

        travis updated the revision "HIVE-2767 [jira] Optionally use framed transport with metastore".
        Reviewers: JIRA, ashutoshc

        -

        REVISION DETAIL
        https://reviews.facebook.net/D2661

        AFFECTED FILES
        common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
        conf/hive-default.xml.template
        metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
        metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java

        Show
        Phabricator added a comment - travis updated the revision " HIVE-2767 [jira] Optionally use framed transport with metastore". Reviewers: JIRA, ashutoshc - REVISION DETAIL https://reviews.facebook.net/D2661 AFFECTED FILES common/src/java/org/apache/hadoop/hive/conf/HiveConf.java conf/hive-default.xml.template metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
        Hide
        Phabricator added a comment -

        travis updated the revision "HIVE-2767 [jira] Optionally use framed transport with metastore".
        Reviewers: JIRA, ashutoshc

        Update per comments from review. Add option to hive-default.xml, and disallow framed transport when using SASL as that has not been tested.

        REVISION DETAIL
        https://reviews.facebook.net/D2661

        AFFECTED FILES
        common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
        conf/hive-default.xml.template
        metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
        metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java

        Show
        Phabricator added a comment - travis updated the revision " HIVE-2767 [jira] Optionally use framed transport with metastore". Reviewers: JIRA, ashutoshc Update per comments from review. Add option to hive-default.xml, and disallow framed transport when using SASL as that has not been tested. REVISION DETAIL https://reviews.facebook.net/D2661 AFFECTED FILES common/src/java/org/apache/hadoop/hive/conf/HiveConf.java conf/hive-default.xml.template metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
        Hide
        Ashutosh Chauhan added a comment -

        After your patch test ant test -Dtestcase=TestHiveServerSessions is timing out. Can you take a look?

        Show
        Ashutosh Chauhan added a comment - After your patch test ant test -Dtestcase=TestHiveServerSessions is timing out. Can you take a look?
        Hide
        Travis Crawford added a comment -

        @ashutosh - Yeah this test does seem problematic. It works fine in IntelliJ but I can't get it to pass with the command you gave, even when doing a clean trunk build. Looking at Jenkins the test works fine, so perhaps its something to do with my machine (osx 10.7.3).

        I'm looking into why the test doesn't work on trunk, then will see if this change affects it.

        Show
        Travis Crawford added a comment - @ashutosh - Yeah this test does seem problematic. It works fine in IntelliJ but I can't get it to pass with the command you gave, even when doing a clean trunk build. Looking at Jenkins the test works fine, so perhaps its something to do with my machine (osx 10.7.3). I'm looking into why the test doesn't work on trunk, then will see if this change affects it.
        Hide
        Ashutosh Chauhan added a comment -

        @Travis,
        It has nothing to do with your patch. Problem exists on trunk. See, HIVE-2937 And ya, its a race condition, so it doesn't show up always.

        Show
        Ashutosh Chauhan added a comment - @Travis, It has nothing to do with your patch. Problem exists on trunk. See, HIVE-2937 And ya, its a race condition, so it doesn't show up always.
        Hide
        Travis Crawford added a comment -

        Cool - thanks for the pointer! I'll watch that issue and afterwards rebase if necessary and update.

        Show
        Travis Crawford added a comment - Cool - thanks for the pointer! I'll watch that issue and afterwards rebase if necessary and update.
        Hide
        Ashutosh Chauhan added a comment -

        Committed. Thanks, Travis!

        Show
        Ashutosh Chauhan added a comment - Committed. Thanks, Travis!
        Hide
        Phabricator added a comment -

        ashutoshc has accepted the revision "HIVE-2767 [jira] Optionally use framed transport with metastore".

        +1 will commit soon.

        REVISION DETAIL
        https://reviews.facebook.net/D2661

        BRANCH
        HIVE-2767_optional_framed_transport

        Show
        Phabricator added a comment - ashutoshc has accepted the revision " HIVE-2767 [jira] Optionally use framed transport with metastore". +1 will commit soon. REVISION DETAIL https://reviews.facebook.net/D2661 BRANCH HIVE-2767 _optional_framed_transport
        Hide
        Phabricator added a comment -

        travis has committed the revision "HIVE-2767 [jira] Optionally use framed transport with metastore".

        Change committed by hashutosh.

        REVISION DETAIL
        https://reviews.facebook.net/D2661

        COMMIT
        https://reviews.facebook.net/rHIVE1325446

        Show
        Phabricator added a comment - travis has committed the revision " HIVE-2767 [jira] Optionally use framed transport with metastore". Change committed by hashutosh. REVISION DETAIL https://reviews.facebook.net/D2661 COMMIT https://reviews.facebook.net/rHIVE1325446
        Hide
        Hudson added a comment -

        Integrated in Hive-trunk-h0.21 #1370 (See https://builds.apache.org/job/Hive-trunk-h0.21/1370/)
        HIVE-2767 [jira] Optionally use framed transport with metastore
        (Travis Crawford via Ashutosh Chauhan)

        Summary:
        Add support for optionally using the thrift framed transport, enabling
        integration with environments where that is necessary.

        Users may want/need to use thrift's framed transport when communicating with the
        Hive MetaStore. This patch adds a new property
        hive.metastore.thrift.framed.transport.enabled that enables the framed transport
        (defaults to off, aka no change from before the patch). This property must be
        set for both clients and the HMS server.

        It wasn't immediately clear how to use the framed transport with SASL, so as
        written an exception is thrown if you try starting the server with both options.
        If SASL and the framed transport will indeed work together I can update the
        patch (although I don't have a secured environment to test in).

        Test Plan:
        Tested locally that client and server can connect, both with and
        without the flag. Tests pass.

        Reviewers: JIRA, ashutoshc

        Reviewed By: ashutoshc

        Differential Revision: https://reviews.facebook.net/D2661 (Revision 1325446)

        Result = SUCCESS
        hashutosh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1325446
        Files :

        • /hive/trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
        • /hive/trunk/conf/hive-default.xml.template
        • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
        • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
        Show
        Hudson added a comment - Integrated in Hive-trunk-h0.21 #1370 (See https://builds.apache.org/job/Hive-trunk-h0.21/1370/ ) HIVE-2767 [jira] Optionally use framed transport with metastore (Travis Crawford via Ashutosh Chauhan) Summary: Add support for optionally using the thrift framed transport, enabling integration with environments where that is necessary. Users may want/need to use thrift's framed transport when communicating with the Hive MetaStore. This patch adds a new property hive.metastore.thrift.framed.transport.enabled that enables the framed transport (defaults to off, aka no change from before the patch). This property must be set for both clients and the HMS server. It wasn't immediately clear how to use the framed transport with SASL, so as written an exception is thrown if you try starting the server with both options. If SASL and the framed transport will indeed work together I can update the patch (although I don't have a secured environment to test in). Test Plan: Tested locally that client and server can connect, both with and without the flag. Tests pass. Reviewers: JIRA, ashutoshc Reviewed By: ashutoshc Differential Revision: https://reviews.facebook.net/D2661 (Revision 1325446) Result = SUCCESS hashutosh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1325446 Files : /hive/trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java /hive/trunk/conf/hive-default.xml.template /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
        Hide
        Hudson added a comment -

        Integrated in Hive-trunk-hadoop2 #54 (See https://builds.apache.org/job/Hive-trunk-hadoop2/54/)
        HIVE-2767 [jira] Optionally use framed transport with metastore
        (Travis Crawford via Ashutosh Chauhan)

        Summary:
        Add support for optionally using the thrift framed transport, enabling
        integration with environments where that is necessary.

        Users may want/need to use thrift's framed transport when communicating with the
        Hive MetaStore. This patch adds a new property
        hive.metastore.thrift.framed.transport.enabled that enables the framed transport
        (defaults to off, aka no change from before the patch). This property must be
        set for both clients and the HMS server.

        It wasn't immediately clear how to use the framed transport with SASL, so as
        written an exception is thrown if you try starting the server with both options.
        If SASL and the framed transport will indeed work together I can update the
        patch (although I don't have a secured environment to test in).

        Test Plan:
        Tested locally that client and server can connect, both with and
        without the flag. Tests pass.

        Reviewers: JIRA, ashutoshc

        Reviewed By: ashutoshc

        Differential Revision: https://reviews.facebook.net/D2661 (Revision 1325446)

        Result = ABORTED
        hashutosh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1325446
        Files :

        • /hive/trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
        • /hive/trunk/conf/hive-default.xml.template
        • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
        • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
        Show
        Hudson added a comment - Integrated in Hive-trunk-hadoop2 #54 (See https://builds.apache.org/job/Hive-trunk-hadoop2/54/ ) HIVE-2767 [jira] Optionally use framed transport with metastore (Travis Crawford via Ashutosh Chauhan) Summary: Add support for optionally using the thrift framed transport, enabling integration with environments where that is necessary. Users may want/need to use thrift's framed transport when communicating with the Hive MetaStore. This patch adds a new property hive.metastore.thrift.framed.transport.enabled that enables the framed transport (defaults to off, aka no change from before the patch). This property must be set for both clients and the HMS server. It wasn't immediately clear how to use the framed transport with SASL, so as written an exception is thrown if you try starting the server with both options. If SASL and the framed transport will indeed work together I can update the patch (although I don't have a secured environment to test in). Test Plan: Tested locally that client and server can connect, both with and without the flag. Tests pass. Reviewers: JIRA, ashutoshc Reviewed By: ashutoshc Differential Revision: https://reviews.facebook.net/D2661 (Revision 1325446) Result = ABORTED hashutosh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1325446 Files : /hive/trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java /hive/trunk/conf/hive-default.xml.template /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
        Hide
        Ashutosh Chauhan added a comment -

        This issue is fixed and released as part of 0.10.0 release. If you find an issue which seems to be related to this one, please create a new jira and link this one with new jira.

        Show
        Ashutosh Chauhan added a comment - This issue is fixed and released as part of 0.10.0 release. If you find an issue which seems to be related to this one, please create a new jira and link this one with new jira.

          People

          • Assignee:
            Travis Crawford
            Reporter:
            Travis Crawford
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development