> 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 > >
