> On July 27, 2012, 10:09 p.m., Francis Liu wrote: > > First pass skipped a few things but wanted to give early feedback. My main > > concern is that I don't think HiveMetaStoreClient is threadsafe as the > > thrift client is not? If that is the case then having the cache thread > > local will be a cleaner approach. > > Arup Malakar wrote: > If HiveMetaStoreClient cannot be shared across threads then what you say > is right. > > Rohini, what is your opinion? > > I have addressed the rest of the comments (few of them were already taken > care of in the 5th version: https://reviews.apache.org/r/6114/diff/5/ I will > update the review incorporating the "ThreadLocalality" once Rohini gives her > feedback.
According to https://issues.apache.org/jira/browse/HIVE-84 the HiveMetastoreClient is threadsafe and they are internally using threadlocal looking at the patch. May be that has changed. Ashutosh might know better. But I have used multiple instances of HiveMetastoreClient in different threads for two different servers when I wrote the HA testsuite and there were no issues. But then I only did getDatabases and getTables in it. The Driver class I have heard is not thread safe though. Can you send a mail to Ashutosh and check? > On July 27, 2012, 10:09 p.m., Francis Liu wrote: > > http://svn.apache.org/repos/asf/incubator/hcatalog/branches/branch-0.4/src/java/org/apache/hcatalog/common/cache/HiveClientCache.java, > > line 97 > > <https://reviews.apache.org/r/6114/diff/3/?file=130091#file130091line97> > > > > Just catch exception It would be safer to do Throwable here because it is called in finalize() and shutdownhook. And since it is only close() it should not matter. - Rohini ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/6114/#review9547 ----------------------------------------------------------- On July 27, 2012, 8:38 p.m., Arup Malakar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/6114/ > ----------------------------------------------------------- > > (Updated July 27, 2012, 8:38 p.m.) > > > Review request for hcatalog and Rohini Palaniswamy. > > > Description > ------- > > Time expiry based HiveMetaStoreClient cache for HCatalog > > > This addresses bug HCATALOG-370. > https://issues.apache.org/jira/browse/HCATALOG-370 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/incubator/hcatalog/branches/branch-0.4/ivy.xml > 1364830 > > http://svn.apache.org/repos/asf/incubator/hcatalog/branches/branch-0.4/src/java/org/apache/hcatalog/common/HCatConf.java > PRE-CREATION > > http://svn.apache.org/repos/asf/incubator/hcatalog/branches/branch-0.4/src/java/org/apache/hcatalog/common/HCatUtil.java > 1364830 > > http://svn.apache.org/repos/asf/incubator/hcatalog/branches/branch-0.4/src/java/org/apache/hcatalog/common/cache/HiveClientCache.java > PRE-CREATION > > http://svn.apache.org/repos/asf/incubator/hcatalog/branches/branch-0.4/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java > 1364830 > > http://svn.apache.org/repos/asf/incubator/hcatalog/branches/branch-0.4/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java > 1364830 > > http://svn.apache.org/repos/asf/incubator/hcatalog/branches/branch-0.4/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java > 1364830 > > http://svn.apache.org/repos/asf/incubator/hcatalog/branches/branch-0.4/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java > 1364830 > > http://svn.apache.org/repos/asf/incubator/hcatalog/branches/branch-0.4/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java > 1364830 > > http://svn.apache.org/repos/asf/incubator/hcatalog/branches/branch-0.4/src/java/org/apache/hcatalog/pig/PigHCatUtil.java > 1364830 > > http://svn.apache.org/repos/asf/incubator/hcatalog/branches/branch-0.4/src/test/org/apache/hcatalog/common/TestHiveClientCache.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/6114/diff/ > > > Testing > ------- > > > Thanks, > > Arup Malakar > >
