----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52800/#review155256 -----------------------------------------------------------
The patch looks good, however, I have a couple of questions. common/src/java/org/apache/hadoop/hive/conf/HiveConf.java (line 764) <https://reviews.apache.org/r/52800/#comment225169> 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? common/src/java/org/apache/hadoop/hive/conf/HiveConf.java (line 768) <https://reviews.apache.org/r/52800/#comment225171> 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. metastore/src/java/org/apache/hadoop/hive/metastore/AlterHandler.java (line 86) <https://reviews.apache.org/r/52800/#comment225118> 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. - Chaoyu Tang 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 > >
