> On July 19, 2017, 11:46 a.m., Mohit Sabharwal wrote: > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java > > Lines 471 (patched) > > <https://reviews.apache.org/r/60950/diff/3/?file=1779140#file1779140line473> > > > > Don't think you need this sleep since you are explicitly calling > > HiveMetaStoreClient#close
It looks close -> thrift#shutdown is async call (inherited in ThriftHiveMetastore via FacebookService.iface), hence I guess sleep would be necessary. > On July 19, 2017, 11:46 a.m., Mohit Sabharwal wrote: > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java > > Lines 500 (patched) > > <https://reviews.apache.org/r/60950/diff/3/?file=1779140#file1779140line502> > > > > closing explicitly, so sleep should not be needed. Same as above. > On July 19, 2017, 11:46 a.m., Mohit Sabharwal wrote: > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java > > Lines 515 (patched) > > <https://reviews.apache.org/r/60950/diff/3/?file=1779140#file1779140line517> > > > > same here. Same as above. > On July 19, 2017, 11:46 a.m., Mohit Sabharwal wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > > Lines 298 (patched) > > <https://reviews.apache.org/r/60950/diff/3/?file=1779141#file1779141line298> > > > > You can set this to null here. And explicitly initialize the thread > > local in setMetaConf. That way, you don't need to create HashMap for every > > thread if setMetaConf is never called (common case). Done. > On July 19, 2017, 11:46 a.m., Mohit Sabharwal wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > > Lines 395 (patched) > > <https://reviews.apache.org/r/60950/diff/3/?file=1779141#file1779141line395> > > > > null check needed if you init this to null based on feedback comment. Done. Did a smaller change to handle unexpected null case of threadLocalConf on the similar lines. > On July 19, 2017, 11:46 a.m., Mohit Sabharwal wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > > Lines 7598 (patched) > > <https://reviews.apache.org/r/60950/diff/3/?file=1779141#file1779141line7611> > > > > Once you initialize threadLocalModifiedConfig to null, check if > > threadLocalModifiedConfig is non null before calling > > notifyMetaListenersOnShutDown I am already checking for handler (which gets initialized in along with threadLocalModifiedConfig in setMetaConf), so not adding additional check. - PRASHANT ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60950/#review180914 ----------------------------------------------------------- On July 19, 2017, 8:49 p.m., PRASHANT GOLASH wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60950/ > ----------------------------------------------------------- > > (Updated July 19, 2017, 8:49 p.m.) > > > Review request for hive. > > > Repository: hive-git > > > Description > ------- > > Added the code to notify meta listeners during shutdown. Shutdown would > eventually call cleanupRawStore (In both cases HMSHandler#close and > TServerEventHandler#DeleteContext), so called the notification code in that > function. > > > Diffs > ----- > > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java > fd4527e653 > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > 58b9044930 > > > Diff: https://reviews.apache.org/r/60950/diff/4/ > > > Testing > ------- > > Added unit test cases for the affected codepath. > > > Thanks, > > PRASHANT GOLASH > >
