Hive
  1. Hive
  2. HIVE-2467

HA Support for Metastore Server

    Details

      Description

      We require HA deployment for metastore server for HCatalog:

      • Multiple server instances run behind VIP
      • Database provides HA

      Metastore server instances will need to be able to share any state required for VIP outside RDBMS. As of Hive 0.8 affected conversational state that needs to support VIP/HA setup is limited to current delegation tokens. Is this correct?

      We are planning to use ZooKeeper to share current delegation tokens and master keys between nodes of the VIP. ZK is already (optionally) used by Hive for concurrency control. Access to ZK would be limited on the network level or in the future, when ZooKeeper supports security, through Kerberos, similar to NN access.

      Currently Hive taps into Hadoop core security delegation token support through extension of
      org.apache.hadoop.security.token.delegation.AbstractDelegationTokenSecretManager<TokenIdent>

      A solution could amend the Hive specific extension to support:

      • Pluggable delegation token and master key store (ZooKeeper as alternative for in-memory AbstractDelegationTokenSecretManager)
      • Delegation token retrieval from token store when not found in memory (wrap/extend retrievePassword(...))
      • Cancellation of token in token store
      • Purging of expired tokens from token store

      http://www.mail-archive.com/hcatalog-user@incubator.apache.org/msg00053.html

      1. HIVE-2467.patch
        49 kB
        Thomas Weise
      2. HIVE-2467.2.patch
        48 kB
        Thomas Weise

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in Hive-trunk-h0.21 #1129 (See https://builds.apache.org/job/Hive-trunk-h0.21/1129/)
          HIVE-2467 : HA Support for Metastore Server (Thomas Weise via Ashutosh Chauhan)

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

          • /hive/trunk/shims/ivy.xml
          • /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java
          • /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/MemoryTokenStore.java
          • /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java
          • /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/ZooKeeperTokenStore.java
          • /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/security
          • /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/security/token
          • /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/security/token/delegation
          • /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/security/token/delegation/HiveDelegationTokenSupport.java
          • /hive/trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java
          Show
          Hudson added a comment - Integrated in Hive-trunk-h0.21 #1129 (See https://builds.apache.org/job/Hive-trunk-h0.21/1129/ ) HIVE-2467 : HA Support for Metastore Server (Thomas Weise via Ashutosh Chauhan) hashutosh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1211275 Files : /hive/trunk/shims/ivy.xml /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/MemoryTokenStore.java /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/ZooKeeperTokenStore.java /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/security /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/security/token /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/security/token/delegation /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/security/token/delegation/HiveDelegationTokenSupport.java /hive/trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java
          Hide
          Hudson added a comment -

          Integrated in Hive-trunk-h0.23.0 #5 (See https://builds.apache.org/job/Hive-trunk-h0.23.0/5/)
          HIVE-2467 : HA Support for Metastore Server (Thomas Weise via Ashutosh Chauhan)

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

          • /hive/trunk/shims/ivy.xml
          • /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java
          • /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/MemoryTokenStore.java
          • /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java
          • /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/ZooKeeperTokenStore.java
          • /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/security
          • /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/security/token
          • /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/security/token/delegation
          • /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/security/token/delegation/HiveDelegationTokenSupport.java
          • /hive/trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java
          Show
          Hudson added a comment - Integrated in Hive-trunk-h0.23.0 #5 (See https://builds.apache.org/job/Hive-trunk-h0.23.0/5/ ) HIVE-2467 : HA Support for Metastore Server (Thomas Weise via Ashutosh Chauhan) hashutosh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1211275 Files : /hive/trunk/shims/ivy.xml /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/MemoryTokenStore.java /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/ZooKeeperTokenStore.java /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/security /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/security/token /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/security/token/delegation /hive/trunk/shims/src/0.20S/java/org/apache/hadoop/security/token/delegation/HiveDelegationTokenSupport.java /hive/trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java
          Hide
          Ashutosh Chauhan added a comment -

          +1 Committed to trunk. Thanks, Thomas for this useful contribution.

          Show
          Ashutosh Chauhan added a comment - +1 Committed to trunk. Thanks, Thomas for this useful contribution.
          Hide
          Thomas Weise added a comment -

          All unit tests pass with the patch.

          Show
          Thomas Weise added a comment - All unit tests pass with the patch.
          Hide
          jiraposter@reviews.apache.org added a comment -

          On 2011-11-22 02:27:01, Ashutosh Chauhan wrote:

          > trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/MemoryTokenStore.java, lines 40-46

          > <https://reviews.apache.org/r/2721/diff/2/?file=58992#file58992line40>

          >

          > If TokenStore is meant to be Configurable, then have a private variable here to hold the conf in setConf() and return that in getConf()

          Added. It was never meant to implement Configurable, but ReflectionUtils.newInstance(...) does not support constructor injection for conf.

          On 2011-11-22 02:27:01, Ashutosh Chauhan wrote:

          > trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/MemoryTokenStore.java, line 33

          > <https://reviews.apache.org/r/2721/diff/2/?file=58992#file58992line33>

          >

          > Instead of java.util.concurrent.ConcurrentHashMap, use ConcurrentHashMap instead.

          done

          On 2011-11-22 02:27:01, Ashutosh Chauhan wrote:

          > trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java, lines 335-343

          > <https://reviews.apache.org/r/2721/diff/2/?file=58991#file58991line335>

          >

          > For consistency reasons, these should be moved to HiveConf. Also, the other properties which are there in this class.

          Ideally yes (tried that originally), but the shim cannot depend on HiveConf... Perhaps in the future we can define these as part of Hadoop common security.

          On 2011-11-22 02:27:01, Ashutosh Chauhan wrote:

          > trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java, line 123

          > <https://reviews.apache.org/r/2721/diff/2/?file=58993#file58993line123>

          >

          > Either it should return boolean or it should throw Exception. Doing both is confusing. If you want to throw exception, then throw exception on failure and return void, else return false in failure scenario. Token must be removed in all cases if method returns.

          Exception is for errors, return boolean to indicate status of successful operation. Clarified in java doc.

          On 2011-11-22 02:27:01, Ashutosh Chauhan wrote:

          > trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java, line 100

          > <https://reviews.apache.org/r/2721/diff/2/?file=58993#file58993line100>

          >

          > Interface method needs javadoc.

          done

          On 2011-11-22 02:27:01, Ashutosh Chauhan wrote:

          > trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java, line 184

          > <https://reviews.apache.org/r/2721/diff/2/?file=58993#file58993line184>

          >

          > Fully qualified classname.

          done

          On 2011-11-22 02:27:01, Ashutosh Chauhan wrote:

          > trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/ZooKeeperTokenStore.java, lines 49-50

          > <https://reviews.apache.org/r/2721/diff/2/?file=58994#file58994line49>

          >

          > Should these come from hiveConf, so they are externally configurable?

          these are relative paths, the root is already configurable, added comment in code

          On 2011-11-22 02:27:01, Ashutosh Chauhan wrote:

          > trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java, line 1

          > <https://reviews.apache.org/r/2721/diff/2/?file=58996#file58996line1>

          >

          > One way to test HA is to start two metastore processes in two different threads configure to use ZK token store and then do operation on first kill it and then do operation on second one and then have it succeeds. If thats not straight forward, may be we can take that up in separate ticket.

          That's what I'm doing as integration test (this type of thing is really e2e (launching multiple servers), not unit testing). We should have the pieces reasonably covered in unit tests though. The added test covers DelegationTokenManager with token store interaction, the existing test case does the full path through the auth bridge.

          On 2011-11-22 02:27:01, Ashutosh Chauhan wrote:

          > trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/ZooKeeperTokenStore.java, lines 137-152

          > <https://reviews.apache.org/r/2721/diff/2/?file=58994#file58994line137>

          >

          > Contract of configurable is to return conf in getConf(), so you must store it in a private variable and return it.

          ditto MemoryTokenStore

          On 2011-11-22 02:27:01, Ashutosh Chauhan wrote:

          > trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java, line 86

          > <https://reviews.apache.org/r/2721/diff/2/?file=58996#file58996line86>

          >

          > Get rid of commented code.

          made this controllable via system property

          • Thomas

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/2721/#review3417
          -----------------------------------------------------------

          On 2011-11-23 19:55:34, Thomas wrote:

          -----------------------------------------------------------

          This is an automatically generated e-mail. To reply, visit:

          https://reviews.apache.org/r/2721/

          -----------------------------------------------------------

          (Updated 2011-11-23 19:55:34)

          Review request for Carl Steinbach, Ashutosh Chauhan and Devaraj Das.

          Summary

          -------

          https://issues.apache.org/jira/browse/HIVE-2467

          This addresses bug HIVE-2467.

          https://issues.apache.org/jira/browse/HIVE-2467

          Diffs

          -----

          trunk/shims/ivy.xml 1205535

          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/DelegationTokenStore.java PRE-CREATION

          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java 1205535

          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/MemoryTokenStore.java PRE-CREATION

          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java PRE-CREATION

          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/ZooKeeperTokenStore.java PRE-CREATION

          trunk/shims/src/0.20S/java/org/apache/hadoop/security/token/delegation/HiveDelegationTokenSupport.java PRE-CREATION

          trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java 1205535

          Diff: https://reviews.apache.org/r/2721/diff

          Testing

          -------

          unit test added, ant clean package test - passed

          Thanks,

          Thomas

          Show
          jiraposter@reviews.apache.org added a comment - On 2011-11-22 02:27:01, Ashutosh Chauhan wrote: > trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/MemoryTokenStore.java, lines 40-46 > < https://reviews.apache.org/r/2721/diff/2/?file=58992#file58992line40 > > > If TokenStore is meant to be Configurable, then have a private variable here to hold the conf in setConf() and return that in getConf() Added. It was never meant to implement Configurable, but ReflectionUtils.newInstance(...) does not support constructor injection for conf. On 2011-11-22 02:27:01, Ashutosh Chauhan wrote: > trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/MemoryTokenStore.java, line 33 > < https://reviews.apache.org/r/2721/diff/2/?file=58992#file58992line33 > > > Instead of java.util.concurrent.ConcurrentHashMap, use ConcurrentHashMap instead. done On 2011-11-22 02:27:01, Ashutosh Chauhan wrote: > trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java, lines 335-343 > < https://reviews.apache.org/r/2721/diff/2/?file=58991#file58991line335 > > > For consistency reasons, these should be moved to HiveConf. Also, the other properties which are there in this class. Ideally yes (tried that originally), but the shim cannot depend on HiveConf... Perhaps in the future we can define these as part of Hadoop common security. On 2011-11-22 02:27:01, Ashutosh Chauhan wrote: > trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java, line 123 > < https://reviews.apache.org/r/2721/diff/2/?file=58993#file58993line123 > > > Either it should return boolean or it should throw Exception. Doing both is confusing. If you want to throw exception, then throw exception on failure and return void, else return false in failure scenario. Token must be removed in all cases if method returns. Exception is for errors, return boolean to indicate status of successful operation. Clarified in java doc. On 2011-11-22 02:27:01, Ashutosh Chauhan wrote: > trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java, line 100 > < https://reviews.apache.org/r/2721/diff/2/?file=58993#file58993line100 > > > Interface method needs javadoc. done On 2011-11-22 02:27:01, Ashutosh Chauhan wrote: > trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java, line 184 > < https://reviews.apache.org/r/2721/diff/2/?file=58993#file58993line184 > > > Fully qualified classname. done On 2011-11-22 02:27:01, Ashutosh Chauhan wrote: > trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/ZooKeeperTokenStore.java, lines 49-50 > < https://reviews.apache.org/r/2721/diff/2/?file=58994#file58994line49 > > > Should these come from hiveConf, so they are externally configurable? these are relative paths, the root is already configurable, added comment in code On 2011-11-22 02:27:01, Ashutosh Chauhan wrote: > trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java, line 1 > < https://reviews.apache.org/r/2721/diff/2/?file=58996#file58996line1 > > > One way to test HA is to start two metastore processes in two different threads configure to use ZK token store and then do operation on first kill it and then do operation on second one and then have it succeeds. If thats not straight forward, may be we can take that up in separate ticket. That's what I'm doing as integration test (this type of thing is really e2e (launching multiple servers), not unit testing). We should have the pieces reasonably covered in unit tests though. The added test covers DelegationTokenManager with token store interaction, the existing test case does the full path through the auth bridge. On 2011-11-22 02:27:01, Ashutosh Chauhan wrote: > trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/ZooKeeperTokenStore.java, lines 137-152 > < https://reviews.apache.org/r/2721/diff/2/?file=58994#file58994line137 > > > Contract of configurable is to return conf in getConf(), so you must store it in a private variable and return it. ditto MemoryTokenStore On 2011-11-22 02:27:01, Ashutosh Chauhan wrote: > trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java, line 86 > < https://reviews.apache.org/r/2721/diff/2/?file=58996#file58996line86 > > > Get rid of commented code. made this controllable via system property Thomas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2721/#review3417 ----------------------------------------------------------- On 2011-11-23 19:55:34, Thomas wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2721/ ----------------------------------------------------------- (Updated 2011-11-23 19:55:34) Review request for Carl Steinbach, Ashutosh Chauhan and Devaraj Das. Summary ------- https://issues.apache.org/jira/browse/HIVE-2467 This addresses bug HIVE-2467 . https://issues.apache.org/jira/browse/HIVE-2467 Diffs ----- trunk/shims/ivy.xml 1205535 trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/DelegationTokenStore.java PRE-CREATION trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java 1205535 trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/MemoryTokenStore.java PRE-CREATION trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java PRE-CREATION trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/ZooKeeperTokenStore.java PRE-CREATION trunk/shims/src/0.20S/java/org/apache/hadoop/security/token/delegation/HiveDelegationTokenSupport.java PRE-CREATION trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java 1205535 Diff: https://reviews.apache.org/r/2721/diff Testing ------- unit test added, ant clean package test - passed Thanks, Thomas
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/2721/
          -----------------------------------------------------------

          (Updated 2011-11-23 19:55:34.604089)

          Review request for Carl Steinbach, Ashutosh Chauhan and Devaraj Das.

          Changes
          -------

          Updated patch to address review comments from Ashutosh.

          Summary
          -------

          https://issues.apache.org/jira/browse/HIVE-2467

          This addresses bug HIVE-2467.
          https://issues.apache.org/jira/browse/HIVE-2467

          Diffs (updated)


          trunk/shims/ivy.xml 1205535
          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/DelegationTokenStore.java PRE-CREATION
          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java 1205535
          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/MemoryTokenStore.java PRE-CREATION
          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java PRE-CREATION
          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/ZooKeeperTokenStore.java PRE-CREATION
          trunk/shims/src/0.20S/java/org/apache/hadoop/security/token/delegation/HiveDelegationTokenSupport.java PRE-CREATION
          trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java 1205535

          Diff: https://reviews.apache.org/r/2721/diff

          Testing
          -------

          unit test added, ant clean package test - passed

          Thanks,

          Thomas

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2721/ ----------------------------------------------------------- (Updated 2011-11-23 19:55:34.604089) Review request for Carl Steinbach, Ashutosh Chauhan and Devaraj Das. Changes ------- Updated patch to address review comments from Ashutosh. Summary ------- https://issues.apache.org/jira/browse/HIVE-2467 This addresses bug HIVE-2467 . https://issues.apache.org/jira/browse/HIVE-2467 Diffs (updated) trunk/shims/ivy.xml 1205535 trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/DelegationTokenStore.java PRE-CREATION trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java 1205535 trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/MemoryTokenStore.java PRE-CREATION trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java PRE-CREATION trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/ZooKeeperTokenStore.java PRE-CREATION trunk/shims/src/0.20S/java/org/apache/hadoop/security/token/delegation/HiveDelegationTokenSupport.java PRE-CREATION trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java 1205535 Diff: https://reviews.apache.org/r/2721/diff Testing ------- unit test added, ant clean package test - passed Thanks, Thomas
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/2721/#review3417
          -----------------------------------------------------------

          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java
          <https://reviews.apache.org/r/2721/#comment7646>

          For consistency reasons, these should be moved to HiveConf. Also, the other properties which are there in this class.

          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/MemoryTokenStore.java
          <https://reviews.apache.org/r/2721/#comment7647>

          Refer to it by Map.

          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/MemoryTokenStore.java
          <https://reviews.apache.org/r/2721/#comment7648>

          Instead of java.util.concurrent.ConcurrentHashMap, use ConcurrentHashMap instead.

          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/MemoryTokenStore.java
          <https://reviews.apache.org/r/2721/#comment7650>

          If TokenStore is meant to be Configurable, then have a private variable here to hold the conf in setConf() and return that in getConf()

          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/MemoryTokenStore.java
          <https://reviews.apache.org/r/2721/#comment7651>

          use ArrayList, instead of fully qualified with package name

          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java
          <https://reviews.apache.org/r/2721/#comment7652>

          Interface method needs javadoc.

          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java
          <https://reviews.apache.org/r/2721/#comment7653>

          Interface method needs javadoc.

          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java
          <https://reviews.apache.org/r/2721/#comment7654>

          Either it should return boolean or it should throw Exception. Doing both is confusing. If you want to throw exception, then throw exception on failure and return void, else return false in failure scenario. Token must be removed in all cases if method returns.

          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java
          <https://reviews.apache.org/r/2721/#comment7655>

          HashMap instead of java.util.HashMap

          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java
          <https://reviews.apache.org/r/2721/#comment7656>

          Fully qualified classname.

          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/ZooKeeperTokenStore.java
          <https://reviews.apache.org/r/2721/#comment7658>

          Should these come from hiveConf, so they are externally configurable?

          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/ZooKeeperTokenStore.java
          <https://reviews.apache.org/r/2721/#comment7659>

          Contract of configurable is to return conf in getConf(), so you must store it in a private variable and return it.

          trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java
          <https://reviews.apache.org/r/2721/#comment7660>

          One way to test HA is to start two metastore processes in two different threads configure to use ZK token store and then do operation on first kill it and then do operation on second one and then have it succeeds. If thats not straight forward, may be we can take that up in separate ticket.

          trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java
          <https://reviews.apache.org/r/2721/#comment7661>

          Get rid of commented code.

          • Ashutosh

          On 2011-11-17 00:57:32, Thomas wrote:

          -----------------------------------------------------------

          This is an automatically generated e-mail. To reply, visit:

          https://reviews.apache.org/r/2721/

          -----------------------------------------------------------

          (Updated 2011-11-17 00:57:32)

          Review request for Carl Steinbach, Ashutosh Chauhan and Devaraj Das.

          Summary

          -------

          https://issues.apache.org/jira/browse/HIVE-2467

          This addresses bug HIVE-2467.

          https://issues.apache.org/jira/browse/HIVE-2467

          Diffs

          -----

          trunk/shims/ivy.xml 1202918

          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java 1202918

          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/MemoryTokenStore.java PRE-CREATION

          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java PRE-CREATION

          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/ZooKeeperTokenStore.java PRE-CREATION

          trunk/shims/src/0.20S/java/org/apache/hadoop/security/token/delegation/HiveDelegationTokenSupport.java PRE-CREATION

          trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java 1202918

          Diff: https://reviews.apache.org/r/2721/diff

          Testing

          -------

          unit test added, ant clean package test - passed

          Thanks,

          Thomas

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2721/#review3417 ----------------------------------------------------------- trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java < https://reviews.apache.org/r/2721/#comment7646 > For consistency reasons, these should be moved to HiveConf. Also, the other properties which are there in this class. trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/MemoryTokenStore.java < https://reviews.apache.org/r/2721/#comment7647 > Refer to it by Map. trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/MemoryTokenStore.java < https://reviews.apache.org/r/2721/#comment7648 > Instead of java.util.concurrent.ConcurrentHashMap, use ConcurrentHashMap instead. trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/MemoryTokenStore.java < https://reviews.apache.org/r/2721/#comment7650 > If TokenStore is meant to be Configurable, then have a private variable here to hold the conf in setConf() and return that in getConf() trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/MemoryTokenStore.java < https://reviews.apache.org/r/2721/#comment7651 > use ArrayList, instead of fully qualified with package name trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java < https://reviews.apache.org/r/2721/#comment7652 > Interface method needs javadoc. trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java < https://reviews.apache.org/r/2721/#comment7653 > Interface method needs javadoc. trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java < https://reviews.apache.org/r/2721/#comment7654 > Either it should return boolean or it should throw Exception. Doing both is confusing. If you want to throw exception, then throw exception on failure and return void, else return false in failure scenario. Token must be removed in all cases if method returns. trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java < https://reviews.apache.org/r/2721/#comment7655 > HashMap instead of java.util.HashMap trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java < https://reviews.apache.org/r/2721/#comment7656 > Fully qualified classname. trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/ZooKeeperTokenStore.java < https://reviews.apache.org/r/2721/#comment7658 > Should these come from hiveConf, so they are externally configurable? trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/ZooKeeperTokenStore.java < https://reviews.apache.org/r/2721/#comment7659 > Contract of configurable is to return conf in getConf(), so you must store it in a private variable and return it. trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java < https://reviews.apache.org/r/2721/#comment7660 > One way to test HA is to start two metastore processes in two different threads configure to use ZK token store and then do operation on first kill it and then do operation on second one and then have it succeeds. If thats not straight forward, may be we can take that up in separate ticket. trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java < https://reviews.apache.org/r/2721/#comment7661 > Get rid of commented code. Ashutosh On 2011-11-17 00:57:32, Thomas wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2721/ ----------------------------------------------------------- (Updated 2011-11-17 00:57:32) Review request for Carl Steinbach, Ashutosh Chauhan and Devaraj Das. Summary ------- https://issues.apache.org/jira/browse/HIVE-2467 This addresses bug HIVE-2467 . https://issues.apache.org/jira/browse/HIVE-2467 Diffs ----- trunk/shims/ivy.xml 1202918 trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java 1202918 trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/MemoryTokenStore.java PRE-CREATION trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java PRE-CREATION trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/ZooKeeperTokenStore.java PRE-CREATION trunk/shims/src/0.20S/java/org/apache/hadoop/security/token/delegation/HiveDelegationTokenSupport.java PRE-CREATION trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java 1202918 Diff: https://reviews.apache.org/r/2721/diff Testing ------- unit test added, ant clean package test - passed Thanks, Thomas
          Hide
          Ashutosh Chauhan added a comment -

          @Thomas,
          Now that there are no users of DelegationTokenSecretManager, I think cleaner way to organize this code is to merge DelegationTokenSecretManager and TokenStoreDelegationSecretManager and just call it DelegationTokenSecretManager. Also, TokenStore instead of being contained within a class, I think should exist independently of it.

          Show
          Ashutosh Chauhan added a comment - @Thomas, Now that there are no users of DelegationTokenSecretManager , I think cleaner way to organize this code is to merge DelegationTokenSecretManager and TokenStoreDelegationSecretManager and just call it DelegationTokenSecretManager. Also, TokenStore instead of being contained within a class, I think should exist independently of it.
          Hide
          Ashutosh Chauhan added a comment -

          @Ed,

          • ZK is optional for storing tokens. If someone believes ZK doesn't serve them appropriately for this purpose then they can implement TokenStore interface and use that implementation for storing tokens.
          • Assumption is that ZK's api will be backward compatible. So, there will be minimal changes (if at all) required in this code to take advantage of kerberized ZK.
          Show
          Ashutosh Chauhan added a comment - @Ed, ZK is optional for storing tokens. If someone believes ZK doesn't serve them appropriately for this purpose then they can implement TokenStore interface and use that implementation for storing tokens. Assumption is that ZK's api will be backward compatible. So, there will be minimal changes (if at all) required in this code to take advantage of kerberized ZK.
          Hide
          Edward Capriolo added a comment -

          If I understand correctly storing tokens in something third party that is not kerberiz'ed defeats the purpose of kerberos. I am not expressly -1. But it seems like doing it once right when ZK has kerberos is better then doing the work twice and in the interim have a quasi-secure solution.

          Show
          Edward Capriolo added a comment - If I understand correctly storing tokens in something third party that is not kerberiz'ed defeats the purpose of kerberos. I am not expressly -1. But it seems like doing it once right when ZK has kerberos is better then doing the work twice and in the interim have a quasi-secure solution.
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/2721/
          -----------------------------------------------------------

          (Updated 2011-11-17 00:57:32.772077)

          Review request for Carl Steinbach, Ashutosh Chauhan and Devaraj Das.

          Changes
          -------

          https://issues.apache.org/jira/browse/HIVE-2467?focusedCommentId=13151686&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13151686

          Functional test: Running 2 metastore servers pointed to ZK and a test client that obtains token through first server and uses it to perform metastore op through the other server. - PASSED

          Summary
          -------

          https://issues.apache.org/jira/browse/HIVE-2467

          This addresses bug HIVE-2467.
          https://issues.apache.org/jira/browse/HIVE-2467

          Diffs (updated)


          trunk/shims/ivy.xml 1202918
          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java 1202918
          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/MemoryTokenStore.java PRE-CREATION
          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java PRE-CREATION
          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/ZooKeeperTokenStore.java PRE-CREATION
          trunk/shims/src/0.20S/java/org/apache/hadoop/security/token/delegation/HiveDelegationTokenSupport.java PRE-CREATION
          trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java 1202918

          Diff: https://reviews.apache.org/r/2721/diff

          Testing
          -------

          unit test added, ant clean package test - passed

          Thanks,

          Thomas

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2721/ ----------------------------------------------------------- (Updated 2011-11-17 00:57:32.772077) Review request for Carl Steinbach, Ashutosh Chauhan and Devaraj Das. Changes ------- https://issues.apache.org/jira/browse/HIVE-2467?focusedCommentId=13151686&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13151686 Functional test: Running 2 metastore servers pointed to ZK and a test client that obtains token through first server and uses it to perform metastore op through the other server. - PASSED Summary ------- https://issues.apache.org/jira/browse/HIVE-2467 This addresses bug HIVE-2467 . https://issues.apache.org/jira/browse/HIVE-2467 Diffs (updated) trunk/shims/ivy.xml 1202918 trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java 1202918 trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/MemoryTokenStore.java PRE-CREATION trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java PRE-CREATION trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/ZooKeeperTokenStore.java PRE-CREATION trunk/shims/src/0.20S/java/org/apache/hadoop/security/token/delegation/HiveDelegationTokenSupport.java PRE-CREATION trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java 1202918 Diff: https://reviews.apache.org/r/2721/diff Testing ------- unit test added, ant clean package test - passed Thanks, Thomas
          Hide
          Thomas Weise added a comment -

          @Ashutosh:

          I will file JIRA to incorporate relevant changes to Hadoop trunk so the next shim version (.23 and later) can use it from there (without the various "tricks" to make this work for .20). I would suggest to address the TokenStore first class interface as part of that patch?

          The logic that puts tokens into the cache and removes them immediately is necessary due to field/method visibility issues in the Hadoop code. All that won't be needed when we prepare a Hadoop patch. I added a few comments in the code.

          Show
          Thomas Weise added a comment - @Ashutosh: I will file JIRA to incorporate relevant changes to Hadoop trunk so the next shim version (.23 and later) can use it from there (without the various "tricks" to make this work for .20). I would suggest to address the TokenStore first class interface as part of that patch? The logic that puts tokens into the cache and removes them immediately is necessary due to field/method visibility issues in the Hadoop code. All that won't be needed when we prepare a Hadoop patch. I added a few comments in the code.
          Hide
          Thomas Weise added a comment -

          Updated patch:

          • No need to load master keys on retrievePassword
          • Make MemoryStore default when no token store is configured (instead of using old implementation)
          Show
          Thomas Weise added a comment - Updated patch: No need to load master keys on retrievePassword Make MemoryStore default when no token store is configured (instead of using old implementation)
          Hide
          Ashutosh Chauhan added a comment -
          • Does it make sense to have TokenStore as a first class interface instead being contained in TokenStoreDelegationTokenSecretManager and then secret manager contain an instance of it?
          • I think it makes sense to always use TokenStore based DelegationTokenSecretManager and use MemoryStore as default case instead of creating two code paths in ThriftAuthBridge20S.
          • Various tricks are currently done to get around limited interfaces exported by Hadoop. We should file jiras on hadoop so some of this could be eventually be migrated there.
          • After retrieving tokens from tokenstore we put them in abstract class's maps, pull it back and then delete immediately. This looks unnecessary. If possible we should avoid that.
          Show
          Ashutosh Chauhan added a comment - Does it make sense to have TokenStore as a first class interface instead being contained in TokenStoreDelegationTokenSecretManager and then secret manager contain an instance of it? I think it makes sense to always use TokenStore based DelegationTokenSecretManager and use MemoryStore as default case instead of creating two code paths in ThriftAuthBridge20S. Various tricks are currently done to get around limited interfaces exported by Hadoop. We should file jiras on hadoop so some of this could be eventually be migrated there. After retrieving tokens from tokenstore we put them in abstract class's maps, pull it back and then delete immediately. This looks unnecessary. If possible we should avoid that.
          Hide
          Andreas Neumann added a comment -

          If Hadoop .23 relies on ZooKeeper, then I would hope that ZK is already baked in... otherwise I would tend to think, if ZK is tolerably secure for Hadoop, then it should be tolerable for Hive/HCatalog. After all, afaik both Hive and HCatalog piggy back on HDFS for security.

          But I like the idea of having different implementations for the TokenStore, even though my preferred deployment would be ZooKeeper-based.

          Show
          Andreas Neumann added a comment - If Hadoop .23 relies on ZooKeeper, then I would hope that ZK is already baked in... otherwise I would tend to think, if ZK is tolerably secure for Hadoop, then it should be tolerable for Hive/HCatalog. After all, afaik both Hive and HCatalog piggy back on HDFS for security. But I like the idea of having different implementations for the TokenStore, even though my preferred deployment would be ZooKeeper-based.
          Hide
          Thomas Weise added a comment -

          Yes, this was the intention. Also, the current ZK token store implementation could be translated into a SQL implementation with relatively small effort (it does not depend on ZK watches for invalidation). For our use, we plan to deal with the security limitation at L3 and use ZK security when it becomes available. Longer term, assuming there will be use cases outside HCatalog/Hive for long running processes that require token support in conjunction with HA but without HA RDBMS already available, a ZK based token storage might be a more likely choice. While plain HDFS is secure it does not support the access pattern needed here (fast random reads and writes).

          Show
          Thomas Weise added a comment - Yes, this was the intention. Also, the current ZK token store implementation could be translated into a SQL implementation with relatively small effort (it does not depend on ZK watches for invalidation). For our use, we plan to deal with the security limitation at L3 and use ZK security when it becomes available. Longer term, assuming there will be use cases outside HCatalog/Hive for long running processes that require token support in conjunction with HA but without HA RDBMS already available, a ZK based token storage might be a more likely choice. While plain HDFS is secure it does not support the access pattern needed here (fast random reads and writes).
          Hide
          Ashutosh Chauhan added a comment -

          ZK security in upcoming release may require some time to bake in. Another alternative approaches could have been to store tokens in hdfs or mysql server. Both of them are secure right now. Though, good part of your design is you have made TokenStore abstract so there could be multiple implementations of it, if need be.

          Show
          Ashutosh Chauhan added a comment - ZK security in upcoming release may require some time to bake in. Another alternative approaches could have been to store tokens in hdfs or mysql server. Both of them are secure right now. Though, good part of your design is you have made TokenStore abstract so there could be multiple implementations of it, if need be.
          Hide
          Thomas Weise added a comment -

          That is correct. The ZooKeeper installation that will be used for this purpose will have to be secured on layer 3 until we have security available in ZK (which is also coming). We are aware of and consider this a limitation that will be addressed by ZK security, not by the client using ZK.

          Show
          Thomas Weise added a comment - That is correct. The ZooKeeper installation that will be used for this purpose will have to be secured on layer 3 until we have security available in ZK (which is also coming). We are aware of and consider this a limitation that will be addressed by ZK security, not by the client using ZK.
          Hide
          Ashutosh Chauhan added a comment -

          Tokens are not cached. Each server will fetch the token from ZooKeeper when the client establishes a metastore connection

          AFAIK ZK currently doesn't have a security. So, is it possible that a malicious user connects to ZK and gets hold of all the tokens and then can exploit those?

          Show
          Ashutosh Chauhan added a comment - Tokens are not cached. Each server will fetch the token from ZooKeeper when the client establishes a metastore connection AFAIK ZK currently doesn't have a security. So, is it possible that a malicious user connects to ZK and gets hold of all the tokens and then can exploit those?
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/2721/
          -----------------------------------------------------------

          (Updated 2011-11-04 02:34:21.317393)

          Review request for Carl Steinbach, Ashutosh Chauhan and Devaraj Das.

          Summary (updated)
          -------

          https://issues.apache.org/jira/browse/HIVE-2467

          This addresses bug HIVE-2467.
          https://issues.apache.org/jira/browse/HIVE-2467

          Diffs


          trunk/shims/ivy.xml 1196916
          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java 1196916
          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/MemoryTokenStore.java PRE-CREATION
          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java PRE-CREATION
          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/ZooKeeperTokenStore.java PRE-CREATION
          trunk/shims/src/0.20S/java/org/apache/hadoop/security/token/delegation/HiveDelegationTokenSupport.java PRE-CREATION
          trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java 1196916

          Diff: https://reviews.apache.org/r/2721/diff

          Testing
          -------

          unit test added, ant clean package test - passed

          Thanks,

          Thomas

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2721/ ----------------------------------------------------------- (Updated 2011-11-04 02:34:21.317393) Review request for Carl Steinbach, Ashutosh Chauhan and Devaraj Das. Summary (updated) ------- https://issues.apache.org/jira/browse/HIVE-2467 This addresses bug HIVE-2467 . https://issues.apache.org/jira/browse/HIVE-2467 Diffs trunk/shims/ivy.xml 1196916 trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java 1196916 trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/MemoryTokenStore.java PRE-CREATION trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java PRE-CREATION trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/ZooKeeperTokenStore.java PRE-CREATION trunk/shims/src/0.20S/java/org/apache/hadoop/security/token/delegation/HiveDelegationTokenSupport.java PRE-CREATION trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java 1196916 Diff: https://reviews.apache.org/r/2721/diff Testing ------- unit test added, ant clean package test - passed Thanks, Thomas
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/2721/
          -----------------------------------------------------------

          Review request for Carl Steinbach, Ashutosh Chauhan and Devaraj Das.

          Summary
          -------

          https://issues.apache.org/jira/browse/HIVE-2467

          This addresses bug HIVE-2467.
          https://issues.apache.org/jira/browse/HIVE-2467

          Diffs


          trunk/shims/ivy.xml 1196916
          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java 1196916
          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/MemoryTokenStore.java PRE-CREATION
          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java PRE-CREATION
          trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/ZooKeeperTokenStore.java PRE-CREATION
          trunk/shims/src/0.20S/java/org/apache/hadoop/security/token/delegation/HiveDelegationTokenSupport.java PRE-CREATION
          trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java 1196916

          Diff: https://reviews.apache.org/r/2721/diff

          Testing
          -------

          unit test added, ant clean package test - passed

          Thanks,

          Thomas

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2721/ ----------------------------------------------------------- Review request for Carl Steinbach, Ashutosh Chauhan and Devaraj Das. Summary ------- https://issues.apache.org/jira/browse/HIVE-2467 This addresses bug HIVE-2467 . https://issues.apache.org/jira/browse/HIVE-2467 Diffs trunk/shims/ivy.xml 1196916 trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java 1196916 trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/MemoryTokenStore.java PRE-CREATION trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TokenStoreDelegationTokenSecretManager.java PRE-CREATION trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/ZooKeeperTokenStore.java PRE-CREATION trunk/shims/src/0.20S/java/org/apache/hadoop/security/token/delegation/HiveDelegationTokenSupport.java PRE-CREATION trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java 1196916 Diff: https://reviews.apache.org/r/2721/diff Testing ------- unit test added, ant clean package test - passed Thanks, Thomas
          Hide
          Carl Steinbach added a comment -
          Show
          Carl Steinbach added a comment - @Thomas: Please submit a review request on reviewboard https://cwiki.apache.org/confluence/display/Hive/HowToContribute#HowToContribute-ReviewProcess
          Hide
          Thomas Weise added a comment -

          Attached is the initial patch (with unit test coverage). Limited end to end testing has been done to validate the token sharing across multiple servers. No scalability testing has been done yet.

          Changes all go into the Hadoop 20S shim module (depend on secure Hadoop). Everything except the ZooKeeper token store implementation logically extends Hadoop security. AbstractDelegationTokenSecretManager was not designed for extensibility and this reflects in the subclass introduced here, which nevertheless duplicates as little code as possible. Longer term Hadoop should support the token store abstraction or at least more appropriate hooks for extension and dependency injection, which will allow for significant enhanced simplicity and clarity (and of course sharing with other projects).

          The ZooKeeper backed token store: The delegation keys and tokens are stored in ZooKeeper. Multiple metastore servers access ZooKeeper and load keys and tokens from there. Keys are cached and each metastore server instance creates its dedicated key, shared through ZooKeeper with other server instances for token validation. Tokens are not cached. Each server will fetch the token from ZooKeeper when the client establishes a metastore connection (not per request, but per connection).

          By default, none of the extensions introduced with this patch will be used (previous behavior and dependencies remain intact). To enable ZooKeeper token store, additional configuration is needed (on top of secure metastore setup):

          <property>
          <name>hive.cluster.delegation.token.store.class</name>
          <value>org.apache.hadoop.hive.thrift.ZooKeeperTokenStore</value>
          <description>The delegation token store implementation class.</description>
          </property>

          <property>
          <name>hive.cluster.delegation.token.store.zookeeper.connectString</name>
          <value>localhost:2181</value>
          <description>The ZooKeeper token store connect string.</description>
          </property>

          <property>
          <name>hive.cluster.delegation.token.store.zookeeper.rootNode</name>
          <value>/hcat/tokenstore</value>
          <description>The root path for token store data.</description>
          </property>

          There are two potential enhancements I would like to put up for discussion:

          1) Partition token storage by master key id (right now all tokens are children of a single ZooKeeper node. This will make it easier to manually inspect the store and make token expiration more efficient (no need to read all children at once).

          2) Introduce dependency injection point for HadoopThriftAuthBridge20S.createServer, which can be used to plug-in alternative implementations of TokenStore, DelegationTokenManager and potentially other objects for customization and unit testing.

          Show
          Thomas Weise added a comment - Attached is the initial patch (with unit test coverage). Limited end to end testing has been done to validate the token sharing across multiple servers. No scalability testing has been done yet. Changes all go into the Hadoop 20S shim module (depend on secure Hadoop). Everything except the ZooKeeper token store implementation logically extends Hadoop security. AbstractDelegationTokenSecretManager was not designed for extensibility and this reflects in the subclass introduced here, which nevertheless duplicates as little code as possible. Longer term Hadoop should support the token store abstraction or at least more appropriate hooks for extension and dependency injection, which will allow for significant enhanced simplicity and clarity (and of course sharing with other projects). The ZooKeeper backed token store: The delegation keys and tokens are stored in ZooKeeper. Multiple metastore servers access ZooKeeper and load keys and tokens from there. Keys are cached and each metastore server instance creates its dedicated key, shared through ZooKeeper with other server instances for token validation. Tokens are not cached. Each server will fetch the token from ZooKeeper when the client establishes a metastore connection (not per request, but per connection). By default, none of the extensions introduced with this patch will be used (previous behavior and dependencies remain intact). To enable ZooKeeper token store, additional configuration is needed (on top of secure metastore setup): <property> <name>hive.cluster.delegation.token.store.class</name> <value>org.apache.hadoop.hive.thrift.ZooKeeperTokenStore</value> <description>The delegation token store implementation class.</description> </property> <property> <name>hive.cluster.delegation.token.store.zookeeper.connectString</name> <value>localhost:2181</value> <description>The ZooKeeper token store connect string.</description> </property> <property> <name>hive.cluster.delegation.token.store.zookeeper.rootNode</name> <value>/hcat/tokenstore</value> <description>The root path for token store data.</description> </property> There are two potential enhancements I would like to put up for discussion: 1) Partition token storage by master key id (right now all tokens are children of a single ZooKeeper node. This will make it easier to manually inspect the store and make token expiration more efficient (no need to read all children at once). 2) Introduce dependency injection point for HadoopThriftAuthBridge20S.createServer, which can be used to plug-in alternative implementations of TokenStore, DelegationTokenManager and potentially other objects for customization and unit testing.

            People

            • Assignee:
              Thomas Weise
              Reporter:
              Thomas Weise
            • Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development