Re: Review Request: Addressing Carl's comments.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/618/#review522 --- trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java https://reviews.apache.org/r/618/#comment1070 spacing trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java https://reviews.apache.org/r/618/#comment1071 formatting trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java https://reviews.apache.org/r/618/#comment1072 spacing trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java https://reviews.apache.org/r/618/#comment1073 spacing trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java https://reviews.apache.org/r/618/#comment1074 spacing trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java https://reviews.apache.org/r/618/#comment1075 This change should go in a separate ticket. What does it mean to finalize a partition? trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreEventListener.java https://reviews.apache.org/r/618/#comment1076 spacing trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreEventListener.java https://reviews.apache.org/r/618/#comment1087 Instead of passing in raw Table/Partition/Database objects please wrap these objects in containers, e.g. CreateTableEvent, DropTableEvent, etc. Q: Whats the advantage of wrapper container objects? The advantage of container objects is that it allows us to evolve the API without breaking compatibility with older clients. With the current interface if I want to add a parameter to one of these methods I will break compatibility, but if we use container objects I can add a new field to the container without affecting older clients. trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreEventListener.java https://reviews.apache.org/r/618/#comment1077 spacing trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreEventListener.java https://reviews.apache.org/r/618/#comment1078 spacing trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreEventListener.java https://reviews.apache.org/r/618/#comment1079 spacing trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreEventListener.java https://reviews.apache.org/r/618/#comment1080 Please remove this and include it in a separate patch. We need to discuss what finalizing a partition actually means, and how it relates to locking. trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java https://reviews.apache.org/r/618/#comment1083 spacing trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java https://reviews.apache.org/r/618/#comment1082 spacing trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java https://reviews.apache.org/r/618/#comment1081 spacing trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java https://reviews.apache.org/r/618/#comment1086 spacing - Carl On 2011-04-18 18:44:05, Ashutosh Chauhan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/618/ --- (Updated 2011-04-18 18:44:05) Review request for hive and Carl Steinbach. Summary --- Addresses Carl's comment for previous patch. Also added a new method finalizePartition in metastore through which metastore client can indicate to metastore that partition can be finalized. This addresses bug HIVE-2038. https://issues.apache.org/jira/browse/HIVE-2038 Diffs - trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1094688 trunk/conf/hive-default.xml 1094688 trunk/metastore/if/hive_metastore.thrift 1094688 trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h 1094688 trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp 1094688 trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp 1094688 trunk/metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java 1094688 trunk/metastore/src/gen/thrift/gen-php/hive_metastore/ThriftHiveMetastore.php 1094688 trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote 1094688 trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py 1094688 trunk/metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb 1094688 trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 1094688
Re: Review Request: Addressing Carl's comments.
On 2011-04-21 22:51:56, Carl Steinbach wrote: trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreEventListener.java, line 41 https://reviews.apache.org/r/618/diff/1/?file=15927#file15927line41 Instead of passing in raw Table/Partition/Database objects please wrap these objects in containers, e.g. CreateTableEvent, DropTableEvent, etc. Q: Whats the advantage of wrapper container objects? The advantage of container objects is that it allows us to evolve the API without breaking compatibility with older clients. With the current interface if I want to add a parameter to one of these methods I will break compatibility, but if we use container objects I can add a new field to the container without affecting older clients. It seems like this same concern also dictates that MetaStoreListener should be an abstract class instead of an interface. Can you please make this change? - Carl --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/618/#review522 --- On 2011-04-18 18:44:05, Ashutosh Chauhan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/618/ --- (Updated 2011-04-18 18:44:05) Review request for hive and Carl Steinbach. Summary --- Addresses Carl's comment for previous patch. Also added a new method finalizePartition in metastore through which metastore client can indicate to metastore that partition can be finalized. This addresses bug HIVE-2038. https://issues.apache.org/jira/browse/HIVE-2038 Diffs - trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1094688 trunk/conf/hive-default.xml 1094688 trunk/metastore/if/hive_metastore.thrift 1094688 trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h 1094688 trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp 1094688 trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp 1094688 trunk/metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java 1094688 trunk/metastore/src/gen/thrift/gen-php/hive_metastore/ThriftHiveMetastore.php 1094688 trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote 1094688 trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py 1094688 trunk/metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb 1094688 trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 1094688 trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 1094688 trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 1094688 trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreEventListener.java PRE-CREATION trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 1094688 trunk/metastore/src/java/org/apache/hadoop/hive/metastore/NoOpListener.java PRE-CREATION trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 1094688 trunk/metastore/src/test/org/apache/hadoop/hive/metastore/DummyListener.java PRE-CREATION trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java PRE-CREATION Diff: https://reviews.apache.org/r/618/diff Testing --- Thanks, Ashutosh
Re: Review Request: Addressing Carl's comments.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/618/#review526 --- trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java https://reviews.apache.org/r/618/#comment1090 I think it should be possible to specify multiple metastore listeners. Can you please make the following changes: * Change the name of the conf property from hive.metastore.listener to hive.metastore.event.listeners and update the description to indicate that this is a comma separated list of listener classnames. * Change the default value for this property to be an empty string. * Update the code that initializes these listeners. You can cut and paste the code that's used to initialize hive.exec.pre.hooks - Carl On 2011-04-18 18:44:05, Ashutosh Chauhan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/618/ --- (Updated 2011-04-18 18:44:05) Review request for hive and Carl Steinbach. Summary --- Addresses Carl's comment for previous patch. Also added a new method finalizePartition in metastore through which metastore client can indicate to metastore that partition can be finalized. This addresses bug HIVE-2038. https://issues.apache.org/jira/browse/HIVE-2038 Diffs - trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1094688 trunk/conf/hive-default.xml 1094688 trunk/metastore/if/hive_metastore.thrift 1094688 trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h 1094688 trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp 1094688 trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp 1094688 trunk/metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java 1094688 trunk/metastore/src/gen/thrift/gen-php/hive_metastore/ThriftHiveMetastore.php 1094688 trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote 1094688 trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py 1094688 trunk/metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb 1094688 trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 1094688 trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 1094688 trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 1094688 trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreEventListener.java PRE-CREATION trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 1094688 trunk/metastore/src/java/org/apache/hadoop/hive/metastore/NoOpListener.java PRE-CREATION trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 1094688 trunk/metastore/src/test/org/apache/hadoop/hive/metastore/DummyListener.java PRE-CREATION trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java PRE-CREATION Diff: https://reviews.apache.org/r/618/diff Testing --- Thanks, Ashutosh
