> On Nov. 8, 2016, 4:13 p.m., Chaoyu Tang wrote: > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java, line 790 > > <https://reviews.apache.org/r/52800/diff/1/?file=1556279#file1556279line790> > > > > What kind of events need to be invoked in a separate JDO transactions? > > or did you mean the events that do not require a JDO transaction?
There are few scenarios where it may make sense to still use separate JDO transactions (and hence the old config), like: (1) When the listener method invocation is expensive. For example, say an external authorization service has it's own listener implementation that makes an RPC call to an (external) server. This call may block forever. To avoid rolling back original metadata from HMS db, user may want to do these in separate transactions -- with the understanding that the two may get inconsistent. If, on the other hand, the listener is doing a relatively predictable operation like adding notification to a queue or persisting to the HMS db schema (like DbNotificationListener), it may be reasonable to expect that both happen in the same transaction. This is the motivation for the new config. (2) When the notification needs to be logged irrespective of whether the metadata event failed or succeeds. Here the notification acts more like a audit. (3) For backward compatibility. > On Nov. 8, 2016, 4:13 p.m., Chaoyu Tang wrote: > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java, line 794 > > <https://reviews.apache.org/r/52800/diff/1/?file=1556279#file1556279line794> > > > > I wonder why we need a separate transactional event listener in Hive. > > Can it be deferred to listener implementation? For example, a listener > > which needs be transactional first checks if it is under a transaction > > context, joins (or enlists) itself to the transaction if it is. This way > > might also be applicable to the cases where multiple backends invovle (e.g. > > distributed transaction). It is just my thought, and may not necessarily be > > right or feasible. Any implementation of MetaStoreEventListener interface can be configured so that it's invoked in the same transaction as the metadata event. This patch does not introduce any separate transactional event listener. If user wants both the metadata event and notification to happen in the same JDO transaction, they configure hive.metastore.transactional.event.listeners, otherwise they configure hive.metastore.event.listeners. This way the interface implementation does not need to be changed. Only the configuration needs to be changed. > On Nov. 8, 2016, 4:13 p.m., Chaoyu Tang wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/AlterHandler.java, line > > 86 > > <https://reviews.apache.org/r/52800/diff/1/?file=1556283#file1556283line86> > > > > I wonder if the API changes will cause some backward compatibility > > issue, if the HiveMetaStore is currently the only consumer of the > > AlterHandler, it should be fine. That's a good point. I have created new versions of these methods. But adding new methods makes it backward compatibility as well, right ? I'm not sure there is a good solution here. - Mohit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52800/#review155256 ----------------------------------------------------------- On Nov. 7, 2016, 11:25 p.m., Mohit Sabharwal wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52800/ > ----------------------------------------------------------- > > (Updated Nov. 7, 2016, 11:25 p.m.) > > > Review request for hive. > > > Bugs: HIVE-13966 > https://issues.apache.org/jira/browse/HIVE-13966 > > > Repository: hive-git > > > Description > ------- > > Metadata event and associated notification should be committed in the same > JDO transaction. > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java > 80cd5ada060331797a603848e268c7d2a78a679c > > itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java > PRE-CREATION > > itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java > 81ce67bdc8fdaf11ff4fec3f255ed0021a4752c7 > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java > af16f75e63c372c37bfd73567b777bba53f94db3 > metastore/src/java/org/apache/hadoop/hive/metastore/AlterHandler.java > dedd4497adfcc9d57090a943f6bb4f35ea87fa61 > metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java > 40b337a9e40ea04a37f108146853d2d1f42dcd29 > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > 60e462fd06a3f84d5b87cd335afb49768cb27562 > > Diff: https://reviews.apache.org/r/52800/diff/ > > > Testing > ------- > > Enhanced TestDbNotificationListener > > > Thanks, > > Mohit Sabharwal > >
