----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54826/#review159520 -----------------------------------------------------------
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java (line 440) <https://reviews.apache.org/r/54826/#comment230534> lets keep this disabled for now as this work is in early stages common/src/java/org/apache/hadoop/hive/conf/HiveConf.java (line 442) <https://reviews.apache.org/r/54826/#comment230535> any HCFS should work for this (Hadoop compatible file system) (at least eventually). I think we can remove the HDFS reference. common/src/java/org/apache/hadoop/hive/conf/HiveConf.java (line 443) <https://reviews.apache.org/r/54826/#comment230536> should we use hours instead as default unit ? it seems most people would think of this parameter in hours/days. itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestReplChangeManager.java (line 56) <https://reviews.apache.org/r/54826/#comment230765> with recent changes by Vaibhav to ProxyFileSystem, local files would also have checksums. Can we switch to local files, so that test speed is better (local fs might be more reliable than minidfs, as it has less parts to it!) itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestReplChangeManager.java (line 59) <https://reviews.apache.org/r/54826/#comment230766> why is this needed ? itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestReplChangeManager.java (line 124) <https://reviews.apache.org/r/54826/#comment230768> how about creating a method for this and calling -Partition part1 = createAndPartition("20160101") itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestReplChangeManager.java (line 241) <https://reviews.apache.org/r/54826/#comment230772> can you add a comment like - // verify cm.recycle(Path) api moves file to cmroot dir itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestReplChangeManager.java (line 247) <https://reviews.apache.org/r/54826/#comment230773> // verify cm.recycle(db, table) api moves file to cmroot dir metastore/src/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java (line 151) <https://reviews.apache.org/r/54826/#comment230782> should we call it getCksumString() to make it more easy to understand what is expected ? metastore/src/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java (line 175) <https://reviews.apache.org/r/54826/#comment230776> is there any generic way to find this from FileSystem api ? these configs are specific to HDFS. But i guess we don't need this if we go with storing only the signature (see following comment) metastore/src/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java (line 182) <https://reviews.apache.org/r/54826/#comment230780> as discussed offline, looks like we don't want to rely on the directory structure of table. This can cause problems in finding file in case of insert -> rename -> drop . The signature should get used as the 'key' for finding the file. metastore/src/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java (line 205) <https://reviews.apache.org/r/54826/#comment230774> can you add a log message to indicate that this has started. Since this would typically be run only once in hour or so , INFO level should be good IMO metastore/src/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java (line 225) <https://reviews.apache.org/r/54826/#comment230775> can you add a debug level log message for when files get deleted ? - Thejas Nair On Dec. 16, 2016, 11:14 p.m., Daniel Dai wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54826/ > ----------------------------------------------------------- > > (Updated Dec. 16, 2016, 11:14 p.m.) > > > Review request for hive and Thejas Nair. > > > Repository: hive-git > > > Description > ------- > > See HIVE-15448 > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 9064e49 > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestReplChangeManager.java > PRE-CREATION > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > f7b2ed7 > metastore/src/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java > PRE-CREATION > metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java 6aca1b7 > > Diff: https://reviews.apache.org/r/54826/diff/ > > > Testing > ------- > > > Thanks, > > Daniel Dai > >
