hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chaoyu Tang <ctang...@gmail.com>
Subject Re: Review Request 44271: HIVE-12270 : Add DBTokenStore support to HS2 delegation token
Date Fri, 04 Mar 2016 01:55:18 GMT


> On March 4, 2016, 12:37 a.m., Szehon Ho wrote:
> > service/src/java/org/apache/hive/service/auth/HiveAuthFactory.java, line 129
> > <https://reviews.apache.org/r/44271/diff/2/?file=1277523#file1277523line129>
> >
> >     I'm a bit afraid of concurrency issues by caching Hive.  There seems to be quite
some issues with Hive object lately and multi-threading and caching seems discouraged now
(see HIVE-13002, HIVE-13194, HIVE-13150)
> >     
> >     Can we have DbTokenStore get Hive on demand via thread-local when it needs it,
say if hmsHandler is not passed in?
> >     
> >     And also can you double-check if it will not leak, ie Hive object is closed
somehow by the thread once its done?
> 
> Szehon Ho wrote:
>     Or I guess ServerMode solves that as well.. we can just do Hive.get instead of use
the passed-in object if its HS2 mode?

Hi Szehon, Thanks for review. 
Yeah, initally I also thought to use Hive.get to get (actually initiate) a threadLocal Hive
object and its contained MetaStoreClient in DBTokenStore. But I changed the idea because these
token API calls usually happen before a session is opened, and the HMS connection opened for
them is usually different from that used in session, since the HMS connection used later in
the session may have a different user credential rather than the HS2 owner hive. So this HMS
connection can not be reused in the session/queries. 
The Hive object now got and used for token APIs is HS2 server main thread local, its Hive.get
is called in new HiveAuthFactory(hiveConf) <- ThriftBinaryCLIService.run() <-HiveServer2.start()
HS2 thread, it won't be used by any session/queries which run in different threads. The only
one other use of this Hive object (or MSC) is CLIService.applyAuthorizationConfigPolicy in
CLIService.init during HS2 start, so this Hive object in the main thread is currently only
used for token APIs during runtime, and it should not have concrruency issue. In addition,
we only use MetaStoreClient in Hive object and not other instance variable values, and the
MetaStoreClient is a synchronized client (See Hive getMSC method, HiveMetaStoreClient.newSynchronizedClient(metaStoreClient)),
so I think passing in Hive (or HiveMetaStoreClient) and sharing it for token API calls should
be quite safe and also save a lot of HMS connections. 
I wonder above consideration makes sense or not. Thanks.


- Chaoyu


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


On March 2, 2016, 4:39 p.m., Chaoyu Tang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44271/
> -----------------------------------------------------------
> 
> (Updated March 2, 2016, 4:39 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-12270
>     https://issues.apache.org/jira/browse/HIVE-12270
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Add HMS APIs to support DB delegation token in HS2. 
> Only upload the patch without other thrift generated files for review here.
> 
> 
> Diffs
> -----
> 
>   itests/hive-minikdc/src/test/java/org/apache/hive/minikdc/MiniHiveKdc.java dedbf35

>   itests/hive-minikdc/src/test/java/org/apache/hive/minikdc/TestJdbcWithDBTokenStore.java
PRE-CREATION 
>   itests/hive-minikdc/src/test/java/org/apache/hive/minikdc/TestJdbcWithMiniKdc.java
3ef2ce3 
>   metastore/if/hive_metastore.thrift e8f0a68 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java bfebfdc 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java b5c4d1d

>   metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java cb092d1 
>   service/src/java/org/apache/hive/service/auth/HiveAuthFactory.java 0c7455d 
>   shims/common/src/main/java/org/apache/hadoop/hive/thrift/DBTokenStore.java de39d3d

> 
> Diff: https://reviews.apache.org/r/44271/diff/
> 
> 
> Testing
> -------
> 
> Manuall tests
> New Unit test TestJdbcWithDBToken
> Precommit tests
> 
> 
> Thanks,
> 
> Chaoyu Tang
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message