----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68827/#review209031 -----------------------------------------------------------
Code looks clean, but I have some scary questions. standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java Lines 517 (patched) <https://reviews.apache.org/r/68827/#comment293294> Make this clearer. A list of comma separated regeses that are used to reduced the size of Hms Notifiaction messages. If a partition name (?) or table name (?) matches a regex then the partition or table is excluded from the notification. Or something standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java Lines 1412 (patched) <https://reviews.apache.org/r/68827/#comment293295> Add javadoc standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java Lines 913 (patched) <https://reviews.apache.org/r/68827/#comment293296> Good doumentation! Are you 100% sure that this Map is never sused by anyone else? What about future code? standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java Lines 917 (patched) <https://reviews.apache.org/r/68827/#comment293297> make private standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java Lines 106 (patched) <https://reviews.apache.org/r/68827/#comment293298> So what if I make a miskae in a regex in config? This code will fail, what will happen then? How will I know what failed if my server won't start? standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java Line 297 (original), 310 (patched) <https://reviews.apache.org/r/68827/#comment293302> This is used by the notifications that (we think) we understand, but it is also used by JSONAcidWriteMessage. So what happens if someone uses your new mechanism to reduce the size of messages, but affects JSONAcidWriteMessage? In other words there could be multile uses for notifications in a complex system, and this mechanism affects them all. standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/common/TestMetaStoreUtils.java Lines 35 (patched) <https://reviews.apache.org/r/68827/#comment293299> Add javadoc standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/common/TestMetaStoreUtils.java Lines 36 (patched) <https://reviews.apache.org/r/68827/#comment293300> We already have TestMetaStoreServerUtils, does this need a new file? standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/common/TestMetaStoreUtils.java Lines 66 (patched) <https://reviews.apache.org/r/68827/#comment293301> add test with the default param map from MetastoreConf - Andrew Sherman On Sept. 24, 2018, 8:37 p.m., Bharathkrishna Guruvayoor Murali wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68827/ > ----------------------------------------------------------- > > (Updated Sept. 24, 2018, 8:37 p.m.) > > > Review request for hive and Alexander Kolbasov. > > > Repository: hive-git > > > Description > ------- > > Clients can add large-sized parameters in Table/Partition objects. So we need > to enable adding regex patterns through HiveConf to match parameters to be > filtered from table and partition objects before serialization in HMS > notifications. > > > Diffs > ----- > > > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java > 30ea7f8129 > > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java > c681a87a1c > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java > 2668b05320 > > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/common/TestMetaStoreUtils.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/68827/diff/1/ > > > Testing > ------- > > > Thanks, > > Bharathkrishna Guruvayoor Murali > >
